Skip to content

Commit

Permalink
fix: WebVTT line not correctly positioned in UITextDisplayer (#4567) (#…
Browse files Browse the repository at this point in the history
…4682)

As per spectification,
[here](https://w3c.github.io/webvtt/#webvtt-line-cue-setting) and
[there](https://w3c.github.io/webvtt/#webvtt-cue-line-alignment), my
understanding is that a cue line with an optional alignment position
`end`, e.g. `line:91%,end`, should have its bottom edge at 91% from the
top of the screen.

But the current code places it at 91% from the bottom of the screen.

Fixes #4567

Co-authored-by: Rodolphe Breton <[email protected]>
Co-authored-by: Joey Parrish <[email protected]>
  • Loading branch information
3 people committed Dec 8, 2022
1 parent dff6683 commit fecf044
Show file tree
Hide file tree
Showing 29 changed files with 22 additions and 5 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Philo Inc. <*@philo.com>
Prakash <[email protected]>
Robert Colantuoni <[email protected]>
Robert Galluccio <[email protected]>
Rodolphe Breton <[email protected]>
Roi Lipman <[email protected]>
Roksolana Ivanyshyn <[email protected]>
Rostislav Hejduk <[email protected]>
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Peter Nycander <[email protected]>
Prakash Duggaraju <[email protected]>
Robert Colantuoni <[email protected]>
Robert Galluccio <[email protected]>
Rodolphe Breton <[email protected]>
Rohit Makasana <[email protected]>
Roi Lipman <[email protected]>
Roksolana Ivanyshyn <[email protected]>
Expand Down
10 changes: 5 additions & 5 deletions lib/text/ui_text_displayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ shaka.text.UITextDisplayer = class {
// (for vertical growing left) is aligned at the line.
// TODO: Implement line alignment with line number.
// TODO: Implement lineAlignment of 'CENTER'.
if (cue.line) {
if (cue.line != null) {
if (cue.lineInterpretation == Cue.lineInterpretation.PERCENTAGE) {
// When setting absolute positioning, you need to set x/y/width/height
// so the element is positioned correctly. Set these as default and
Expand All @@ -596,21 +596,21 @@ shaka.text.UITextDisplayer = class {
if (cue.lineAlign == Cue.lineAlign.START) {
style.top = cue.line + '%';
} else if (cue.lineAlign == Cue.lineAlign.END) {
style.bottom = cue.line + '%';
style.bottom = (100 - cue.line) + '%';
}
} else if (cue.writingMode == Cue.writingMode.VERTICAL_LEFT_TO_RIGHT) {
style.height = '100%';
if (cue.lineAlign == Cue.lineAlign.START) {
style.left = cue.line + '%';
} else if (cue.lineAlign == Cue.lineAlign.END) {
style.right = cue.line + '%';
style.right = (100 - cue.line) + '%';
}
} else {
style.height = '100%';
if (cue.lineAlign == Cue.lineAlign.START) {
style.right = cue.line + '%';
} else if (cue.lineAlign == Cue.lineAlign.END) {
style.left = cue.line + '%';
style.left = (100 - cue.line) + '%';
}
}
}
Expand All @@ -620,7 +620,7 @@ shaka.text.UITextDisplayer = class {

// The position defines the indent of the text container in the
// direction defined by the writing direction.
if (cue.position) {
if (cue.position != null) {
if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) {
style.paddingLeft = cue.position;
} else {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions test/test/assets/screenshots/review.html
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
'deeply-nested-cues',
'end-time-edge-case',
'region-with-display-alignment',
'line-alignment',
];

const prefixes = [
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 14 additions & 0 deletions test/ui/text_displayer_layout_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,20 @@ filterDescribe('TextDisplayer layout', supportsScreenshots, () => {

await checkScreenshot(prefix, 'deeply-nested-cues');
});

// Regression test for #4567, in which "end" line alignment for VTT was
// inverted.
it('line alignment', async () => {
const cue = new shaka.text.Cue(0, 1, 'Captain\'s log, stardate 41636.9');

cue.line = 70;
cue.lineInterpretation = shaka.text.Cue.lineInterpretation.PERCENTAGE;
cue.lineAlign = shaka.text.Cue.lineAlign.END;

textDisplayer.append([cue]);

await checkScreenshot(prefix, 'line-alignment');
});
}

/** @param {!HTMLElement} element */
Expand Down

0 comments on commit fecf044

Please sign in to comment.