feat(text): Clarify and clean up text Cue contract

Changes:
 - Update comments on shaka.extern.Cue to clarify how fields should be
   used and interpreted
 - Add support for arbitrarily-deep nested cues in both text
   displayers
 - Stop adding whitespace between nested cues (parsers should emit
   exactly what they want displayed) and update tests to match
 - Remove obscure special cases from UITextDisplayer styles and make
   styling consistent with the Cue docs
 - Fix rendering for single line break (cue.spacer) elements
 - Add a layout test for deeply-nested cues
 - Remove reundant and identical set of UITextDisplayer unit tests
 - Update layout test for backgrounds to reflect the clarified rules
 - Update remaining UITextDisplayer unit tests to look for div instead
   of span for top-level cues
 - Updates screenshots for region-position, which changed slightly in
   appearance due to changes in whitespace rules
 - Updates screenshots for nested-cue-bg, to reflect simplified rules
   for applying backgrounds

TODO:
 - Temporarily breaks TTML output until the TTML parser can be updated
   to conform to the clarified rules in the Cue docs
 - Still more special cases around regions in UITextDisplayer
 - Rendering for multiple line break elements in UITextDisplayer
 - Support bold, italic, and underline in SimpleTextDisplayer
 - None of the displayers will honor the wrapLine field yet

Closes shaka-project#2762 (extra vertical space seen for line breaks)
Issue shaka-project#2761 (deeply-nested cues)

Change-Id: I02ac8213e4de67a65fb38d596d2c59536f3722ee

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

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,42 +233,37 @@ 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
      */
     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.<!shaka.extern.Cue>}
+     * Top-level cues are blocks, and nested cues are inline elements.
+     * Cues can be nested arbitrarily deeply.
+     * @type {!Array.<!shaka.extern.Cue>}
      * @exportDoc
      */
     this.nestedCues;
@@ -358,6 +356,7 @@ shaka.extern.Cue = class {
      * @exportDoc
      */
     this.spacer;
+    // TODO: Rename "spacer" to "lineBreak".
   }
 };
 

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;
 

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);