Closed Bug 1557882 Opened 1 year ago Closed 1 year ago

[webvtt] the cue text with different base direction should align to different side of the cue box

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- unaffected
firefox67.0.1 --- unaffected
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Depends on 1 open bug, Blocks 1 open bug, Regression, )

Details

(Keywords: regression)

Attachments

(2 files)

STR.

  1. go to https://alastor0325.github.io/htmltests/webvtt/different-base-direction.html

Expected.
2. Hello! is align to the left side of the video, and שלום! is align to the right side of the video.

Actual.
2. both line are aligned to the left side of the video.

Note, this issue didn't happen on Firefox Release 67, but I can reproduce it on the latest Nightly

Hi, Jonathan,

I heard that you might be the right persion to consult for this issue.

In our VTT implementaion, I created a div element and appended a text node with Hello! (line-break) שלום! on it. Because of line-break, the layout would generate two line boxes for each line. One line for Hello! and another line for שלום!. I found that the second line, which contains RTL texts didn't align to the correct position, as I set the alignment to start, it should be aligned to the right side of the box.
(In addition, here is the frame tree I dumped from --layoutdebug [1]).

For the comparison, I created a HTML markup [2] to simulate the same situation, and I found that this reference file can work well as I expect on Chrome, but not on Firefox. Therefore, I wonder if it's a layout bug which didn't set the writing direction well when generate the new line box.

Thank you.

[1] https://pastebin.com/4fyLRD9G
[2] https://alastor0325.github.io/htmltests/webvtt/different-base-direction-ref.html

Flags: needinfo?(jfkthame)

Yes, it looks like this is a bug in how unicode-bidi:plaintext behaves. I made a reduced testcase at https://codepen.io/anon/pen/RzwQPd. Note that here, the bug occurs on the first of the three examples, but not on the other two.

The issue only occurs on the initial reflow of the document; if something triggers a new reflow (e.g. try zooming the page in the browser with Ctrl-+, or use the style editor to tweak the width property of the enclosing div), then the Hebrew line jumps to its expected position.

A key factor appears to be the fact that the forced line-break is inside a <span> rather than at the top level of the block that's using unicode-bidi:plaintext. Without that span, or with the <br> lifted out of it (as in my second and third examples) I was unable to reproduce the problem.

So the "regression" probably occurred because the previous styling didn't depend on unicode-bidi:plaintext in the same way, or else something about the previous CSS/JS was causing an additional reflow that effectively masked the bug.

There's clearly a core layout bug here, so we should fix that; but in the meantime (as it may not be an immediate fix -- bidi resolution is a complex web of code!) you may want to consider some kind of workaround.

As an interim fix, you can probably work around this (at some minor perf cost) by doing something that triggers an extra reflow of the .video element. For example, if I toggle the unicode-bidi property to initial and then back to plaintext after the document loads, this fixes the bad display: https://codepen.io/anon/pen/agbYbM.

Flags: needinfo?(jfkthame)
Depends on: 1558431

I filed bug 1558431 for the core layout issue here, rather than morphing this bug to that component, as I figure you may want to take a workaround in the front end here while waiting for the real fix.

Because of bug1558431, now the text alignment for bidi won't take effect on the initial reflow of the document, so we have to delay setting 'unicode-bidi' in order to trigger reflow again.

It would make the bidi text showing correctly after reflow.

Add a new wpt 'start_alignment.html' to ensure that two cues would have different text alignment.

In addition, add line:0 in vtt file to put cue on the top in order to reduce the complexity of using CSS to markup the test, because if we don't specifiy the postion for cue, those two cues won't be put the bottom of the video, instead they would be move upward one line above the bottom according to the spec.

The reason is that the base moving unit for adjusting cue box's position is the Bsize of the first line box, it would require extra moving if one cue contains two different cues which line boxes are not the same height.

Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50efff6cae91
part1 : delay setting 'unicode-bidi' until finishing the position calculation. r=heycam
https://hg.mozilla.org/integration/autoland/rev/65393203d1c5
part2 : add wpt 'start_alignment.html'. r=heycam
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17324 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Does the user impact of this bug warrant uplift consideration or can this ride the trains?

Flags: needinfo?(alwu)
Flags: in-testsuite+

As this is a rare case which require VTT file contains two different base direction in one cue, and put them on different lines, I think we can just let the fix riding the train.

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