1488673 - [webvtt] HTML5 Video track/subtitles do not scale when using css transform scale on parent elements

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Michael Kling]

Attachment: Example HTML with Video and Subtitle to reproduce the issue

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36

Steps to reproduce:

A video with track (subtitles) is placed in an html element which is transformed via css transform:scale.





Actual results:

The subtitles do scale in size, but their position is off. For scale < 1 the subtitles are getting into the video, for scale > 1 subtitles are even displayed outside of the video. 


Expected results:

The subtitles do scale together with the video and stay at the same relative position.

Comment 1

[Michael Kling]

Attachment: 2018-09-05 09_36_19-Video Subtitle Test.png

Picture showing the issue.

Comment 2

[Karl Tomlinson (:karlt)]

It looks like the transform is being applied twice.

Comment 4

[Nevo Kaw]

Is there an update on this bug? Will it be fixed anytime soon?

Comment 5

[Alastor Wu [:alwu]]

I can reproduce this issue on Nightly 67.0a1 (2019-02-06), OSX 10.13.5.

Comment 6

[Alastor Wu [:alwu]]

I'm going to take a look for this bug, but it might take some time.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part4 : adjust cue's position.

Follow the spec 7.2.10 to adjust cue's position.

Comment 8

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part6 : remove unused functions.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part2 : keep font size consistent even if applying CSS transform scale on the parent element

When calculating font size, we should use the original bounding box as a reference, or it would cause incorrect font size scale.

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part7 : huge replacement of 'var' by 'let'.

In order to keep the local variable only visible inside the function scope, we should only use 'let' for those variables.

Comment 11

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part8 : remove unnecessary anonymous function.

Comment 12

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part9 : disable vtt wpt rendering tests.

These wpt tests are using the wrong ref html which always assume the cue should be on the bottom of the video element.

However, according to the spec, the result of cue's position is calulated by rendering area and it won't always be on the bottom of the video.

Comment 13

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part9 : enable wpt 'line_1_wrapped_cue_grow_downwards.html'.

Comment 14

[Alastor Wu [:alwu]]

It's anoying that the result of wpt reftest is not very stable, last time when I pushed my patches to try server, some certain tests would only pass on Windows so that I had to disable them on other platforms.

However, I pushed my patches to try server again today, those tests are magically pass...

Comment 15

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part1 : always use non-transformed attributes in 'getSimpleBoxPosition()'.

Bounding box would be affected by CSS transformation, using its width and height would result in calculating the incorrect position.

Therefore, we should always use non-transformed attributes for its width, height, top and left, and then we can use those properties to get the correct right and bottom.

Comment 16

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part5 : find the best position for the cue box in order not to overlap it with other cue boxes.

According to the spec 7.2.10 [1], the step13 to step21 (snap-to-line is true) and the step3 to step5 (snap-to-line is false), we need to find the best position where the cue box has least overlap with other cue boxes.

In addition, in order not to be affected by CSS transformation, use non-tranformed attribute in BoxPosition.

[1]
https://w3c.github.io/webvtt/#ref-for-webvtt-cue-snap-to-lines-flag-12
https://w3c.github.io/webvtt/#ref-for-webvtt-cue-snap-to-lines-flag-13

Comment 17

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part1 : use correct size to calculate position.

According to the spec [1], we have calculated the size in step 7.3 and then we should use that for calculating x-position and y-position, instead of using cue's size.

[1] https://w3c.github.io/webvtt/#apply-webvtt-cue-settings

Comment 18

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part3 : only use BoxPosition format for position calculation

It's confused that we have both simpleBoxPosition object and BoxPosition object, we should only use one format to perform all box related operations.

Therefore, BoxPosition should be able to be initiaized by StyleBox, HTMLElement or BoxPosition.

In addition, as right and bottom can be calculated from other attributes, we remove these two attributes from BoxPosition, and use getter to get the correct value, which can reduce some unnessary modification when we changes the height or width.

Comment 19

[Alastor Wu [:alwu]]

Attachment: Bug 1488673 - part10 : enable wpt tests.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ca1266e7fbad
part1 : use correct size to calculate position. r=heycam
https://hg.mozilla.org/integration/autoland/rev/9206cafa8778
part2 : keep font size consistent even if applying CSS transform scale on the parent element r=heycam
https://hg.mozilla.org/integration/autoland/rev/e5030bf46846
part3 : only use BoxPosition format for position calculation r=heycam
https://hg.mozilla.org/integration/autoland/rev/d18858a1fe8b
part4 : adjust cue's position. r=heycam
https://hg.mozilla.org/integration/autoland/rev/9fa072a4b67d
part5 : find the best position for the cue box in order not to overlap it with other cue boxes. r=heycam
https://hg.mozilla.org/integration/autoland/rev/5b3ae7eb7842
part6 : remove unused functions. r=heycam
https://hg.mozilla.org/integration/autoland/rev/f264bc709b32
part7 : huge replacement of 'var' by 'let'. r=heycam
https://hg.mozilla.org/integration/autoland/rev/b7cea852fc6b
part8 : remove unnecessary anonymous function. r=heycam
https://hg.mozilla.org/integration/autoland/rev/faa494b301b9
part9 : disable vtt wpt rendering tests. r=heycam
https://hg.mozilla.org/integration/autoland/rev/58bd4444d2c5
part10 : enable wpt tests. r=heycam

Comment 21

[Stefan Hindli [:stefan_hindli]]

Backed out 10 changesets (Bug 1488673) for permafailing wpt tests in /html/semantics/embedded-content/media-elements/track/track-element/track-cue-rendering-line-doesnt-fit.html CLOSED TREE

Failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=os%2Cx%2C10.10%2Cdebug%2Cweb%2Cplatform%2Ctests%2Cwith%2Ce10s%2Ctest-macosx64%2Fdebug-web-platform-tests-reftests-e10s-3%2Cw-e10s%28wr3%29&selectedJob=234486694

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=234486694&repo=autoland&lineNumber=21537

09:14:19 INFO - TEST-UNEXPECTED-PASS | /html/semantics/embedded-content/media-elements/track/track-element/track-cue-rendering-line-doesnt-fit.html | Testing http://web-platform.test:8000/html/semantics/embedded-content/media-elements/track/track-element/track-cue-rendering-line-doesnt-fit.html == http://web-platform.test:8000/html/semantics/embedded-content/media-elements/track/track-element/track-cue-rendering-line-doesnt-fit-ref.html

Comment 22

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e3d01a1b848
Backed out 10 changesets for permafailing wpt tests in  /html/semantics/embedded-content/media-elements/track/track-element/track-cue-rendering-line-doesnt-fit.html CLOSED TREE

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Can we fix the WPT tests instead of disabling them pretty please?

Comment 24

[Alastor Wu [:alwu]]

Can we fix the WPT tests instead of disabling them pretty please?

These patches are the basic alogirithm which is used to render cue, so I don't want those fail tests blocking it. In addition, this module hasn't been maintained for a while, so there are still some other problems need to be fixed. I've filed another bug1534862, which is used to investigate the fail reason of each wpt test and then enable them.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93cb5631f24b
part1 : use correct size to calculate position. r=heycam
https://hg.mozilla.org/integration/autoland/rev/367a2a68fb2d
part2 : keep font size consistent even if applying CSS transform scale on the parent element r=heycam
https://hg.mozilla.org/integration/autoland/rev/e89cf185fc33
part3 : only use BoxPosition format for position calculation r=heycam
https://hg.mozilla.org/integration/autoland/rev/9e2f80f5ec79
part4 : adjust cue's position. r=heycam
https://hg.mozilla.org/integration/autoland/rev/a1841a4aa3f1
part5 : find the best position for the cue box in order not to overlap it with other cue boxes. r=heycam
https://hg.mozilla.org/integration/autoland/rev/3f6a9b725f21
part6 : remove unused functions. r=heycam
https://hg.mozilla.org/integration/autoland/rev/5934ef641056
part7 : huge replacement of 'var' by 'let'. r=heycam
https://hg.mozilla.org/integration/autoland/rev/c2aad16bf544
part8 : remove unnecessary anonymous function. r=heycam
https://hg.mozilla.org/integration/autoland/rev/2a0158d3e47c
part9 : disable vtt wpt rendering tests. r=heycam
https://hg.mozilla.org/integration/autoland/rev/7626a0df2e26
part10 : enable wpt tests. r=heycam

Comment 26

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/93cb5631f24b
https://hg.mozilla.org/mozilla-central/rev/367a2a68fb2d
https://hg.mozilla.org/mozilla-central/rev/e89cf185fc33
https://hg.mozilla.org/mozilla-central/rev/9e2f80f5ec79
https://hg.mozilla.org/mozilla-central/rev/a1841a4aa3f1
https://hg.mozilla.org/mozilla-central/rev/3f6a9b725f21
https://hg.mozilla.org/mozilla-central/rev/5934ef641056
https://hg.mozilla.org/mozilla-central/rev/c2aad16bf544
https://hg.mozilla.org/mozilla-central/rev/2a0158d3e47c
https://hg.mozilla.org/mozilla-central/rev/7626a0df2e26

Comment 29

[James Graham [:jgraham]]

FTR even when we can't fix wpt tests right away — which itself should be considered a problem — we should never mark them as disabled, but set the expectation to FAIL, unless the test is causing instability in CI.

Comment 30

[Alastor Wu [:alwu]]

For those tests which were disabled, they were sometime unexpected passed when I pushed them on try server.
Now I'm investigating all intermittent fail test and those disable tests, and will reopen them later.

Comment 31

[Intermittent Failures Robot]

1 failures in 3025 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-beta: 1

Platform breakdown:

• windows10-64-nightly: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1488673&startday=2019-04-08&endday=2019-04-14&tree=all