diff --git a/build/updateScreenshots.py b/build/updateScreenshots.py index 39bb45d888..c91e4cf47e 100755 --- a/build/updateScreenshots.py +++ b/build/updateScreenshots.py @@ -38,12 +38,12 @@ def main(args): # Skip hidden files like .gitignore and non-folders continue - # If any platforms were specified, only update those. - if len(args): - if not platform in args: + for child in os.listdir(platformFolder): + # If any args were specified, use them to filter. Either the platform or + # base of the filename must match the filter. + if args and not platform in args and not child.split('.')[0] in args: continue - for child in os.listdir(platformFolder): fullPath = os.path.join(platformFolder, child) # If this has the "-new" suffix, it was just written by the layout tests. # Rename it to overwrite the "official" version, which is stored in diff --git a/externs/shaka/text.js b/externs/shaka/text.js index 6bce28e142..0762898d83 100644 --- a/externs/shaka/text.js +++ b/externs/shaka/text.js @@ -98,7 +98,7 @@ shaka.extern.CueRegion = class { * @type {shaka.text.CueRegion.scrollMode} * @exportDoc */ - shaka.extern.CueRegion.prototype.scroll; + this.scroll; } }; @@ -134,7 +134,8 @@ shaka.extern.Cue = class { this.payload; /** - * The region to render the cue into. + * The region to render the cue into. Only supported on top-level cues, + * because nested cues are inline elements. * @type {shaka.extern.CueRegion} * @exportDoc */ @@ -232,25 +233,22 @@ shaka.extern.Cue = class { this.displayAlign; /** - * Text color represented by any string that would be accepted in CSS. - * E. g. '#FFFFFF' or 'white'. + * Text color as a CSS color, e.g. "#FFFFFF" or "white". * @type {!string} * @exportDoc */ this.color; /** - * Text background color represented by any string that would be - * accepted in CSS. - * E. g. '#FFFFFF' or 'white'. + * Text background color as a CSS color, e.g. "#FFFFFF" or "white". * @type {!string} * @exportDoc */ this.backgroundColor; /** - * The number of horizontal and vertical cells into which - * the Root Container Region area is divided + * The number of horizontal and vertical cells into which the Root Container + * Region area is divided. * * @type {{ columns: number, rows: number }} * @exportDoc @@ -258,16 +256,14 @@ shaka.extern.Cue = class { this.cellResolution; /** - * Image background represented by any string that would be - * accepted in image HTML element. - * E. g. 'data:[mime type];base64,[data]'. + * The URL of the background image, e.g. "data:[mime type];base64,[data]". * @type {!string} * @exportDoc */ this.backgroundImage; /** - * Text border. + * The border around this cue as a CSS border. * @type {!string} * @exportDoc */ @@ -302,21 +298,21 @@ shaka.extern.Cue = class { this.fontFamily; /** - * Text letter spacing. + * Text letter spacing as a CSS letter-spacing value. * @type {!string} * @exportDoc */ this.letterSpacing; /** - * Text line padding. + * Text line padding as a CSS line-padding value. * @type {!string} * @exportDoc */ this.linePadding; /** - * Text opacity. + * Opacity of the cue element, from 0-1. * @type {!number} * @exportDoc */ @@ -346,7 +342,9 @@ shaka.extern.Cue = class { /** * Nested cues, which should be laid out horizontally in one block. - * @type {Array.} + * Top-level cues are blocks, and nested cues are inline elements. + * Cues can be nested arbitrarily deeply. + * @type {!Array.} * @exportDoc */ this.nestedCues; @@ -358,6 +356,7 @@ shaka.extern.Cue = class { * @exportDoc */ this.spacer; + // TODO: Rename "spacer" to "lineBreak". } }; diff --git a/lib/text/cue.js b/lib/text/cue.js index 692d7a517e..eb166d781b 100644 --- a/lib/text/cue.js +++ b/lib/text/cue.js @@ -38,9 +38,14 @@ shaka.text.Cue = class { */ this.endTime = endTime; + // The explicit type here is to work around an apparent compiler bug where + // the compiler seems to get confused about the type. Since we are + // overriding the interface type, it shouldn't be necessary to do this. + // Compiler version 20200517. /** * @override * @exportInterface + * @type {string} */ this.payload = payload; diff --git a/lib/text/simple_text_displayer.js b/lib/text/simple_text_displayer.js index 54911b581a..2b60527f03 100644 --- a/lib/text/simple_text_displayer.js +++ b/lib/text/simple_text_displayer.js @@ -17,12 +17,7 @@ goog.require('shaka.text.Cue'); /** - * @summary - * This defines the default text displayer plugin. An instance of this - * class is used when no custom displayer is given. - * - * This class simply converts shaka.text.Cue objects to - * TextTrackCues and feeds them to the browser. + * A text displayer plugin using the browser's native VTTCue interface. * * @implements {shaka.extern.TextDisplayer} * @export @@ -84,27 +79,31 @@ shaka.text.SimpleTextDisplayer = class { * @export */ append(cues) { - // Flatten nestedCues. If a cue has nested cues, their contents should be - // combined and replace the payload of the parent. However, we don't want - // to modify the array or objects passed in, since we don't technically own - // them. So we build a new array and replace certain items in it if they - // need to be flattened. + // Flatten nested cue payloads recursively. If a cue has nested cues, + // their contents should be combined and replace the payload of the parent. + const flattenPayload = (cue) => { + // TODO: If we want to support bold, italic, underline here, we would + // insert markup into the payload. + + if (cue.spacer) { + // This is a vertical spacer, so insert a newline. + return '\n'; + } else if (cue.nestedCues.length) { + return cue.nestedCues.map(flattenPayload).join(''); + } else { + // This is a real cue. + return cue.payload; + } + }; + + // We don't want to modify the array or objects passed in, since we don't + // technically own them. So we build a new array and replace certain items + // in it if they need to be flattened. const flattenedCues = cues.map((cue) => { if (cue.nestedCues.length) { - const payload = cue.nestedCues.map((inner) => { - if (inner.spacer) { - // This is a vertical spacer, so insert a newline. - return '\n'; - } else { - // This is a real cue. Add a space after it. Extra spaces at the - // end or before a vertical spacer are removed with a Regexp below. - return inner.payload + ' '; - } - }).join('').replace(/ $/m, ''); - const flatCue = cue.clone(); flatCue.nestedCues = []; - flatCue.payload = payload; + flatCue.payload = flattenPayload(cue); return flatCue; } else { return cue; @@ -211,7 +210,8 @@ shaka.text.SimpleTextDisplayer = class { const Cue = shaka.text.Cue; /** @type {VTTCue} */ - const vttCue = new VTTCue(shakaCue.startTime, + const vttCue = new VTTCue( + shakaCue.startTime, shakaCue.endTime, shakaCue.payload); diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js index d713963e75..af8abb6cf5 100644 --- a/lib/text/ui_text_displayer.js +++ b/lib/text/ui_text_displayer.js @@ -15,6 +15,9 @@ goog.require('shaka.util.Timer'); /** + * The text displayer plugin for the Shaka Player UI. Can also be used directly + * by providing an appropriate container element. + * * @implements {shaka.extern.TextDisplayer} * @final * @export @@ -188,142 +191,151 @@ shaka.text.UITextDisplayer = class { }); for (const cue of currentCues) { - this.displayCue_(this.textContainer_, cue); + const cueElement = this.displayCue_( + this.textContainer_, cue, /* isNested= */ false); + this.currentCuesMap_.set(cue, cueElement); } } /** - * Displays a nested cue + * Displays a cue * * @param {Element} container * @param {!shaka.extern.Cue} cue * @param {boolean} isNested - * @return {!Element} the created captions container + * @return {!Element} the created captions element * @private */ - displayLeafCue_(container, cue, isNested) { - const captions = shaka.util.Dom.createHTMLElement('span'); - if (isNested) { - captions.classList.add('shaka-nested-cue'); - } + displayCue_(container, cue, isNested) { + // Nested cues are inline elements. Top-level cues are block elements. + const cueElement = shaka.util.Dom.createHTMLElement( + isNested ? 'span' : 'div'); - if (cue.spacer) { - captions.style.display = 'block'; - } else { - this.setCaptionStyles_(captions, cue, /* isLeaf= */ true); - } + this.setCaptionStyles_(cueElement, cue, isNested); - container.appendChild(captions); + for (const nestedCue of cue.nestedCues) { + this.displayCue_(cueElement, nestedCue, /* isNested= */ true); + } - return captions; + container.appendChild(cueElement); + return cueElement; } /** - * Displays a cue - * - * @param {Element} container + * @param {!HTMLElement} cueElement * @param {!shaka.extern.Cue} cue + * @param {boolean} isNested * @private */ - displayCue_(container, cue) { - if (cue.nestedCues.length) { - const nestedCuesContainer = shaka.util.Dom.createHTMLElement('p'); - nestedCuesContainer.style.width = '100%'; - this.setCaptionStyles_(nestedCuesContainer, cue, /* isLeaf= */ false); - - for (let i = 0; i < cue.nestedCues.length; i++) { - this.displayLeafCue_( - nestedCuesContainer, cue.nestedCues[i], /* isNested= */ true); - } + setCaptionStyles_(cueElement, cue, isNested) { + const Cue = shaka.text.Cue; + const style = cueElement.style; + const isLeaf = cue.nestedCues.length == 0; - container.appendChild(nestedCuesContainer); - this.currentCuesMap_.set(cue, nestedCuesContainer); - } else { - this.currentCuesMap_.set(cue, - this.displayLeafCue_(container, cue, /* isNested= */ false)); + if (cue.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; } - } - /** - * @param {!HTMLElement} captions - * @param {!shaka.extern.Cue} cue - * @param {boolean} isLeaf - * @private - */ - setCaptionStyles_(captions, cue, isLeaf) { - const Cue = shaka.text.Cue; - const captionsStyle = captions.style; + // TODO: wrapLine is not yet supported. Lines always wrap. + + // White space should be preserved if emitted by the text parser. It's the + // job of the parser to omit any whitespace that should not be displayed. + // Using 'pre-wrap' means that whitespace is preserved even at the end of + // the text, but that lines which overflow can still be broken. + style.whiteSpace = 'pre-wrap'; + + // Using 'break-spaces' would be better, as it would preserve even trailing + // spaces, but that only shipped in Chrome 76. As of July 2020, Safari + // 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 nonBreakingSpace = '\xa0'; + return nonBreakingSpace.repeat(match.length); + }); - // Set white-space to 'pre-line' to enable showing line breaks in the text. - captionsStyle.whiteSpace = 'pre-line'; - captions.textContent = cue.payload; - if (isLeaf) { - captionsStyle.backgroundColor = cue.backgroundColor; - } - captionsStyle.border = cue.border; - captionsStyle.color = cue.color; - captionsStyle.direction = cue.direction; - captionsStyle.opacity = cue.opacity; - captionsStyle.paddingLeft = shaka.text.UITextDisplayer.convertLengthValue_( - cue.linePadding, cue, this.videoContainer_ - ); - captionsStyle.paddingRight = shaka.text.UITextDisplayer.convertLengthValue_( - cue.linePadding, cue, this.videoContainer_ - ); + style.backgroundColor = cue.backgroundColor; + style.border = cue.border; + style.color = cue.color; + style.direction = cue.direction; + style.opacity = cue.opacity; + style.paddingLeft = shaka.text.UITextDisplayer.convertLengthValue_( + cue.linePadding, cue, this.videoContainer_); + style.paddingRight = shaka.text.UITextDisplayer.convertLengthValue_( + cue.linePadding, cue, this.videoContainer_); if (cue.backgroundImage) { - captionsStyle.backgroundImage = 'url(\'' + cue.backgroundImage + '\')'; - captionsStyle.backgroundRepeat = 'no-repeat'; - captionsStyle.backgroundSize = 'contain'; - captionsStyle.backgroundPosition = 'center'; + style.backgroundImage = 'url(\'' + cue.backgroundImage + '\')'; + style.backgroundRepeat = 'no-repeat'; + style.backgroundSize = 'contain'; + style.backgroundPosition = 'center'; + if (cue.backgroundColor == '') { - captionsStyle.backgroundColor = 'transparent'; + // In text-based cues, background color can default in CSS. + // In bitmap-based cues, we default to a transparent background color, + // so that the bitmap can be the only background. + style.backgroundColor = 'transparent'; + } + + if (cue.region) { + // Region settings are used to size bitmap-based subtitles. + // TODO: Revisit this as we update the TTML parser. This is a special + // case that is not intuitive. + const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE; + const heightUnit = + cue.region.heightUnits == percentageUnit ? '%' : 'px'; + const widthUnit = cue.region.widthUnits == percentageUnit ? '%' : 'px'; + style.height = cue.region.height + heightUnit; + style.width = cue.region.width + widthUnit; } - } - if (cue.backgroundImage && cue.region) { - const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE; - const heightUnit = cue.region.heightUnits == percentageUnit ? '%' : 'px'; - const widthUnit = cue.region.widthUnits == percentageUnit ? '%' : 'px'; - captionsStyle.height = cue.region.height + heightUnit; - captionsStyle.width = cue.region.width + widthUnit; } // The displayAlign attribute specifys 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) { - captionsStyle.justifyContent = 'flex-start'; + style.justifyContent = 'flex-start'; } else if (cue.displayAlign == Cue.displayAlign.CENTER) { - captionsStyle.justifyContent = 'center'; + style.justifyContent = 'center'; } else { - captionsStyle.justifyContent = 'flex-end'; + style.justifyContent = 'flex-end'; } - if (cue.nestedCues.length) { - captionsStyle.display = 'flex'; - captionsStyle.flexDirection = 'row'; - captionsStyle.margin = '0'; - // Setting flexDirection to "row" inverts the sense of align and justify. - // Now align is vertical and justify is horizontal. See comments above on - // vertical alignment for displayAlign. - captionsStyle.alignItems = captionsStyle.justifyContent; - captionsStyle.justifyContent = 'center'; + if (isLeaf) { + style.display = 'inline-block'; + } else { + style.display = 'flex'; + style.flexDirection = 'row'; + style.flexWrap = 'wrap'; + 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 (isLeaf) { + 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 - captionsStyle.maxWidth = '100%'; + style.maxWidth = '100%'; } - captionsStyle.fontFamily = cue.fontFamily; - captionsStyle.fontWeight = cue.fontWeight.toString(); - captionsStyle.fontStyle = cue.fontStyle; - captionsStyle.letterSpacing = cue.letterSpacing; - captionsStyle.fontSize = shaka.text.UITextDisplayer.convertLengthValue_( - cue.fontSize, cue, this.videoContainer_ - ); + style.fontFamily = cue.fontFamily; + style.fontWeight = cue.fontWeight.toString(); + style.fontStyle = cue.fontStyle; + style.letterSpacing = cue.letterSpacing; + style.fontSize = shaka.text.UITextDisplayer.convertLengthValue_( + cue.fontSize, cue, this.videoContainer_); // The line attribute defines the positioning of the text container inside // the video container. @@ -344,72 +356,75 @@ shaka.text.UITextDisplayer = class { // TODO: Implement lineAlignment of 'CENTER'. if (cue.line) { if (cue.lineInterpretation == Cue.lineInterpretation.PERCENTAGE) { - captionsStyle.position = 'absolute'; + style.position = 'absolute'; if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { if (cue.lineAlign == Cue.lineAlign.START) { - captionsStyle.top = cue.line + '%'; + style.top = cue.line + '%'; } else if (cue.lineAlign == Cue.lineAlign.END) { - captionsStyle.bottom = cue.line + '%'; + style.bottom = cue.line + '%'; } } else if (cue.writingMode == Cue.writingMode.VERTICAL_LEFT_TO_RIGHT) { if (cue.lineAlign == Cue.lineAlign.START) { - captionsStyle.left = cue.line + '%'; + style.left = cue.line + '%'; } else if (cue.lineAlign == Cue.lineAlign.END) { - captionsStyle.right = cue.line + '%'; + style.right = cue.line + '%'; } } else { if (cue.lineAlign == Cue.lineAlign.START) { - captionsStyle.right = cue.line + '%'; + style.right = cue.line + '%'; } else if (cue.lineAlign == Cue.lineAlign.END) { - captionsStyle.left = cue.line + '%'; + style.left = cue.line + '%'; } } } - } else if (cue.region && cue.region.id && !isLeaf) { + } else if (cue.region && cue.region.id && !isNested && !isLeaf) { + // Regions are only applied to block container (!isNested && !isLeaf). + // TODO: Revisit this as we update the TTML parser. See special case for + // backgroundImage + region above. const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE; const heightUnit = cue.region.heightUnits == percentageUnit ? '%' : 'px'; const widthUnit = cue.region.widthUnits == percentageUnit ? '%' : 'px'; const viewportAnchorUnit = cue.region.viewportAnchorUnits == percentageUnit ? '%' : 'px'; - captionsStyle.height = cue.region.height + heightUnit; - captionsStyle.width = cue.region.width + widthUnit; - captionsStyle.position = 'absolute'; - captionsStyle.top = cue.region.viewportAnchorY + viewportAnchorUnit; - captionsStyle.left = cue.region.viewportAnchorX + viewportAnchorUnit; + style.height = cue.region.height + heightUnit; + style.width = cue.region.width + widthUnit; + style.position = 'absolute'; + style.top = cue.region.viewportAnchorY + viewportAnchorUnit; + style.left = cue.region.viewportAnchorX + viewportAnchorUnit; } - captionsStyle.lineHeight = cue.lineHeight; + style.lineHeight = cue.lineHeight; // The position defines the indent of the text container in the // direction defined by the writing direction. if (cue.position) { if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { - captionsStyle.paddingLeft = cue.position; + style.paddingLeft = cue.position; } else { - captionsStyle.paddingTop = cue.position; + style.paddingTop = cue.position; } } // The positionAlign attribute is an alignment for the text container in // the dimension of the writing direction. if (cue.positionAlign == Cue.positionAlign.LEFT) { - captionsStyle.cssFloat = 'left'; + style.cssFloat = 'left'; } else if (cue.positionAlign == Cue.positionAlign.RIGHT) { - captionsStyle.cssFloat = 'right'; + style.cssFloat = 'right'; } - captionsStyle.textAlign = cue.textAlign; - captionsStyle.textDecoration = cue.textDecoration.join(' '); - captionsStyle.writingMode = cue.writingMode; + style.textAlign = cue.textAlign; + style.textDecoration = cue.textDecoration.join(' '); + style.writingMode = cue.writingMode; // The size is a number giving the size of the text container, to be // interpreted as a percentage of the video, as defined by the writing // direction. if (cue.size) { if (cue.writingMode == Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM) { - captionsStyle.width = cue.size + '%'; + style.width = cue.size + '%'; } else { - captionsStyle.height = cue.size + '%'; + style.height = cue.size + '%'; } } } diff --git a/test/test/assets/screenshots/MicrosoftEdge-Windows/deeply-nested-cues.png b/test/test/assets/screenshots/MicrosoftEdge-Windows/deeply-nested-cues.png new file mode 100644 index 0000000000..08838bdaf9 Binary files /dev/null and b/test/test/assets/screenshots/MicrosoftEdge-Windows/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/MicrosoftEdge-Windows/nested-cue-bg.png b/test/test/assets/screenshots/MicrosoftEdge-Windows/nested-cue-bg.png index dc1378496d..c1dcd4366d 100644 Binary files a/test/test/assets/screenshots/MicrosoftEdge-Windows/nested-cue-bg.png and b/test/test/assets/screenshots/MicrosoftEdge-Windows/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/MicrosoftEdge-Windows/region-position.png b/test/test/assets/screenshots/MicrosoftEdge-Windows/region-position.png index cdaa2a4ff7..eb154d6b54 100644 Binary files a/test/test/assets/screenshots/MicrosoftEdge-Windows/region-position.png and b/test/test/assets/screenshots/MicrosoftEdge-Windows/region-position.png differ diff --git a/test/test/assets/screenshots/chrome-Android/deeply-nested-cues.png b/test/test/assets/screenshots/chrome-Android/deeply-nested-cues.png new file mode 100644 index 0000000000..98dcdbe33d Binary files /dev/null and b/test/test/assets/screenshots/chrome-Android/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/chrome-Android/nested-cue-bg.png b/test/test/assets/screenshots/chrome-Android/nested-cue-bg.png index b88f5e9f0a..e1a590fc76 100644 Binary files a/test/test/assets/screenshots/chrome-Android/nested-cue-bg.png and b/test/test/assets/screenshots/chrome-Android/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/chrome-Android/region-position.png b/test/test/assets/screenshots/chrome-Android/region-position.png index c3a2cfd6fd..7832429d68 100644 Binary files a/test/test/assets/screenshots/chrome-Android/region-position.png and b/test/test/assets/screenshots/chrome-Android/region-position.png differ diff --git a/test/test/assets/screenshots/chrome-Linux/deeply-nested-cues.png b/test/test/assets/screenshots/chrome-Linux/deeply-nested-cues.png new file mode 100644 index 0000000000..225ec9f6c6 Binary files /dev/null and b/test/test/assets/screenshots/chrome-Linux/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/chrome-Linux/nested-cue-bg.png b/test/test/assets/screenshots/chrome-Linux/nested-cue-bg.png index 5e141a767b..9889da379f 100644 Binary files a/test/test/assets/screenshots/chrome-Linux/nested-cue-bg.png and b/test/test/assets/screenshots/chrome-Linux/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/chrome-Linux/region-position.png b/test/test/assets/screenshots/chrome-Linux/region-position.png index c75e5d25b9..d6645c9fc9 100644 Binary files a/test/test/assets/screenshots/chrome-Linux/region-position.png and b/test/test/assets/screenshots/chrome-Linux/region-position.png differ diff --git a/test/test/assets/screenshots/chrome-Mac/deeply-nested-cues.png b/test/test/assets/screenshots/chrome-Mac/deeply-nested-cues.png new file mode 100644 index 0000000000..fbb6146b2a Binary files /dev/null and b/test/test/assets/screenshots/chrome-Mac/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/chrome-Mac/nested-cue-bg.png b/test/test/assets/screenshots/chrome-Mac/nested-cue-bg.png index 51ee5d1ad1..e63a87319f 100644 Binary files a/test/test/assets/screenshots/chrome-Mac/nested-cue-bg.png and b/test/test/assets/screenshots/chrome-Mac/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/chrome-Mac/region-position.png b/test/test/assets/screenshots/chrome-Mac/region-position.png index e8b42ae014..e81877ee51 100644 Binary files a/test/test/assets/screenshots/chrome-Mac/region-position.png and b/test/test/assets/screenshots/chrome-Mac/region-position.png differ diff --git a/test/test/assets/screenshots/chrome-Windows/deeply-nested-cues.png b/test/test/assets/screenshots/chrome-Windows/deeply-nested-cues.png new file mode 100644 index 0000000000..ef457a5806 Binary files /dev/null and b/test/test/assets/screenshots/chrome-Windows/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/chrome-Windows/nested-cue-bg.png b/test/test/assets/screenshots/chrome-Windows/nested-cue-bg.png index 44d6ad101a..be44749fc9 100644 Binary files a/test/test/assets/screenshots/chrome-Windows/nested-cue-bg.png and b/test/test/assets/screenshots/chrome-Windows/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/chrome-Windows/region-position.png b/test/test/assets/screenshots/chrome-Windows/region-position.png index 2c67cb7d32..513a6a06b2 100644 Binary files a/test/test/assets/screenshots/chrome-Windows/region-position.png and b/test/test/assets/screenshots/chrome-Windows/region-position.png differ diff --git a/test/test/assets/screenshots/firefox-Linux/deeply-nested-cues.png b/test/test/assets/screenshots/firefox-Linux/deeply-nested-cues.png new file mode 100644 index 0000000000..3e038c1539 Binary files /dev/null and b/test/test/assets/screenshots/firefox-Linux/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/firefox-Linux/nested-cue-bg.png b/test/test/assets/screenshots/firefox-Linux/nested-cue-bg.png index 3048fa3703..a8b24ffceb 100644 Binary files a/test/test/assets/screenshots/firefox-Linux/nested-cue-bg.png and b/test/test/assets/screenshots/firefox-Linux/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/firefox-Linux/region-position.png b/test/test/assets/screenshots/firefox-Linux/region-position.png index 8d8599840b..f1682131ff 100644 Binary files a/test/test/assets/screenshots/firefox-Linux/region-position.png and b/test/test/assets/screenshots/firefox-Linux/region-position.png differ diff --git a/test/test/assets/screenshots/firefox-Mac/deeply-nested-cues.png b/test/test/assets/screenshots/firefox-Mac/deeply-nested-cues.png new file mode 100644 index 0000000000..2935bca688 Binary files /dev/null and b/test/test/assets/screenshots/firefox-Mac/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/firefox-Mac/nested-cue-bg.png b/test/test/assets/screenshots/firefox-Mac/nested-cue-bg.png index 1f1706077d..b67e512163 100644 Binary files a/test/test/assets/screenshots/firefox-Mac/nested-cue-bg.png and b/test/test/assets/screenshots/firefox-Mac/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/firefox-Mac/region-position.png b/test/test/assets/screenshots/firefox-Mac/region-position.png index df6326b8ab..0b84a2c96c 100644 Binary files a/test/test/assets/screenshots/firefox-Mac/region-position.png and b/test/test/assets/screenshots/firefox-Mac/region-position.png differ diff --git a/test/test/assets/screenshots/firefox-Windows/deeply-nested-cues.png b/test/test/assets/screenshots/firefox-Windows/deeply-nested-cues.png new file mode 100644 index 0000000000..190ffdc33b Binary files /dev/null and b/test/test/assets/screenshots/firefox-Windows/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/firefox-Windows/nested-cue-bg.png b/test/test/assets/screenshots/firefox-Windows/nested-cue-bg.png index e88b39e12e..b501338bc1 100644 Binary files a/test/test/assets/screenshots/firefox-Windows/nested-cue-bg.png and b/test/test/assets/screenshots/firefox-Windows/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/firefox-Windows/region-position.png b/test/test/assets/screenshots/firefox-Windows/region-position.png index a1c1782464..bf2e2fd4af 100644 Binary files a/test/test/assets/screenshots/firefox-Windows/region-position.png and b/test/test/assets/screenshots/firefox-Windows/region-position.png differ diff --git a/test/test/assets/screenshots/internet explorer-Windows/deeply-nested-cues.png b/test/test/assets/screenshots/internet explorer-Windows/deeply-nested-cues.png new file mode 100644 index 0000000000..c2711997a1 Binary files /dev/null and b/test/test/assets/screenshots/internet explorer-Windows/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/internet explorer-Windows/nested-cue-bg.png b/test/test/assets/screenshots/internet explorer-Windows/nested-cue-bg.png index 1d3418fd43..f556e86624 100644 Binary files a/test/test/assets/screenshots/internet explorer-Windows/nested-cue-bg.png and b/test/test/assets/screenshots/internet explorer-Windows/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/internet explorer-Windows/region-position.png b/test/test/assets/screenshots/internet explorer-Windows/region-position.png index 7ce795e364..a45ff60d09 100644 Binary files a/test/test/assets/screenshots/internet explorer-Windows/region-position.png and b/test/test/assets/screenshots/internet explorer-Windows/region-position.png differ diff --git a/test/test/assets/screenshots/review.html b/test/test/assets/screenshots/review.html index afae8f2d6a..35841959d3 100644 --- a/test/test/assets/screenshots/review.html +++ b/test/test/assets/screenshots/review.html @@ -50,6 +50,7 @@ 'bitmap-cue', 'nested-cue-bg', 'flat-cue-bg', + 'deeply-nested-cues', ]; diff --git a/test/test/assets/screenshots/safari-Mac/deeply-nested-cues.png b/test/test/assets/screenshots/safari-Mac/deeply-nested-cues.png new file mode 100644 index 0000000000..4b5eae861f Binary files /dev/null and b/test/test/assets/screenshots/safari-Mac/deeply-nested-cues.png differ diff --git a/test/test/assets/screenshots/safari-Mac/nested-cue-bg.png b/test/test/assets/screenshots/safari-Mac/nested-cue-bg.png index 2c61fe664d..9f267af63f 100644 Binary files a/test/test/assets/screenshots/safari-Mac/nested-cue-bg.png and b/test/test/assets/screenshots/safari-Mac/nested-cue-bg.png differ diff --git a/test/test/assets/screenshots/safari-Mac/region-position.png b/test/test/assets/screenshots/safari-Mac/region-position.png index 00ce7aab04..5aa8ab69d6 100644 Binary files a/test/test/assets/screenshots/safari-Mac/region-position.png and b/test/test/assets/screenshots/safari-Mac/region-position.png differ diff --git a/test/text/simple_text_displayer_unit.js b/test/text/simple_text_displayer_unit.js index d54c155669..3ed4fd8460 100644 --- a/test/text/simple_text_displayer_unit.js +++ b/test/text/simple_text_displayer_unit.js @@ -84,7 +84,7 @@ describe('SimpleTextDisplayer', () => { it('appends nested cues', () => { const shakaCue = new shaka.text.Cue(10, 20, ''); - const nestedCue1 = new shaka.text.Cue(10, 20, 'Test1'); + const nestedCue1 = new shaka.text.Cue(10, 20, 'Test1 '); const nestedCue2 = new shaka.text.Cue(10, 20, 'Test2'); shakaCue.nestedCues = [nestedCue1, nestedCue2]; diff --git a/test/text/ui_text_displayer_unit.js b/test/text/ui_text_displayer_unit.js index ee81c749dd..3be0e821af 100644 --- a/test/text/ui_text_displayer_unit.js +++ b/test/text/ui_text_displayer_unit.js @@ -83,7 +83,7 @@ describe('UITextDisplayer', () => { const textContainer = videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); + const captions = textContainer.querySelector('div'); const cssObj = parseCssText(captions.style.cssText); expect(cssObj).toEqual( jasmine.objectContaining({ @@ -105,7 +105,7 @@ describe('UITextDisplayer', () => { it('correctly displays styles for nested cues', async () => { /** @type {!shaka.text.Cue} */ - const cue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); + const cue = new shaka.text.Cue(0, 100, ''); const nestedCue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); cue.nestedCues = [nestedCue]; nestedCue.textAlign = 'center'; @@ -170,7 +170,7 @@ describe('UITextDisplayer', () => { const textContainer = videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); + const captions = textContainer.querySelector('div'); const cssObj = parseCssText(captions.style.cssText); expect(cssObj).toEqual( jasmine.objectContaining({ @@ -200,7 +200,7 @@ describe('UITextDisplayer', () => { const textContainer = videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); + const captions = textContainer.querySelector('div'); const cssObj = parseCssText(captions.style.cssText); expect(cssObj).toEqual( jasmine.objectContaining({'font-size': expectedFontSize})); @@ -214,7 +214,7 @@ describe('UITextDisplayer', () => { await shaka.test.Util.delay(0.5); /** @type {Element} */ const textContainer = videoContainer.querySelector('.shaka-text-container'); - let captions = textContainer.querySelectorAll('span'); + let captions = textContainer.querySelectorAll('div'); // Expect textContainer to display this cue. expect(captions.length).toBe(1); @@ -222,7 +222,7 @@ describe('UITextDisplayer', () => { textDisplayer.append([cue2]); // Wait until updateCaptions_() gets called. await shaka.test.Util.delay(0.5); - captions = textContainer.querySelectorAll('span'); + captions = textContainer.querySelectorAll('div'); // Expect textContainer to display one cue without duplication. expect(captions.length).toBe(1); }); diff --git a/test/ui/text_displayer_layout_unit.js b/test/ui/text_displayer_layout_unit.js index c4ab5e6d8a..44ca365ab0 100644 --- a/test/ui/text_displayer_layout_unit.js +++ b/test/ui/text_displayer_layout_unit.js @@ -6,6 +6,7 @@ const Util = shaka.test.Util; +// TODO: Move this suite to the text/ folder where it belongs filterDescribe('UITextDisplayer layout', Util.supportsScreenshots, () => { const UiUtils = shaka.test.UiUtils; @@ -129,7 +130,7 @@ filterDescribe('UITextDisplayer layout', Util.supportsScreenshots, () => { it('two nested cues', async () => { const cue = new shaka.text.Cue(0, 1, ''); cue.nestedCues = [ - new shaka.text.Cue(0, 1, 'Captain\'s log,'), + new shaka.text.Cue(0, 1, 'Captain\'s log, '), new shaka.text.Cue(0, 1, 'stardate 41636.9'), ]; textDisplayer.append([cue]); @@ -203,19 +204,22 @@ filterDescribe('UITextDisplayer layout', Util.supportsScreenshots, () => { await Util.checkScreenshot(videoContainer, 'bitmap-cue', threshold); }); - // Regression test for #2623 - it('colors background for nested cues but not parent', async () => { + // Used to be a regression test for #2623, but that should have been fixed in + // the TTML parser instead. Now we ensure that backgroundColor is honored at + // all levels, making this a regression test for the fix to our original fix. + // :-) + it('honors background settings at all levels', async () => { const cue = new shaka.text.Cue(0, 1, ''); cue.nestedCues = [ - new shaka.text.Cue(0, 1, 'Captain\'s log,'), + new shaka.text.Cue(0, 1, 'Captain\'s '), + new shaka.text.Cue(0, 1, 'log, '), new shaka.text.Cue(0, 1, 'stardate 41636.9'), ]; - // These are shown. + // These middle nested cue will show through to the parent cue's background. cue.nestedCues[0].backgroundColor = 'blue'; - cue.nestedCues[1].backgroundColor = 'yellow'; - - // This is not. + cue.nestedCues[1].backgroundColor = 'transparent'; + cue.nestedCues[2].backgroundColor = 'yellow'; cue.backgroundColor = 'purple'; textDisplayer.append([cue]); @@ -223,7 +227,6 @@ filterDescribe('UITextDisplayer layout', Util.supportsScreenshots, () => { await Util.checkScreenshot(videoContainer, 'nested-cue-bg', threshold); }); - // Related to the fix for #2623 it('colors background for flat cues', async () => { const cue = new shaka.text.Cue(0, 1, 'Captain\'s log, stardate 41636.9'); // This is shown. @@ -233,4 +236,29 @@ filterDescribe('UITextDisplayer layout', Util.supportsScreenshots, () => { await Util.checkScreenshot(videoContainer, 'flat-cue-bg', threshold); }); + + // https://github.com/google/shaka-player/issues/2761 + it('deeply-nested cues', async () => { + const makeCue = (text, fg = '', bg = '', nestedCues = []) => { + const cue = new shaka.text.Cue(0, 1, text); + cue.color = fg; + cue.backgroundColor = bg; + cue.nestedCues = nestedCues; + return cue; + }; + + const cue = makeCue('', '', '', [ + makeCue('', '', 'black', [ + makeCue('Captain\'s '), + makeCue('log, ', 'red'), + makeCue('stardate ', 'green', 'blue', [ + makeCue('41636.9', 'purple'), + ]), + ]), + ]); + + textDisplayer.append([cue]); + + await Util.checkScreenshot(videoContainer, 'deeply-nested-cues', threshold); + }); }); diff --git a/test/ui/text_displayer_unit.js b/test/ui/text_displayer_unit.js deleted file mode 100644 index ee81c749dd..0000000000 --- a/test/ui/text_displayer_unit.js +++ /dev/null @@ -1,229 +0,0 @@ -/*! @license - * Shaka Player - * Copyright 2016 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -describe('UITextDisplayer', () => { - /** @type {!HTMLElement} */ - let videoContainer; - /** @type {!HTMLVideoElement} */ - let video; - /** @type {shaka.text.UITextDisplayer} */ - let textDisplayer; - /** @type {number} */ - const videoContainerHeight = 450; - - /** - * Transform a cssText to an object. - * Example: - * cssText: 'background-color: black; color: green; font-size: 10px;' - * cssObject: { - * background-color: 'black', - * color: 'green', - * font-size: '10px', - * } - * @param {string} cssStr - * @return {!Object.} - */ - function parseCssText(cssStr) { - // Remove the white spaces in the string. - // Split with ';' and ignore the last one. - const css = cssStr.replace(/\s/g, '').substring(0, cssStr.length - 1) - .split(';'); - const cssObj = {}; - for (const cssStyle of css) { - const propertyAndValue = cssStyle.split(':'); - let value = propertyAndValue[1]; - value = isNaN(value) ? value : Number(value); - cssObj[propertyAndValue[0]] = value; - } - return cssObj; - } - - beforeAll(() => { - videoContainer = - /** @type {!HTMLElement} */ (document.createElement('div')); - videoContainer.style.height = `${videoContainerHeight}px`; - document.body.appendChild(videoContainer); - video = shaka.test.UiUtils.createVideoElement(); - videoContainer.appendChild(video); - }); - - beforeEach(() => { - textDisplayer = new shaka.text.UITextDisplayer(video, videoContainer); - }); - - afterEach(async () => { - await textDisplayer.destroy(); - }); - - afterAll(() => { - document.body.removeChild(videoContainer); - }); - - it('correctly displays styles for cues', async () => { - /** @type {!shaka.text.Cue} */ - const cue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - cue.color = 'green'; - cue.backgroundColor = 'black'; - cue.direction = shaka.text.Cue.direction.HORIZONTAL_LEFT_TO_RIGHT; - cue.fontSize = '10px'; - cue.fontWeight = shaka.text.Cue.fontWeight.NORMAL; - cue.fontStyle = 'normal'; - cue.lineHeight = '2'; - cue.nestedCues = []; - cue.textAlign = 'center'; - cue.writingMode = shaka.text.Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM; - - textDisplayer.setTextVisibility(true); - textDisplayer.append([cue]); - // Wait until updateCaptions_() gets called. - await shaka.test.Util.delay(0.5); - - const textContainer = - videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); - const cssObj = parseCssText(captions.style.cssText); - expect(cssObj).toEqual( - jasmine.objectContaining({ - 'color': 'green', - 'background-color': 'black', - 'direction': 'ltr', - 'font-size': '10px', - 'font-style': 'normal', - 'font-weight': 400, - 'line-height': 2, - 'text-align': 'center', - // TODO: We're not testing writing-mode since IE 11 only supports - // deprecated writing-mode values partially. Add it back once we end - // support for IE 11. - // https://github.com/google/shaka-player/issues/2339 - // 'writing-mode': 'horizontal-tb', - })); - }); - - it('correctly displays styles for nested cues', async () => { - /** @type {!shaka.text.Cue} */ - const cue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - const nestedCue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - cue.nestedCues = [nestedCue]; - nestedCue.textAlign = 'center'; - nestedCue.writingMode = shaka.text.Cue.writingMode.HORIZONTAL_TOP_TO_BOTTOM; - nestedCue.color = 'green'; - nestedCue.backgroundColor = 'black'; - nestedCue.fontSize = '10px'; - nestedCue.fontWeight = shaka.text.Cue.fontWeight.NORMAL; - nestedCue.fontStyle = 'normal'; - nestedCue.lineHeight = '2'; - nestedCue.nestedCues = []; - - textDisplayer.setTextVisibility(true); - textDisplayer.append([cue]); - // Wait until updateCaptions_() gets called. - await shaka.test.Util.delay(0.5); - - // Verify styles applied to the nested cues. - const textContainer = - videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); - const cssObj = parseCssText(captions.style.cssText); - expect(cssObj).toEqual( - jasmine.objectContaining({ - 'color': 'green', - 'background-color': 'black', - 'font-size': '10px', - 'font-style': 'normal', - 'font-weight': 400, - 'text-align': 'center', - // TODO: We're not testing writing-mode since IE 11 only supports - // deprecated writing-mode values partially. Add it back once we end - // support for IE 11. - // https://github.com/google/shaka-player/issues/2339 - // 'writing-mode': 'horizontal-tb', - })); - }); - - it('correctly displays styles for cellResolution units', async () => { - /** @type {!shaka.text.Cue} */ - const cue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - cue.fontSize = '0.80c'; - cue.linePadding = '0.50c'; - cue.cellResolution = { - columns: 60, - rows: 20, - }; - - textDisplayer.setTextVisibility(true); - textDisplayer.append([cue]); - // Wait until updateCaptions_() gets called. - await shaka.test.Util.delay(0.5); - - // Expected value is calculated based on ttp:cellResolution="60 20" - // videoContainerHeight=450px and tts:fontSize="0.80c" on the default style. - const expectedFontSize = '18px'; - - // Expected value is calculated based on ttp:cellResolution="60 20" - // videoContainerHeight=450px and ebutts:linePadding="0.5c" on the default - // style. - const expectedLinePadding = '11.25px'; - - const textContainer = - videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); - const cssObj = parseCssText(captions.style.cssText); - expect(cssObj).toEqual( - jasmine.objectContaining({ - 'font-size': expectedFontSize, - 'padding-left': expectedLinePadding, - 'padding-right': expectedLinePadding, - })); - }); - - it('correctly displays styles for percentages units', async () => { - /** @type {!shaka.text.Cue} */ - const cue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - cue.fontSize = '90%'; - cue.cellResolution = { - columns: 32, - rows: 15, - }; - - textDisplayer.setTextVisibility(true); - textDisplayer.append([cue]); - // Wait until updateCaptions_() gets called. - await shaka.test.Util.delay(0.5); - - // Expected value is calculated based on ttp:cellResolution="32 15" - // videoContainerHeight=450px and tts:fontSize="90%" on the default style. - const expectedFontSize = '27px'; - - const textContainer = - videoContainer.querySelector('.shaka-text-container'); - const captions = textContainer.querySelector('span'); - const cssObj = parseCssText(captions.style.cssText); - expect(cssObj).toEqual( - jasmine.objectContaining({'font-size': expectedFontSize})); - }); - - it('does not display duplicate cues', async () => { - const cue = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - textDisplayer.setTextVisibility(true); - textDisplayer.append([cue]); - // Wait until updateCaptions_() gets called. - await shaka.test.Util.delay(0.5); - /** @type {Element} */ - const textContainer = videoContainer.querySelector('.shaka-text-container'); - let captions = textContainer.querySelectorAll('span'); - // Expect textContainer to display this cue. - expect(captions.length).toBe(1); - - const cue2 = new shaka.text.Cue(0, 100, 'Captain\'s log.'); - textDisplayer.append([cue2]); - // Wait until updateCaptions_() gets called. - await shaka.test.Util.delay(0.5); - captions = textContainer.querySelectorAll('span'); - // Expect textContainer to display one cue without duplication. - expect(captions.length).toBe(1); - }); -}); diff --git a/ui/less/containers.less b/ui/less/containers.less index 4038c58847..d3590d4089 100644 --- a/ui/less/containers.less +++ b/ui/less/containers.less @@ -224,27 +224,11 @@ justify-content: flex-end; /* These are defaults which are overridden by JS or cue styles. */ - font-size: 20px; - line-height: 1.4; // relative to font size. - - /* This makes nested elements have this specific font size, too. Without - * this, defaults at the app level for generic elements like

may be - * specific enough to override font size for those elements in captions. */ - * { + div { font-size: 20px; line-height: 1.4; // relative to font size. - } - - span { - /* These are defaults which are overridden by JS or cue styles. */ background-color: rgba(0, 0, 0, 0.8); color: rgb(255, 255, 255); - display: inline-block; - } - - .shaka-nested-cue:not(:last-of-type):after { - content: " "; - white-space: pre; } }