1534888 - [webvtt] Enable wpt '2_cues_overlapping_completely_move_up.html' and '2_cues_overlapping_partially_move_up.html'

Status: RESOLVED FIXED

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

Description

[Alastor Wu [:alwu]]

After landing bug1488673 and modify the wrong part of these wpt, we could enable them.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1534888 - part1 : modify wpt '2_cues_overlapping_completely_move_up.html' and '2_cues_overlapping_partially_move_up.html'.

Bug 1534888 - enable wpt '2_cues_overlapping_completely_move_up.html' and '2_cues_overlapping_partially_move_up.html'.

According to the spec [1], the line alignment of the cue is start by default, which means the top side of the box should align to the position.

In this case, the cues in 2_cues_overlapping_completely_move_up.vtt and 2_cues_overlapping_partially_move_up.html are set to line position 50% which means the top side of the first cue box should be align to the 50% of the height of video.

The second cue are set to 50% and 49% in two files, which would overlap with the first cue. According to the spec [2], we should keep their relative position, and move the box to the place where no boxes are overlapped. Therefore, we should move the second cue up from the top side of the first cue by the font size (9px).

Therefore, in the reference file, the first cue should not have the margin-top offset, and the second cue should have margin-top=9px.

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

Comment 2

[Alastor Wu [:alwu]]

Here I have some questions, in the wpt test, we usually use font-family to define multiple fonts [1], which are Ahem and sans-serif.

However, when I runs the tests, sometime I would see the the sans-serif applying on the text, and sometime I would see the Ahem applyiing on the text. Why we would have such difference? What causes this differences?

In addition, when we use the sans-serif, the offsetHeight of div and cueDiv[2] which are in the CueStyleBox would be the same. But they were different when we apply Ahem on the text....

[1] https://searchfox.org/mozilla-central/rev/89414a1df52d06cfc35529afb9a5a8542a6e4270/testing/web-platform/tests/webvtt/rendering/cues-with-video/processing-model/2_cues_overlapping_completely_move_up.html#9

[2] https://searchfox.org/mozilla-central/rev/89414a1df52d06cfc35529afb9a5a8542a6e4270/dom/media/webvtt/vtt.jsm#517-519

---

Hi, Cameron,

Do you have any idea about these problems?

Thank you!

Comment 3

[Alastor Wu [:alwu]]

We've known that the inner box's size is not always equal to the outer box's size, and we now use the first line box's size to adjust cue's position after landing bug1536762. So no matter what fonts we're using, we are able to find the correct bounding box size to adjust cues' position.

Comment 4

[Alastor Wu [:alwu]]

For 2_cues_overlapping_partially_move_up.html, now I have to find a way to extend the distance between first cue and second cue, because the spece between them should be 1.8px (1% of the video height) wider than the normal distance we have between lines.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1534888 - part2 : move the cue upward 1% of video's height.

In 2_cues_overlapping_partially_move_up.vtt, the second cue was put on the 99% place where the second cue is 1% higher than the first cue, and then we would move the second cue up by a line box height in order to prevent those two cues overlapping.

Therefore, in the reference files, the second cue should be 1%(of 180px) higher than its original place.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1534888 - part3 : enable wpt '2_cues_overlapping_completely_move_up.html' and '2_cues_overlapping_partially_move_up.html'.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5242587b6db4
part1 : modify wpt '2_cues_overlapping_completely_move_up.html' and '2_cues_overlapping_partially_move_up.html'. r=heycam
https://hg.mozilla.org/integration/autoland/rev/c789b732dde8
part2 : move the cue upward 1% of video's height. r=heycam
https://hg.mozilla.org/integration/autoland/rev/56997ade68ef
part3 : enable wpt '2_cues_overlapping_completely_move_up.html' and '2_cues_overlapping_partially_move_up.html'. r=heycam

Comment 8

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

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

Comment 9

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

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 10

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

Upstream PR merged

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5242587b6db4
https://hg.mozilla.org/mozilla-central/rev/c789b732dde8
https://hg.mozilla.org/mozilla-central/rev/56997ade68ef