Skip to content

Commit

Permalink
fix(text): Don't use flex layout in subtitles.
Browse files Browse the repository at this point in the history
Using flex layout causes problems with the positioning of elements and
causes the borders to be wrapped over the whole parent.  This changes
to use block/inline layouts instead.

This also removes any padding around the cue lines, hugging the
background around the text.  This is a bit more consistent with other
renderers by having a gap between lines.  This also changes a bit of
the nested cues padding.

The deeply nested cue test is different because the `background-color`
CSS attribute isn't inherited, so the default in our CSS sets the
background to black.

Fixes shaka-project#3013

Change-Id: I3e4b63b1b4de1f12e69fce2460d142e0a69bfcd9
  • Loading branch information
TheModMaker committed Mar 17, 2021
1 parent f23ee0a commit 9c2315e
Show file tree
Hide file tree
Showing 113 changed files with 37 additions and 50 deletions.
79 changes: 33 additions & 46 deletions lib/text/ui_text_displayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,25 @@ shaka.text.UITextDisplayer = class {
* @private
*/
displayCue_(container, cue, isNested) {
let type = isNested ? 'span' : 'div';
if (cue.lineBreak || cue.spacer) {
if (cue.spacer) {
shaka.Deprecate.deprecateFeature(4,
'shaka.extern.Cue',
'Please use lineBreak instead of spacer.');
}
type = 'br';
}

// Nested cues are inline elements. Top-level cues are block elements.
const cueElement = shaka.util.Dom.createHTMLElement(
isNested ? 'span' : 'div');
const cueElement = shaka.util.Dom.createHTMLElement(type);

this.setCaptionStyles_(cueElement, cue, isNested);
if (type != 'br') {
this.setCaptionStyles_(cueElement, cue, isNested);

for (const nestedCue of cue.nestedCues) {
this.displayCue_(cueElement, nestedCue, /* isNested= */ true);
for (const nestedCue of cue.nestedCues) {
this.displayCue_(cueElement, nestedCue, /* isNested= */ true);
}
}

container.appendChild(cueElement);
Expand All @@ -240,26 +251,9 @@ shaka.text.UITextDisplayer = class {
*/
setCaptionStyles_(cueElement, cue, isNested) {
const Cue = shaka.text.Cue;
const style = cueElement.style;
let style = cueElement.style;
const isLeaf = cue.nestedCues.length == 0;

if (cue.lineBreak || cue.spacer) {
if (cue.spacer) {
shaka.Deprecate.deprecateFeature(4,
'shaka.extern.Cue',
'Please use lineBreak instead of spacer.');
}
// This takes up a whole line on its own, but that line is 0-height,
// making it effectively a line-break.
style.flexBasis = '100%';
style.height = '0';
// TODO: support multiple line breaks in a row, in which case second and
// up need to take up vertical space.

// Line breaks have no other styles applied.
return;
}

// TODO: wrapLine is not yet supported. Lines always wrap.

// White space should be preserved if emitted by the text parser. It's the
Expand All @@ -273,10 +267,20 @@ shaka.text.UITextDisplayer = class {
// still has not implemented break-spaces, and the original Chromecast will
// never have this feature since it no longer gets firmware updates.
// So we need to replace trailing spaces with non-breaking spaces.
cueElement.textContent = cue.payload.replace(/\s+$/g, (match) => {
const text = cue.payload.replace(/\s+$/g, (match) => {
const nonBreakingSpace = '\xa0';
return nonBreakingSpace.repeat(match.length);
});
if (isNested) {
cueElement.textContent = text;
} else if (text.length) {
// If a top-level cue has text, move to a <span> so the background is
// styled correctly.
const span = shaka.util.Dom.createHTMLElement('span');
span.textContent = text;
cueElement.appendChild(span);
style = span.style;
}

style.backgroundColor = cue.backgroundColor;
style.border = cue.border;
Expand All @@ -302,36 +306,19 @@ shaka.text.UITextDisplayer = class {
}
}

// The displayAlign attribute specifys the vertical alignment of the
// The displayAlign attribute specifies the vertical alignment of the
// captions inside the text container. Before means at the top of the
// text container, and after means at the bottom.
if (cue.displayAlign == Cue.displayAlign.BEFORE) {
style.justifyContent = 'flex-start';
style.verticalAlign = 'top';
} else if (cue.displayAlign == Cue.displayAlign.CENTER) {
style.justifyContent = 'center';
style.verticalAlign = 'middle';
} else {
style.justifyContent = 'flex-end';
style.verticalAlign = 'bottom';
}

if (isLeaf) {
style.display = 'inline-block';
} else {
style.display = 'flex';
style.flexDirection = 'row';
style.flexWrap = 'wrap';
if (!isLeaf) {
style.margin = '0';
// Setting flexDirection to "row" switches the meanings of align and
// justify. Now align is vertical and justify is horizontal. See
// comments above on vertical alignment for displayAlign.
style.alignItems = style.justifyContent;
style.justifyContent = 'center';
}

if (isNested) {
// Work around an IE 11 flexbox bug in which center-aligned items can
// overflow their container. See
// https://github.com/philipwalton/flexbugs/tree/6e720da8#flexbug-2
style.maxWidth = '100%';
}

style.fontFamily = cue.fontFamily;
Expand Down
Binary file modified test/test/assets/screenshots/chrome-Android/ui-basic-cue.png
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.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-cue-with-newline.png
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.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-duplicate-cues.png
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.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-flat-cue-bg.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-nested-cue-bg.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-region-position.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-two-basic-cues.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Android/ui-two-nested-cues.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-basic-cue.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-cue-with-controls.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-cue-with-newline.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-deeply-nested-cues.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-duplicate-cues.png
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.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-flat-cue-bg.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-nested-cue-bg.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-region-position.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-two-basic-cues.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Linux/ui-two-nested-cues.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-basic-cue.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-cue-with-controls.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-cue-with-newline.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-duplicate-cues.png
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-region-position.png
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-two-basic-cues.png
Binary file modified test/test/assets/screenshots/chrome-Mac/ui-two-nested-cues.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-basic-cue.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-cue-with-newline.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-duplicate-cues.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-region-position.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-two-basic-cues.png
Binary file modified test/test/assets/screenshots/chrome-Windows/ui-two-nested-cues.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-basic-cue.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-cue-with-newline.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-duplicate-cues.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-region-position.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-two-basic-cues.png
Binary file modified test/test/assets/screenshots/firefox-Linux/ui-two-nested-cues.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-basic-cue.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-cue-with-controls.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-cue-with-newline.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-deeply-nested-cues.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-duplicate-cues.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-end-time-edge-case.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-region-position.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-two-basic-cues.png
Binary file modified test/test/assets/screenshots/firefox-Mac/ui-two-nested-cues.png
Binary file modified test/test/assets/screenshots/firefox-Windows/ui-basic-cue.png
Binary file modified test/test/assets/screenshots/firefox-Windows/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/firefox-Windows/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/firefox-Windows/ui-region-position.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-basic-cue.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-cue-with-newline.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-duplicate-cues.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-region-position.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-two-basic-cues.png
Binary file modified test/test/assets/screenshots/msedge-Windows/ui-two-nested-cues.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-basic-cue.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-cue-with-controls.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-cue-with-newline.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-deeply-nested-cues.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-duplicate-cues.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-flat-cue-bg.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-nested-cue-bg.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-region-position.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-two-basic-cues.png
Binary file modified test/test/assets/screenshots/safari-Mac/ui-two-nested-cues.png
6 changes: 3 additions & 3 deletions test/text/ui_text_displayer_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('UITextDisplayer', () => {

const textContainer =
videoContainer.querySelector('.shaka-text-container');
const captions = textContainer.querySelector('div');
const captions = textContainer.querySelector('span');
const cssObj = parseCssText(captions.style.cssText);
expect(cssObj).toEqual(
jasmine.objectContaining({
Expand Down Expand Up @@ -175,7 +175,7 @@ describe('UITextDisplayer', () => {

const textContainer =
videoContainer.querySelector('.shaka-text-container');
const captions = textContainer.querySelector('div');
const captions = textContainer.querySelector('span');
const cssObj = parseCssText(captions.style.cssText);
expect(cssObj).toEqual(
jasmine.objectContaining({
Expand Down Expand Up @@ -205,7 +205,7 @@ describe('UITextDisplayer', () => {

const textContainer =
videoContainer.querySelector('.shaka-text-container');
const captions = textContainer.querySelector('div');
const captions = textContainer.querySelector('span');
const cssObj = parseCssText(captions.style.cssText);
expect(cssObj).toEqual(
jasmine.objectContaining({'font-size': expectedFontSize}));
Expand Down
2 changes: 1 addition & 1 deletion ui/less/containers.less
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@
transition-delay: 500ms;

/* These are defaults which are overridden by JS or cue styles. */
div {
span {
font-size: 20px;
line-height: 1.4; // relative to font size.
background-color: rgba(0, 0, 0, 0.8);
Expand Down

0 comments on commit 9c2315e

Please sign in to comment.