fix: Fix rendering of TTML nested cues and spacers

TTML nested cues are meant to be displayed horizontally as inline
elements.  This fixes the rendering of these nested cues in both
SimpleTextDisplayer and UITextDisplayer.

In UITextDisplayer, the styles have been adjusted to lay out the
nested cues horizontally rather than vertically.

In SimpleTextDisplayer, the nested cues were being displayed as if
they were top-level cues.  This change concatenates the nested cues
into a single cue displayed in the browser.

This also improves comments on the poorly-named "spacer" property,
which represents a line break in TTML.

This fixes the rendering of "spacer" in SimpleTextDisplayer by
inserting an actual newline character into the collapsed nested cues.

Finally, this fixes and clarifies names used internally in
UITextDisplayer.  For example, there is a difference between a nested
cue and leaf cue.  A nested cue and a top-level cue without nested
cues are both "leaf" cues, but a top-level cue is never a "nested"
cue, since it is at the top level.  The conflation of these names
before this fix made it difficult to understand and fix the code in
the first place.

Closes #2760

Change-Id: I89633761d12704e253371d17e2e786c5b2ed67a7

externs/shaka/text.js

@@ -126,7 +126,8 @@ shaka.extern.Cue = class {
     this.endTime;
 
     /**
-     * The text payload of the cue.
+     * The text payload of the cue.  If nestedCues is non-empty, this should be
+     * empty.  Top-level block containers should have no payload of their own.
      * @type {!string}
      * @exportDoc
      */
@@ -344,14 +345,15 @@ shaka.extern.Cue = class {
     this.id;
 
     /**
-     * Nested cues
+     * Nested cues, which should be laid out horizontally in one block.
      * @type {Array.<!shaka.extern.Cue>}
      * @exportDoc
      */
     this.nestedCues;
 
     /**
-     * Whether or not the cue only acts as a spacer between two cues
+     * Whether or not the cue only acts as a line break between two nested cues.
+     * Should only appear in nested cues.
      * @type {boolean}
      * @exportDoc
      */

lib/text/cue.js

@@ -217,6 +217,28 @@ shaka.text.Cue = class {
       rows: 15,
     };
   }
+
+  /**
+   * Create a copy of the cue with the same properties.
+   * @return {!shaka.text.Cue}
+   * @suppress {checkTypes} since we must use [] and "in" with a struct type.
+   */
+  clone() {
+    const clone = new shaka.text.Cue(0, 0, '');
+
+    for (const k in this) {
+      clone[k] = this[k];
+
+      // Make copies of array fields, but only one level deep.  That way, if we
+      // change, for instance, textDecoration on the clone, we don't affect the
+      // original.
+      if (clone[k] && clone[k].constructor == Array) {
+        clone[k] = /** @type {!Array} */(clone[k]).slice();
+      }
+    }
+
+    return clone;
+  }
 };
 
 

lib/text/simple_text_displayer.js

@@ -84,15 +84,33 @@ shaka.text.SimpleTextDisplayer = class {
    * @export
    */
   append(cues) {
-    // Flatten the cues and their nestedCues into a list.
-    let flattenedCues = [];
-    for (const cue of 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.
+    const flattenedCues = cues.map((cue) => {
       if (cue.nestedCues.length) {
-        flattenedCues = flattenedCues.concat(cue.nestedCues);
+        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;
+        return flatCue;
       } else {
-        flattenedCues.push(cue);
+        return cue;
       }
-    }
+    });
+
     // Convert cues.
     const textTrackCues = [];
     const cuesInTextTrack = this.textTrack_.cues ?

lib/text/ui_text_displayer.js

@@ -197,16 +197,20 @@ shaka.text.UITextDisplayer = class {
    *
    * @param {Element} container
    * @param {!shaka.extern.Cue} cue
+   * @param {boolean} isNested
    * @return {!Element} the created captions container
    * @private
    */
-  displayNestedCue_(container, cue) {
+  displayLeafCue_(container, cue, isNested) {
     const captions = shaka.util.Dom.createHTMLElement('span');
+    if (isNested) {
+      captions.classList.add('shaka-nested-cue');
+    }
 
     if (cue.spacer) {
       captions.style.display = 'block';
     } else {
-      this.setCaptionStyles_(captions, cue, /* isNested= */ true);
+      this.setCaptionStyles_(captions, cue, /* isLeaf= */ true);
     }
 
     container.appendChild(captions);
@@ -225,33 +229,35 @@ shaka.text.UITextDisplayer = class {
     if (cue.nestedCues.length) {
       const nestedCuesContainer = shaka.util.Dom.createHTMLElement('p');
       nestedCuesContainer.style.width = '100%';
-      this.setCaptionStyles_(nestedCuesContainer, cue, /* isNested= */ false);
+      this.setCaptionStyles_(nestedCuesContainer, cue, /* isLeaf= */ false);
 
       for (let i = 0; i < cue.nestedCues.length; i++) {
-        this.displayNestedCue_(nestedCuesContainer, cue.nestedCues[i]);
+        this.displayLeafCue_(
+            nestedCuesContainer, cue.nestedCues[i], /* isNested= */ true);
       }
 
       container.appendChild(nestedCuesContainer);
       this.currentCuesMap_.set(cue, nestedCuesContainer);
     } else {
-      this.currentCuesMap_.set(cue, this.displayNestedCue_(container, cue));
+      this.currentCuesMap_.set(cue,
+          this.displayLeafCue_(container, cue, /* isNested= */ false));
     }
   }
 
   /**
    * @param {!HTMLElement} captions
    * @param {!shaka.extern.Cue} cue
-   * @param {boolean} isNested
+   * @param {boolean} isLeaf
    * @private
    */
-  setCaptionStyles_(captions, cue, isNested) {
+  setCaptionStyles_(captions, cue, isLeaf) {
     const Cue = shaka.text.Cue;
     const captionsStyle = captions.style;
 
     // Set white-space to 'pre-line' to enable showing line breaks in the text.
     captionsStyle.whiteSpace = 'pre-line';
     captions.textContent = cue.payload;
-    if (isNested) {
+    if (isLeaf) {
       captionsStyle.backgroundColor = cue.backgroundColor;
     }
     captionsStyle.border = cue.border;
@@ -292,13 +298,19 @@ shaka.text.UITextDisplayer = class {
     } else {
       captionsStyle.justifyContent = 'flex-end';
     }
+
     if (cue.nestedCues.length) {
-      captionsStyle.alignItems = 'center';
       captionsStyle.display = 'flex';
-      captionsStyle.flexDirection = 'column';
+      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 (isNested) {
+
+    if (isLeaf) {
       // 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
@@ -353,7 +365,7 @@ shaka.text.UITextDisplayer = class {
           }
         }
       }
-    } else if (cue.region && cue.region.id && !isNested) {
+    } else if (cue.region && cue.region.id && !isLeaf) {
       const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE;
       const heightUnit = cue.region.heightUnits == percentageUnit ? '%' : 'px';
       const widthUnit = cue.region.widthUnits == percentageUnit ? '%' : 'px';

test/text/simple_text_displayer_unit.js

@@ -90,8 +90,7 @@ describe('SimpleTextDisplayer', () => {
       shakaCue.nestedCues = [nestedCue1, nestedCue2];
       verifyHelper(
           [
-            {startTime: 10, endTime: 20, text: 'Test2'},
-            {startTime: 10, endTime: 20, text: 'Test1'},
+            {startTime: 10, endTime: 20, text: 'Test1 Test2'},
           ],
           [shakaCue]);
     });

ui/less/containers.less

@@ -227,12 +227,25 @@
   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 <p> may be
+   * specific enough to override font size for those elements in captions. */
+  * {
+    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;
+  }
 }
 
 .shaka-controls-container[shown="true"] ~ .shaka-text-container {