Closed Bug 1557185 Opened 5 months ago Closed 4 months ago

[webvtt] enable wpt 'too_many_cues.html'.

Categories

(Core :: Audio/Video: Playback, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

There are several things we have to do in order to enable this test,

  1. do not use <br> in reference files
  2. eliminate text's length in order to avoid line break, which is not somthing we would like to test in this test
  3. add line number for each cue in order to easily know how many cue would be displayed in the screen

We might enable too_many_cues_wrapped.html in this bug as well.

Depends on: 1556079
No longer depends on: 1556581

As the line box's height is computed by the font size and the line-height (default is 1.2x), so the actual height is around 11px (9*1.2).

The video's height is 180px, which means we can put at most 16 lines within video's rendering area.

Therefore, for too_many_cues.vtt, triming the showing cues number to 16. For too_many_cues_wrapped.vtt, triming the total cues number to 16.

<br> has 1 app unit width, which would push text 1 app unit left and cause the positioning difference.

We should use <div> to enforce them break line without having extra space in the line box.

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task

too_many_cues.vtt would fail on Windows because of different line box height. I found that the line box's height varies by different platforms and is also depending on different system languages, because those setting might have different default font.

While I was running the test on different situation, I saw different line box's height. On my local Windows 10, the height was 10.6; on Windows on tty server, the height was 11.6; on my local Linux with English as default language, the height was 11.08; setting language to Chinese, the height was 13.

That makes me hard to know how many line I should show in the reference file....

Attachment #9070388 - Attachment description: Bug 1557185 - part1 : reduce cues number. → Bug 1557185 - part1 : use recommended font size for Ahem.
Attachment #9070389 - Attachment description: Bug 1557185 - part2 : do not use <br> to generate line break in reference files → Bug 1557185 - part2 : correct CSS style in reference files in order to show cue correctly.
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/69150f666319
part1 : use recommended font size for Ahem. r=heycam
https://hg.mozilla.org/integration/autoland/rev/6ead08887181
part2 : correct CSS style in reference files in order to show cue correctly. r=heycam
https://hg.mozilla.org/integration/autoland/rev/5607d2f3ef7d
part3 : enable wpt 'too_many_cues.html' and 'too_many_cues_wrapped.html'. r=heycam
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/367d45a1ef48
part3 : enable wpt 'too_many_cues.html' and 'too_many_cues_wrapped.html'. r=heycam
Flags: needinfo?(alwu)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17702 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/17702
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/Wn9BaoVdT2GYp1dXWDxFsQ)

Hi, James,

This failure upstream check is due to lacking of the fuzzy annotation on upstream, could you help me manually merge this change?

INFO Found 4800 pixels different, maximum difference per channel 2
TEST-UNEXPECTED-FAIL | /webvtt/rendering/cues-with-video/processing-model/too_many_cues_wrapped.html

Thank you.

Flags: needinfo?(james)

Done! Thanks for the clear explaination.

Flags: needinfo?(james)
You need to log in before you can comment on or make changes to this bug.