Skip to content

Commit

Permalink
REGRESSION(265596@main) Vertical WebVTT cues are laid out horizontally
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258980
rdar://111909315

Reviewed by Eric Carlson.

Use the calculated value of `writing-mode` in applyCSSProperties() rather
than the default provided in the UA style sheet.

* LayoutTests/media/track/track-cue-vertical-style-expected.txt: Added.
* LayoutTests/media/track/track-cue-vertical-style.html: Added.
* Source/WebCore/html/track/VTTCue.cpp:
(WebCore::VTTCueBox::applyCSSProperties):

Canonical link: https://commits.webkit.org/265860@main
  • Loading branch information
jernoble committed Jul 7, 2023
1 parent cb37d35 commit c4d1167
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 1 deletion.
13 changes: 13 additions & 0 deletions LayoutTests/media/track/track-cue-vertical-style-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

RUN(video.src = findMediaFile("video", "../content/test"))
EVENT(canplay)
RUN(cue = new VTTCue(0, 1, "vertical"))
RUN(textTrack.addCue(cue))
EXPECTED (firstCueElement() != 'null') OK
EXPECTED (window.getComputedStyle(firstCueElement()).writingMode == 'horizontal-tb') OK
RUN(cue.vertical = "rl")
EXPECTED (window.getComputedStyle(firstCueElement()).writingMode == 'vertical-rl') OK
RUN(cue.vertical = "lr")
EXPECTED (window.getComputedStyle(firstCueElement()).writingMode == 'vertical-lr') OK
END OF TEST

51 changes: 51 additions & 0 deletions LayoutTests/media/track/track-cue-vertical-style.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>track-cue-vertical-style</title>
<script src=../media-file.js></script>
<script src=../video-test.js></script>
<script>
function firstCueElement() {
return internals.shadowRoot(video).querySelector('[pseudo="cue"]');
}

async function runTest() {
findMediaElement();
run('video.src = findMediaFile("video", "../content/test")');
await waitFor(video, 'canplay');

window.textTrack = video.addTextTrack("subtitles");
textTrack.mode = 'showing';

run('cue = new VTTCue(0, 1, "vertical")');
run('textTrack.addCue(cue)');

if (!window.testRunner) {
consoleWrite('Test requires internals; manually verify <code>vertical</code> cue is vertical.');
endTest();
}

await testExpectedEventually('firstCueElement()', null, '!=');

await testExpectedEventually(`window.getComputedStyle(firstCueElement()).writingMode`, 'horizontal-tb')

run('cue.vertical = "rl"');

await testExpectedEventually(`window.getComputedStyle(firstCueElement()).writingMode`, 'vertical-rl')

run('cue.vertical = "lr"');

await testExpectedEventually(`window.getComputedStyle(firstCueElement()).writingMode`, 'vertical-lr')
}

window.addEventListener('load', event => {
runTest().then(endTest).catch(failTest);
});
</script>
</head>
<body>
<video></video>
</body>
</html>
4 changes: 3 additions & 1 deletion Source/WebCore/html/track/VTTCue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ void VTTCueBox::applyCSSProperties()

// the 'position' property must be set to 'absolute'
// the 'unicode-bidi' property must be set to 'plaintext'
// the 'writing-mode' property must be set to writing-mode
// the overflow-wrap property must be set to break-word
// the text-wrap property must be set to balance [CSS-TEXT-4]
// The color property on the (root) list of WebVTT Node Objects must be set
Expand All @@ -187,6 +186,9 @@ void VTTCueBox::applyCSSProperties()
// Use 'cqh' and 'cqw' rather than 'vh' and 'vw' here, as the video viewport
// is not a true viewport, but it is a container, so they serve the same purpose.

// the 'writing-mode' property must be set to writing-mode
setInlineStyleProperty(CSSPropertyWritingMode, cue->getCSSWritingMode(), false);

// the 'top' property must be set to top
std::visit(WTF::makeVisitor([&] (double top) {
setInlineStyleProperty(CSSPropertyTop, top, CSSUnitType::CSS_CQH);
Expand Down

0 comments on commit c4d1167

Please sign in to comment.