1950616 - WebVTT cues get oddly large on portrait videos, and cannot be styled properly

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P3)

Description

[Andreas Pehrson [:pehrsons]]

Several issues here, mainly:

• https://github.com/w3c/webvtt/issues/529 where we differ in behavior from Chrome. We seem to follow spec closer in that we set the font-size to 5% of the video element's height, but Chrome's behavior of picking 5% of min(width, height) seems sensible for portrait videos.
• https://github.com/w3c/webvtt/issues/530 where our container for the cues sets font: ~5vh sans-serif essentially prohibiting a site from styling the cues to anything smaller than the container's line-height. Per my discussion with emilio, this font style is needed on the container or we'd inherit from the video element which a site can influence. But we can make it 0 on the container just for the purpose of preventing inheritance from the video element, and set it to ~5vh on the cue elements directly, which a site can override through the ::cue pseudo element.

I say ~5vh above because we compute it manually based on the video element height. Probably because vh is wrong for ::cue: bug 1664754.

The second point above means sites cannot work around this issue even by styling, so P2.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1950616 - In WebVTT use min(width, height) for more sensible font-size for portrait videos. r?emilio!, r?alwu!

This matches Chromium's behavior.
Spec issue at https://github.com/w3c/webvtt/issues/529

Comment 2

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1950616 - In WebVTT use font-size 9px on the cue container. r?emilio!, r?alwu!

A larger font-size on the cue container prevents a site properly styling ::cue
pseudo elements to anything smaller than that font-size since the container's
line-height cannot be overriden. Per the spec issue we propose 0, but 9px is
small enough that sites won't have a need to style cues to something
significantly smaller, while still keeping WPT passing.

The spec issue is https://github.com/w3c/webvtt/issues/530

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54db16088469
In WebVTT use min(width, height) for more sensible font-size for portrait videos. r=alwu,emilio
https://hg.mozilla.org/integration/autoland/rev/519b23205a49
In WebVTT use font-size 9px on the cue container. r=alwu,emilio

Comment 4

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/51116 for changes under testing/web-platform/tests

Comment 5

[amarc]

Backed out for causing wr fails @ portrait.tentative.html

• Backout link
• Push with failures
• Failure Log

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fdf6a31b5d9
In WebVTT use min(width, height) for more sensible font-size for portrait videos. r=alwu,emilio
https://hg.mozilla.org/integration/autoland/rev/4785027fdf00
In WebVTT use font-size 9px on the cue container. r=alwu,emilio

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0fdf6a31b5d9
https://hg.mozilla.org/mozilla-central/rev/4785027fdf00

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 10

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1950616 - In WebVTT use min(width, height) for more sensible font-size for portrait videos. r?emilio!, r?alwu!

This matches Chromium's behavior.
Spec issue at https://github.com/w3c/webvtt/issues/529

Original Revision: https://phabricator.services.mozilla.com/D239850

Comment 11

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1950616 - In WebVTT use font-size 9px on the cue container. r?emilio!, r?alwu!

A larger font-size on the cue container prevents a site properly styling ::cue
pseudo elements to anything smaller than that font-size since the container's
line-height cannot be overriden. Per the spec issue we propose 0, but 9px is
small enough that sites won't have a need to style cues to something
significantly smaller, while still keeping WPT passing.

The spec issue is https://github.com/w3c/webvtt/issues/530

Original Revision: https://phabricator.services.mozilla.com/D239851

Comment 12

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Sites cannot rely on WebVTT for displaying subtitles on portrait videos. The more narrow the video, the larger the font. The worst is, when sites reduce the font-size through the only styling option available, the line-height is unaffected. I have word that a fairly large service in Sweden would start to recommend ESR users another browser if this isn't fixed when they roll out WebVTT support "this spring".
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: Low
• Explanation of risk level: Minimal code changes, good test coverage
• String changes made/needed: None
• Is Android affected?: yes

Comment 13

[Pascal Chevrel:pascalc]

Andreas, shouldn't this be also uplifted in 137 beta?

Comment 14

[Andreas Pehrson [:pehrsons]]

This service I'm in contact with is looking to ship WebVTT support "this spring" so beta is not of a concern. ESR doesn't migrate to 140 until mid-september IIRC so a bigger concern there. Not a regression either, I think this behavior is ~ a decade old.

Of course we could do a beta uplift if we think other services may already be impacted, but I'm not aware of any. After all, I consider the risk being pretty low.

Comment 15

[Pascal Chevrel:pascalc]

Spring starts next week so 137, will ship in spring season. If we don't need this change of behaviour for 137 but in late spring when 138 is out, we can uplift it for 128.10 or later rather than 128.9. Setting the tracking, flag for uplifting to ESR 128.10 during the 138 release cycle.

Comment 16

[Andreas Pehrson [:pehrsons]]

It sounded more like May but I don't have anything more concrete.

128.10 is good with the extra bake time in case someone relies on the old behaviour, thanks.