From 4081434eba7f90ea7fe8544665baf99f59ec5863 Mon Sep 17 00:00:00 2001
From: David HM Morgan <37144605+david-hm-morgan@users.noreply.github.com>
Date: Wed, 7 Dec 2022 22:23:12 +0000
Subject: [PATCH] fix: Fix subtitles not added to DOM region (#4733)

Reinstate previously removed region elements when next caption finds it
is not there: Detect the absence and ensure `updateDOM` is true so its
reinstated and the next caption can be shown.

Closes #4680

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
---
 lib/text/ui_text_displayer.js       | 17 +++++++
 test/text/ui_text_displayer_unit.js | 76 +++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js
index f88d9e49fa..4678dcb92c 100644
--- a/lib/text/ui_text_displayer.js
+++ b/lib/text/ui_text_displayer.js
@@ -199,6 +199,19 @@ shaka.text.UITextDisplayer = class {
     this.isTextVisible_ = on;
   }
 
+  /**
+   * @private
+   */
+  isElementUnderTextContainer_(elemToCheck) {
+    while (elemToCheck != null) {
+      if (elemToCheck == this.textContainer_) {
+        return true;
+      }
+      elemToCheck = elemToCheck.parentElement;
+    }
+    return false;
+  }
+
   /**
    * @param {!Array.<!shaka.extern.Cue>} cues
    * @param {!HTMLElement} container
@@ -260,6 +273,9 @@ shaka.text.UITextDisplayer = class {
           cueRegistry = this.currentCuesMap_.get(cue);
           wrapper = cueRegistry.wrapper;
           updateDOM = true;
+        } else if (!this.isElementUnderTextContainer_(wrapper)) {
+          // We found that the wrapper needs to be in the DOM
+          updateDOM = true;
         }
       }
 
@@ -276,6 +292,7 @@ shaka.text.UITextDisplayer = class {
       const topCue = parents.pop();
       goog.asserts.assert(topCue == cue, 'Parent cues should be kept in order');
     }
+
     if (updateDOM) {
       for (const element of toUproot) {
         // NOTE: Because we uproot shared region elements, too, we might hit an
diff --git a/test/text/ui_text_displayer_unit.js b/test/text/ui_text_displayer_unit.js
index 306cd45f7c..5b3e0678f3 100644
--- a/test/text/ui_text_displayer_unit.js
+++ b/test/text/ui_text_displayer_unit.js
@@ -461,4 +461,80 @@ describe('UITextDisplayer', () => {
       expect(Object.keys(cueCssObj)).not.toContain('left');
     }
   });
+
+  it('does not lose second item in a region', () => {
+    const cueRegion = new shaka.text.CueRegion();
+    cueRegion.id = 'regionId';
+    cueRegion.height = 80;
+    cueRegion.heightUnits = shaka.text.CueRegion.units.PERCENTAGE;
+    cueRegion.width = 80;
+    cueRegion.widthUnits = shaka.text.CueRegion.units.PERCENTAGE;
+    cueRegion.viewportAnchorX = 10;
+    cueRegion.viewportAnchorY = 10;
+    cueRegion.viewportAnchorUnits = shaka.text.CueRegion.units.PERCENTAGE;
+
+    // These have identical nested.
+    const cue1 = new shaka.text.Cue(168, 181.84, '');
+    cue1.nestedCues = [
+      new shaka.text.Cue(168, 181.84, ''),
+    ];
+    cue1.region = cueRegion;
+
+    const nested1 = new shaka.text.Cue(168, 170.92, '');
+    nested1.nestedCues = [new shaka.text.Cue(0, 170.92,
+        'Emo look. I mean listen.')];
+
+    const nested2 = new shaka.text.Cue(172, 174.84, '');
+    nested2.nestedCues = [new shaka.text.Cue(172, 174.84,
+        'You have to learn to listen.')];
+
+    const nested3 = new shaka.text.Cue(175.84, 177.64, '');
+    nested3.nestedCues = [new shaka.text.Cue(175.84, 177.64,
+        'This is not some game.')];
+
+    const nested4 = new shaka.text.Cue(177.68, 181.84, '');
+    nested4.nestedCues = [new shaka.text.Cue(177.68, 181.84,
+        'You - I mean we - we could easily die out here.')];
+
+    cue1.nestedCues[0].nestedCues = [nested1, nested2, nested3, nested4];
+
+    video.currentTime = 170;
+    textDisplayer.setTextVisibility(true);
+    textDisplayer.append([cue1]);
+    updateCaptions();
+
+    /** @type {Element} */
+    const textContainer = videoContainer.querySelector('.shaka-text-container');
+    let captions = textContainer.querySelectorAll('div');
+    expect(captions.length).toBe(1);
+    let allRegionElements = textContainer.querySelectorAll(
+        '.shaka-text-region');
+    // Verify that the nested cues are all attached to a single region element.
+    expect(allRegionElements.length).toBe(1);
+
+    // Advance time to where there is none to show
+    video.currentTime = 171;
+    updateCaptions();
+
+    allRegionElements = textContainer.querySelectorAll(
+        '.shaka-text-region');
+    expect(allRegionElements.length).toBe(1);
+
+    // Advance time to where there is something to show
+    video.currentTime = 173;
+    updateCaptions();
+
+    allRegionElements = textContainer.querySelectorAll(
+        '.shaka-text-region');
+    expect(allRegionElements.length).toBe(1);
+
+    captions = textContainer.querySelectorAll('div');
+
+    expect(captions.length).toBe(1);
+    expect(captions[0].textContent).toBe('You have to learn to listen.');
+
+    allRegionElements = textContainer.querySelectorAll(
+        '.shaka-text-region');
+    expect(allRegionElements.length).toBe(1);
+  });
 });