diff --git a/src/js/tech/tech.js b/src/js/tech/tech.js index fe56df343e..5fa48ddf20 100644 --- a/src/js/tech/tech.js +++ b/src/js/tech/tech.js @@ -291,6 +291,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. * @@ -531,20 +548,33 @@ class Tech extends Component { * * @param {Object} options The object should contain values for * kind, language, label and src (location of the WebVTT file) + * @param {Boolean} manualCleanup if set to false, the TextTrack will be + * automatically removed from the video element whenever the source changes * @return {HTMLTrackElement} * @method addRemoteTextTrack */ - addRemoteTextTrack(options) { + addRemoteTextTrack(options, manualCleanup) { const track = mergeOptions(options, { tech: this }); - const htmlTrackElement = new HTMLTrackElement(track); + 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); + if (manualCleanup !== true) { + // create the TextTrackList if it doesn't exist + this.autoRemoteTextTracks_ = this.autoRemoteTextTracks_ || new TextTrackList(); + this.autoRemoteTextTracks_.addTrack_(htmlTrackElement.track); + } + // must come after remoteTextTracks() this.textTracks().addTrack_(htmlTrackElement.track); @@ -565,6 +595,9 @@ class Tech extends Component { // remove HTMLTrackElement and TextTrack from remote list this.remoteTextTrackEls().removeTrackElement_(trackElement); this.remoteTextTracks().removeTrack_(track); + if (this.autoRemoteTextTracks_) { + this.autoRemoteTextTracks_.removeTrack_(track); + } } /** @@ -820,17 +853,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 +861,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 +876,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 +884,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' };