diff --git a/src/js/player.js b/src/js/player.js index 5406a02be5..358b685983 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -2521,13 +2521,20 @@ class Player extends Component { } /** - * Add a remote text track - * - * @param {Object} options Options for remote text track - */ - addRemoteTextTrack(options) { + * Creates a remote text track object and returns an html track element. + * + * @param {Object} options The object should contain values for + * kind, language, label, and src (location of the WebVTT file) + * @param {Boolean} [manualCleanup=true] if set to false, the TextTrack will be + * automatically removed from the video element whenever the source changes + * @return {HTMLTrackElement} An Html Track Element. + * This can be an emulated {@link HTMLTrackElement} or a native one. + * @deprecated The default value of the "manualCleanup" parameter will default + * to "false" in upcoming versions of Video.js + */ + addRemoteTextTrack(options, manualCleanup) { if (this.tech_) { - return this.tech_.addRemoteTextTrack(options); + return this.tech_.addRemoteTextTrack(options, manualCleanup); } } diff --git a/src/js/tech/html5.js b/src/js/tech/html5.js index 368aa40b6f..f93489dbde 100644 --- a/src/js/tech/html5.js +++ b/src/js/tech/html5.js @@ -608,17 +608,16 @@ class Html5 extends Tech { } /** - * Creates a remote text track object and returns a html track element + * Creates either native TextTrack or an emulated TextTrack depending + * on the value of `featuresNativeTextTracks` * * @param {Object} options The object should contain values for * kind, language, label and src (location of the WebVTT file) - * @return {HTMLTrackElement} */ - addRemoteTextTrack(options = {}) { + createRemoteTextTrack(options) { if (!this.featuresNativeTextTracks) { - return super.addRemoteTextTrack(options); + return super.createRemoteTextTrack(options); } - const htmlTrackElement = document.createElement('track'); if (options.kind) { @@ -640,11 +639,25 @@ class Html5 extends Tech { htmlTrackElement.src = options.src; } - this.el().appendChild(htmlTrackElement); + return htmlTrackElement; + } + + /** + * Creates a remote text track object and returns an html track element. + * + * @param {Object} options The object should contain values for + * kind, language, label, and src (location of the WebVTT file) + * @param {Boolean} [manualCleanup=true] if set to false, the TextTrack will be + * automatically removed from the video element whenever the source changes + * @return {HTMLTrackElement} An Html Track Element. + * This can be an emulated {@link HTMLTrackElement} or a native one. + * @deprecated The default value of the "manualCleanup" parameter will default + * to "false" in upcoming versions of Video.js + */ + addRemoteTextTrack(options, manualCleanup) { + const htmlTrackElement = super.addRemoteTextTrack(options, manualCleanup); - // store HTMLTrackElement and TextTrack to remote list - this.remoteTextTrackEls().addTrackElement_(htmlTrackElement); - this.remoteTextTracks().addTrack_(htmlTrackElement.track); + this.el().appendChild(htmlTrackElement); return htmlTrackElement; } @@ -655,15 +668,7 @@ class Html5 extends Tech { * @param {TextTrackObject} track Texttrack object to remove */ removeRemoteTextTrack(track) { - if (!this.featuresNativeTextTracks) { - return super.removeRemoteTextTrack(track); - } - - const trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track); - - // remove HTMLTrackElement and TextTrack from remote list - this.remoteTextTrackEls().removeTrackElement_(trackElement); - this.remoteTextTracks().removeTrack_(track); + super.removeRemoteTextTrack(track); const tracks = this.$$('track'); diff --git a/src/js/tech/tech.js b/src/js/tech/tech.js index fe56df343e..dbbeaaee1c 100644 --- a/src/js/tech/tech.js +++ b/src/js/tech/tech.js @@ -85,9 +85,11 @@ class Tech extends Component { } if (!this.featuresNativeTextTracks) { - this.on('ready', this.emulateTextTracks); + this.emulateTextTracks(); } + this.autoRemoteTextTracks_ = new TextTrackList(); + this.initTextTrackListeners(); this.initTrackListeners(); @@ -291,6 +293,23 @@ class Tech extends Component { }); } + /** + * Remove any TextTracks added via addRemoteTextTrack that are + * flagged for automatic garbage collection + * + * @method cleanupAutoTextTracks + */ + cleanupAutoTextTracks() { + const list = this.autoRemoteTextTracks_ || []; + let i = list.length; + + while (i--) { + const track = list[i]; + + this.removeRemoteTextTrack(track); + } + } + /** * Reset the tech. Removes all sources and resets readyState. * @@ -394,17 +413,11 @@ class Tech extends Component { } /** - * Emulate texttracks + * Add vtt.js if necessary * - * @method emulateTextTracks + * @private */ - emulateTextTracks() { - const tracks = this.textTracks(); - - if (!tracks) { - return; - } - + addWebVttScript_() { if (!window.WebVTT && this.el().parentNode !== null && this.el().parentNode !== undefined) { const script = document.createElement('script'); @@ -424,6 +437,32 @@ class Tech extends Component { window.WebVTT = true; this.el().parentNode.appendChild(script); } + } + + /** + * Emulate texttracks + * + * @method emulateTextTracks + */ + emulateTextTracks() { + const tracks = this.textTracks(); + + if (!tracks) { + return; + } + + this.remoteTextTracks().on('addtrack', (e) => { + this.textTracks().addTrack_(e.track); + }); + + this.remoteTextTracks().on('removetrack', (e) => { + this.textTracks().removeTrack_(e.track); + }); + + // Initially, Tech.el_ is a child of a dummy-div wait until the Component system + // signals that the Tech is ready at which point Tech.el_ is part of the DOM + // before inserting the WebVTT script + this.on('ready', this.addWebVttScript_); const updateDisplay = () => this.trigger('texttrackchange'); const textTracksChanges = () => { @@ -527,26 +566,51 @@ class Tech extends Component { } /** - * Creates a remote text track object and returns a emulated html track element + * Create an emulated TextTrack for use by addRemoteTextTrack + * + * This is intended to be overridden by classes that inherit from + * Tech in order to create native or custom TextTracks. * * @param {Object} options The object should contain values for * kind, language, label and src (location of the WebVTT file) - * @return {HTMLTrackElement} - * @method addRemoteTextTrack */ - addRemoteTextTrack(options) { + createRemoteTextTrack(options) { const track = mergeOptions(options, { tech: this }); - const htmlTrackElement = new HTMLTrackElement(track); + return new HTMLTrackElement(track); + } + + /** + * Creates a remote text track object and returns an html track element. + * + * @param {Object} options The object should contain values for + * kind, language, label, and src (location of the WebVTT file) + * @param {Boolean} [manualCleanup=true] if set to false, the TextTrack will be + * automatically removed from the video element whenever the source changes + * @return {HTMLTrackElement} An Html Track Element. + * This can be an emulated {@link HTMLTrackElement} or a native one. + * @deprecated The default value of the "manualCleanup" parameter will default + * to "false" in upcoming versions of Video.js + */ + addRemoteTextTrack(options = {}, manualCleanup) { + const htmlTrackElement = this.createRemoteTextTrack(options); + + if (manualCleanup !== true && manualCleanup !== false) { + // deprecation warning + log.warn('Calling addRemoteTextTrack without explicitly setting the "manualCleanup" parameter to `true` is deprecated and default to `false` in future version of video.js'); + manualCleanup = true; + } // store HTMLTrackElement and TextTrack to remote list this.remoteTextTrackEls().addTrackElement_(htmlTrackElement); this.remoteTextTracks().addTrack_(htmlTrackElement.track); - // must come after remoteTextTracks() - this.textTracks().addTrack_(htmlTrackElement.track); + if (manualCleanup !== true) { + // create the TextTrackList if it doesn't exist + this.autoRemoteTextTracks_.addTrack_(htmlTrackElement.track); + } return htmlTrackElement; } @@ -558,13 +622,12 @@ class Tech extends Component { * @method removeRemoteTextTrack */ removeRemoteTextTrack(track) { - this.textTracks().removeTrack_(track); - const trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track); // remove HTMLTrackElement and TextTrack from remote list this.remoteTextTrackEls().removeTrackElement_(trackElement); this.remoteTextTracks().removeTrack_(track); + this.autoRemoteTextTracks_.removeTrack_(track); } /** @@ -685,7 +748,7 @@ Tech.prototype.featuresNativeTextTracks = false; * * ##### EXAMPLE: * - * Tech.withSourceHandlers.call(MyTech); + * Tech.withSourceHandlers(MyTech); * */ Tech.withSourceHandlers = function(_Tech) { @@ -820,17 +883,7 @@ Tech.withSourceHandlers = function(_Tech) { this.disposeSourceHandler(); this.off('dispose', this.disposeSourceHandler); - // if we have a source and get another one - // then we are loading something new - // than clear all of our current tracks - if (this.currentSource_) { - this.clearTracks(['audio', 'video']); - - this.currentSource_ = null; - } - if (sh !== _Tech.nativeSourceHandler) { - this.currentSource_ = source; // Catch if someone replaced the src without calling setSource. @@ -838,7 +891,6 @@ Tech.withSourceHandlers = function(_Tech) { this.off(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_); this.off(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_); this.one(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_); - } this.sourceHandler_ = sh.handleSource(source, this, this.options_); @@ -854,7 +906,6 @@ Tech.withSourceHandlers = function(_Tech) { // On successive loadstarts when setSource has not been called again _Tech.prototype.successiveLoadStartListener_ = function() { - this.currentSource_ = null; this.disposeSourceHandler(); this.one(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_); }; @@ -863,10 +914,25 @@ Tech.withSourceHandlers = function(_Tech) { * Clean up any existing source handler */ _Tech.prototype.disposeSourceHandler = function() { - if (this.sourceHandler_ && this.sourceHandler_.dispose) { + // if we have a source and get another one + // then we are loading something new + // than clear all of our current tracks + if (this.currentSource_) { + this.clearTracks(['audio', 'video']); + this.currentSource_ = null; + } + + // always clean up auto-text tracks + this.cleanupAutoTextTracks(); + + if (this.sourceHandler_) { this.off(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_); this.off(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_); - this.sourceHandler_.dispose(); + + if (this.sourceHandler_.dispose) { + this.sourceHandler_.dispose(); + } + this.sourceHandler_ = null; } }; diff --git a/test/unit/tech/tech.test.js b/test/unit/tech/tech.test.js index f5edd7f5a6..72ea255644 100644 --- a/test/unit/tech/tech.test.js +++ b/test/unit/tech/tech.test.js @@ -146,7 +146,7 @@ QUnit.test('dispose() should clear all tracks that are added after creation', fu assert.equal(tech.audioTracks().length, 2, 'should have two audio tracks at the start'); assert.equal(tech.videoTracks().length, 2, 'should have two video tracks at the start'); - assert.equal(tech.textTracks().length, 2, 'should have two video tracks at the start'); + assert.equal(tech.textTracks().length, 2, 'should have two text tracks at the start'); assert.equal(tech.remoteTextTrackEls().length, 2, 'should have two remote text tracks els'); @@ -167,6 +167,62 @@ QUnit.test('dispose() should clear all tracks that are added after creation', fu assert.equal(tech.textTracks().length, 0, 'should have zero video tracks after dispose'); }); +QUnit.test('switching sources should clear all remote tracks that are added with manualCleanup = false', function(assert) { + // Define a new tech class + const MyTech = extendFn(Tech); + + // Create source handler + const handler = { + canPlayType: () => 'probably', + canHandleSource: () => 'probably', + handleSource: () => { + return { + dispose: () => {} + }; + } + }; + + // Extend Tech with source handlers + Tech.withSourceHandlers(MyTech); + + MyTech.registerSourceHandler(handler); + + const tech = new MyTech(); + + // set the initial source + tech.setSource({src: 'foo.mp4', type: 'mp4'}); + + // default value for manualCleanup is true + tech.addRemoteTextTrack({}); + // should be automatically cleaned up when source changes + tech.addRemoteTextTrack({}, false); + + assert.equal(tech.textTracks().length, 2, 'should have two text tracks at the start'); + assert.equal(tech.remoteTextTrackEls().length, + 2, + 'should have two remote text tracks els'); + assert.equal(tech.remoteTextTracks().length, 2, 'should have two remote text tracks'); + assert.equal(tech.autoRemoteTextTracks_.length, + 1, + 'should have one auto-cleanup remote text track'); + + // change source to force cleanup of auto remote text tracks + tech.setSource({src: 'bar.mp4', type: 'mp4'}); + + assert.equal(tech.textTracks().length, + 1, + 'should have one text track after source change'); + assert.equal(tech.remoteTextTrackEls().length, + 1, + 'should have one remote remote text track els after source change'); + assert.equal(tech.remoteTextTracks().length, + 1, + 'should have one remote text track after source change'); + assert.equal(tech.autoRemoteTextTracks_.length, + 0, + 'should have zero auto-cleanup remote text tracks'); +}); + QUnit.test('should add the source handler interface to a tech', function(assert) { const sourceA = { src: 'foo.mp4', type: 'video/mp4' }; const sourceB = { src: 'no-support', type: 'no-support' }; diff --git a/test/unit/tracks/text-tracks.test.js b/test/unit/tracks/text-tracks.test.js index fd7094c02c..df53357535 100644 --- a/test/unit/tracks/text-tracks.test.js +++ b/test/unit/tracks/text-tracks.test.js @@ -377,7 +377,6 @@ QUnit.test('removes cuechange event when text track is hidden for emulated track endTime: 5 }); player.tech_.textTracks().addTrack_(tt); - player.tech_.emulateTextTracks(); let numTextTrackChanges = 0;