diff --git a/lib/text/ui_text_displayer.js b/lib/text/ui_text_displayer.js index 76e28021be..051ea1b16d 100644 --- a/lib/text/ui_text_displayer.js +++ b/lib/text/ui_text_displayer.js @@ -73,7 +73,14 @@ shaka.text.UITextDisplayer = class { this.updateCaptions_(); }).tickEvery(updatePeriod); - /** private {Map.} */ + /** + * 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.} + */ this.currentCuesMap_ = new Map(); /** @private {shaka.util.EventManager} */ @@ -185,68 +192,143 @@ shaka.text.UITextDisplayer = class { } /** - * Display the current captions. - * @param {boolean=} forceUpdate + * @param {!Array.} cues + * @param {?HTMLElement} container + * @param {number} currentTime + * @param {boolean} isNested * @private */ - updateCaptions_(forceUpdate = false) { - const currentTime = this.video_.currentTime; + 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.} + */ + 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.} + */ + 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; + } + } - // 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; - }; + if (shouldBeDisplayed) { + toPlant.push(cue); + if (!cueRegistry) { + // The cue has to be made! + this.createCue_(cue, isNested); + cueRegistry = this.currentCuesMap_.get(cue); + updateDOM = true; + } + } - // 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); + // 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); } } - - // 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); + // 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); } } + } - // 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; + /** + * Display the current captions. + * @param {boolean=} forceUpdate + * @private + */ + updateCaptions_(forceUpdate = false) { + if (!this.textContainer_) { + return; + } + + const currentTime = this.video_.currentTime; + if (!this.isTextVisible_ || forceUpdate) { + if (this.currentCuesMap_.size > 0) { + // Clear away any existing cues. + shaka.util.Dom.removeAllChildren(this.textContainer_); + this.currentCuesMap_.clear(); + } + } + 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)); } - }); - for (const cue of currentCues) { - const cueElement = this.displayCue_( - this.textContainer_, cue, /* isNested= */ false); - this.currentCuesMap_.set(cue, cueElement); + // 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); + } + } + } } } /** - * 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}); } /** diff --git a/test/text/ui_text_displayer_unit.js b/test/text/ui_text_displayer_unit.js index e51f2eb658..cf1d186343 100644 --- a/test/text/ui_text_displayer_unit.js +++ b/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(''); + }); });