fix: Fixes updating of nested cues

Previously, we added support for respecting the time constraints
of nested cues. However, the UI text displayer did not take the
time constraints of nested cues into account when determining when
and how to update the cues.
This changes the UI text displayer to also do that.

Issue #3524
Issue #3643

Change-Id: I6b643f2aa21f367a8e40a8aca2ebb62492c071c2

lib/text/ui_text_displayer.js

@@ -73,7 +73,14 @@ shaka.text.UITextDisplayer = class {
       this.updateCaptions_();
     }).tickEvery(updatePeriod);
 
-    /** private {Map.<!shaka.extern.Cue, !HTMLElement>} */
+    /**
+     * Maps cues to cue elements. Specifically points out the wrapper element of
+     * the cue (e.g. the HTML element to put nested cues inside).
+     * @private {Map.<!shaka.extern.Cue, !{
+     *   cueElement: !HTMLElement,
+     *   wrapper: !HTMLElement
+     * }>}
+     */
     this.currentCuesMap_ = new Map();
 
     /** @private {shaka.util.EventManager} */
@@ -184,69 +191,144 @@ shaka.text.UITextDisplayer = class {
     this.isTextVisible_ = on;
   }
 
+  /**
+   * @param {!Array.<!shaka.text.Cue>} cues
+   * @param {?HTMLElement} container
+   * @param {number} currentTime
+   * @param {boolean} isNested
+   * @private
+   */
+  updateCuesRecursive_(cues, container, currentTime, isNested) {
+    // Set to true if the cues have changed in some way, which will require
+    // DOM changes. E.g. if a cue was added or removed.
+    let updateDOM = false;
+    /**
+     * The elements to remove from the DOM.
+     * Some of these elements may be added back again, if their corresponding
+     * cue is in toPlant.
+     * These elements are only removed if updateDOM is true.
+     * @type {!Array.<!HTMLElement>}
+     */
+    const toUproot = [];
+    /**
+     * The cues whose corresponding elements should be in the DOM.
+     * Some of these might be new, some might have been displayed beforehand.
+     * These will only be added if updateDOM is true.
+     * @type {!Array.<!shaka.text.Cue>}
+     */
+    const toPlant = [];
+    for (const cue of cues) {
+      let cueRegistry = this.currentCuesMap_.get(cue);
+      const shouldBeDisplayed =
+          cue.startTime <= currentTime && cue.endTime > currentTime;
+
+      if (cueRegistry) {
+        // If the cues are replanted, all existing cues should be uprooted,
+        // even ones which are going to be planted again.
+        toUproot.push(cueRegistry.cueElement);
+
+        // If the cue should not be displayed, remove it entirely.
+        if (!shouldBeDisplayed) {
+          // Since something has to be removed, we will need to update the DOM.
+          updateDOM = true;
+          this.currentCuesMap_.delete(cue);
+          cueRegistry = null;
+        }
+      }
+
+      if (shouldBeDisplayed) {
+        toPlant.push(cue);
+        if (!cueRegistry) {
+          // The cue has to be made!
+          this.createCue_(cue, isNested);
+          cueRegistry = this.currentCuesMap_.get(cue);
+          updateDOM = true;
+        }
+      }
+
+      // Recursively check the nested cues, to see if they need to be added or
+      // removed.
+      if (cue.nestedCues.length > 0) {
+        this.updateCuesRecursive_(
+            cue.nestedCues, cueRegistry ? cueRegistry.wrapper : null,
+            currentTime, /* isNested= */ true);
+      }
+    }
+    // Note that the container might be null, if this is recursing over the
+    // children of a cue that is not displayed. This can happen if the cue was
+    // just removed, but its nested cues still need to be removed from
+    // this.currentCuesMap_.
+    if (updateDOM && container) {
+      for (const cueElement of toUproot) {
+        container.removeChild(cueElement);
+      }
+      toPlant.sort((a, b) => {
+        if (a.startTime != b.startTime) {
+          return a.startTime - b.startTime;
+        } else {
+          return a.endTime - b.endTime;
+        }
+      });
+      for (const cue of toPlant) {
+        const cueRegistry = this.currentCuesMap_.get(cue);
+        goog.asserts.assert(cueRegistry, 'cueRegistry should exist.');
+        container.appendChild(cueRegistry.cueElement);
+      }
+    }
+  }
+
   /**
    * Display the current captions.
    * @param {boolean=} forceUpdate
    * @private
    */
   updateCaptions_(forceUpdate = false) {
+    if (!this.textContainer_) {
+      return;
+    }
+
     const currentTime = this.video_.currentTime;
-
-    // Return true if the cue should be displayed at the current time point.
-    const shouldCueBeDisplayed = (cue) => {
-      return this.cues_.includes(cue) && this.isTextVisible_ &&
-             cue.startTime <= currentTime && cue.endTime > currentTime;
-    };
-
-    // For each cue in the current cues map, if the cue's end time has passed,
-    // remove the entry from the map, and remove the captions from the page.
-    for (const cue of this.currentCuesMap_.keys()) {
-      if (!shouldCueBeDisplayed(cue) || forceUpdate) {
-        const captions = this.currentCuesMap_.get(cue);
-        this.textContainer_.removeChild(captions);
-        this.currentCuesMap_.delete(cue);
+    if (!this.isTextVisible_ || forceUpdate) {
+      if (this.currentCuesMap_.size > 0) {
+        // Clear away any existing cues.
+        shaka.util.Dom.removeAllChildren(this.textContainer_);
+        this.currentCuesMap_.clear();
       }
     }
-
-    // Sometimes we don't remove a cue element correctly.  So check all the
-    // child nodes and remove any that don't have an associated cue.
-    const expectedChildren = new Set(this.currentCuesMap_.values());
-    for (const child of Array.from(this.textContainer_.childNodes)) {
-      if (!expectedChildren.has(child)) {
-        this.textContainer_.removeChild(child);
+    if (this.isTextVisible_) {
+      // Log currently attached cue elements for verification, later.
+      const previousCuesMap = new Map();
+      for (const cue of this.currentCuesMap_.keys()) {
+        previousCuesMap.set(cue, this.currentCuesMap_.get(cue));
       }
-    }
 
-    // Get the current cues that should be added to display. If the cue is not
-    // being displayed already, add it to the map, and add the captions onto the
-    // page.
-    const currentCues = this.cues_.filter((cue) => {
-      return shouldCueBeDisplayed(cue) && !this.currentCuesMap_.has(cue);
-    }).sort((a, b) => {
-      if (a.startTime != b.startTime) {
-        return a.startTime - b.startTime;
-      } else {
-        return a.endTime - b.endTime;
+      // Update the cues.
+      this.updateCuesRecursive_(
+          this.cues_, this.textContainer_, currentTime, false);
+
+      // Sometimes, things fail to remove. Check to make sure that there aren't
+      // any leftover cue elements attached to things. See #2076.
+      for (const cue of previousCuesMap.keys()) {
+        if (!this.currentCuesMap_.has(cue)) {
+          const cueElement = previousCuesMap.get(cue).cueElement;
+          const parentNode = cueElement.parentNode;
+          if (parentNode) {
+            // It wasn't removed successfully...
+            parentNode.removeChild(cueElement);
+          }
+        }
       }
-    });
-
-    for (const cue of currentCues) {
-      const cueElement = this.displayCue_(
-          this.textContainer_, cue, /* isNested= */ false);
-      this.currentCuesMap_.set(cue, cueElement);
     }
   }
 
   /**
-   * Displays a cue
+   * Creates the object for a cue.
    *
-   * @param {Element} container
    * @param {!shaka.extern.Cue} cue
    * @param {boolean} isNested
-   * @return {!Element} the created captions element
    * @private
    */
-  displayCue_(container, cue, isNested) {
+  createCue_(cue, isNested) {
     let type = isNested ? 'span' : 'div';
     if (cue.lineBreak || cue.spacer) {
       if (cue.spacer) {
@@ -273,15 +355,7 @@ shaka.text.UITextDisplayer = class {
       cueElement.appendChild(wrapper);
     }
 
-    const time = this.video_.currentTime;
-    for (const nestedCue of cue.nestedCues) {
-      if (nestedCue.startTime <= time && nestedCue.endTime >= time) {
-        this.displayCue_(wrapper, nestedCue, /* isNested= */ true);
-      }
-    }
-
-    container.appendChild(cueElement);
-    return cueElement;
+    this.currentCuesMap_.set(cue, {cueElement, wrapper});
   }
 
   /**

test/text/ui_text_displayer_unit.js

@@ -320,4 +320,67 @@ describe('UITextDisplayer', () => {
     // duplicates.
     expect(captions.length).toBe(3);
   });
+
+  it('hides and shows nested cues at appropriate times', async () => {
+    const parentCue1 = new shaka.text.Cue(0, 100, '');
+    const cue1 = new shaka.text.Cue(0, 50, 'One');
+    parentCue1.nestedCues.push(cue1);
+    const cue2 = new shaka.text.Cue(25, 75, 'Two');
+    parentCue1.nestedCues.push(cue2);
+    const cue3 = new shaka.text.Cue(50, 100, 'Three');
+    parentCue1.nestedCues.push(cue3);
+
+    const parentCue2 = new shaka.text.Cue(90, 190, '');
+    const cue4 = new shaka.text.Cue(90, 130, 'Four');
+    parentCue2.nestedCues.push(cue4);
+
+    textDisplayer.setTextVisibility(true);
+    textDisplayer.append([parentCue1, parentCue2]);
+
+    video.currentTime = 10;
+    await shaka.test.Util.delay(0.5);
+    /** @type {Element} */
+    const textContainer = videoContainer.querySelector('.shaka-text-container');
+    let parentCueElements = textContainer.querySelectorAll('div');
+
+    expect(parentCueElements.length).toBe(1);
+    expect(parentCueElements[0].textContent).toBe('One');
+
+    video.currentTime = 35;
+    await shaka.test.Util.delay(0.5);
+    parentCueElements = textContainer.querySelectorAll('div');
+    expect(parentCueElements.length).toBe(1);
+    expect(parentCueElements[0].textContent).toBe('OneTwo');
+
+    video.currentTime = 60;
+    await shaka.test.Util.delay(0.5);
+    parentCueElements = textContainer.querySelectorAll('div');
+    expect(parentCueElements.length).toBe(1);
+    expect(parentCueElements[0].textContent).toBe('TwoThree');
+
+    video.currentTime = 85;
+    await shaka.test.Util.delay(0.5);
+    parentCueElements = textContainer.querySelectorAll('div');
+    expect(parentCueElements.length).toBe(1);
+    expect(parentCueElements[0].textContent).toBe('Three');
+
+    video.currentTime = 95;
+    await shaka.test.Util.delay(0.5);
+    parentCueElements = textContainer.querySelectorAll('div');
+    expect(parentCueElements.length).toBe(2);
+    expect(parentCueElements[0].textContent).toBe('Three');
+    expect(parentCueElements[1].textContent).toBe('Four');
+
+    video.currentTime = 105;
+    await shaka.test.Util.delay(0.5);
+    parentCueElements = textContainer.querySelectorAll('div');
+    expect(parentCueElements.length).toBe(1);
+    expect(parentCueElements[0].textContent).toBe('Four');
+
+    video.currentTime = 150;
+    await shaka.test.Util.delay(0.5);
+    parentCueElements = textContainer.querySelectorAll('div');
+    expect(parentCueElements.length).toBe(1);
+    expect(parentCueElements[0].textContent).toBe('');
+  });
 });