Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebVTT rendering tests fail widely because of UA-specific cue backgrounds #46453

Open
foolip opened this issue May 23, 2024 · 5 comments
Open
Labels

Comments

@foolip
Copy link
Member

foolip commented May 23, 2024

https://wpt.fyi/results/webvtt/rendering/cues-with-video/processing-model has widespread failures and only a few tests passing in all browsers.

A lot of Chrome, Edge and Safari failures appear to be because of UA-specific cue backgrounds. For Chromium, https://issues.chromium.org/issues/40652084 tracks the problem and I think this is the code:

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/media_controls/resources/mediaControls.css;l=1282-1287;drc=f235aed1f0b36e760cd1968e8077927d69cf3bd4

To show the problem visually, here's Chrome and Safari's current rendering of basic.html:

Chrome

Safari

Clearly these backgrounds were added deliberately for readability. Is there some other way we can make this test suite work than requiring browsers to remove margins by default?

cc @zcorpan

@zcorpan
Copy link
Member

zcorpan commented May 23, 2024

The rendering that the tests expect is required by the WebVTT spec, so we should fix the spec if we want to allow such padding.

Possibly the tests could make the background transparent and thus not test the details of the background box?

@foolip
Copy link
Member Author

foolip commented May 23, 2024

I assume that the padding can also affect the positioning of the cues, but yeah, if the spec allowed for certain variations in default style, tests could do a reset to avoid the problem. I tried removing the padding in basic.html but that doesn't work because padding isn't one of the properties listed in https://w3c.github.io/webvtt/#the-cue-pseudo-element. (There's probably some special code to still make it work from the UA style sheet.)

We could perhaps consider allowing padding for ::cue as part of the solution.

Meanwhile, I'm checking what would happen if we remove the padding in https://chromium-review.googlesource.com/c/chromium/src/+/5563687.

@gsnedders
Copy link
Member

The rendering that the tests expect is required by the WebVTT spec, so we should fix the spec if we want to allow such padding.

Has anyone filed such a spec issue?

@jernoble
Copy link
Contributor

w3c/webvtt#516

@jernoble
Copy link
Contributor

jernoble commented Sep 17, 2024

@foolip said:

We could perhaps consider allowing padding for ::cue as part of the solution.

That would likely be insufficient. You'd also need to be able to set a background color, but WebVTT requires background attributes to be applied to the inner text run of the cue, and not the wrapping box. I do think this requires a spec change, and the above WebVTT issue lays out what would likely be necessary.

For the purposes of the WPT tests, we could also consider disabling the accessibility integrations that allow users' to override the styling of captions. That's what WebKit does for its regression test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants