From d385f8257f5cc0e77615939e37ce73c8174c4a8f Mon Sep 17 00:00:00 2001 From: David LaPalomento Date: Thu, 4 Sep 2014 11:51:05 -0400 Subject: [PATCH] Fixes #1481 Clear out pending errors on player disposal. Source selection errors are dispatched asynchronously so that there is an opportunity to override the error message. If the player is disposed during that period, the error timeout wasn't being cleared properly. Fix for #1480. Fix whitespace When defining variables inline with declarations, stick to one variable per line. --- CHANGELOG.md | 2 +- src/js/player.js | 9 +++++++-- test/unit/mediafaker.js | 4 ++-- test/unit/player.js | 17 +++++++++++++++++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8c3d780c4..f485a6b6f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ CHANGELOG ========= ## HEAD (Unreleased) -_(none)_ +* @dmlap fixed an issue where an error could be fired after player disposal ([view](https://github.com/videojs/video.js/pull/1481)) -------------------- diff --git a/src/js/player.js b/src/js/player.js index 81fa131951..f98aea2633 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -1155,7 +1155,8 @@ vjs.Player.prototype.src = function(source){ * @private */ vjs.Player.prototype.sourceList_ = function(sources){ - var sourceTech = this.selectSource(sources); + var sourceTech = this.selectSource(sources), + errorTimeout; if (sourceTech) { if (sourceTech.tech === this.techName) { @@ -1167,13 +1168,17 @@ vjs.Player.prototype.sourceList_ = function(sources){ } } else { // We need to wrap this in a timeout to give folks a chance to add error event handlers - setTimeout(vjs.bind(this, function() { + errorTimeout = setTimeout(vjs.bind(this, function() { this.error({ code: 4, message: this.localize(this.options()['notSupportedMessage']) }); }), 0); // we could not find an appropriate tech, but let's still notify the delegate that this is it // this needs a better comment about why this is needed this.triggerReady(); + + this.on('dispose', function() { + clearTimeout(errorTimeout); + }); } }; diff --git a/test/unit/mediafaker.js b/test/unit/mediafaker.js index 907bca62f9..8f422b0cf8 100644 --- a/test/unit/mediafaker.js +++ b/test/unit/mediafaker.js @@ -12,9 +12,9 @@ vjs.MediaFaker = vjs.MediaTechController.extend({ } }); -// Support everything +// Support everything except for "video/unsupported-format" vjs.MediaFaker.isSupported = function(){ return true; }; -vjs.MediaFaker.canPlaySource = function(srcObj){ return true; }; +vjs.MediaFaker.canPlaySource = function(srcObj){ return srcObj.type !== 'video/unsupported-format'; }; vjs.MediaFaker.prototype.createEl = function(){ var el = vjs.MediaTechController.prototype.createEl.call(this, 'div', { diff --git a/test/unit/player.js b/test/unit/player.js index 01d0f051bf..22d927e682 100644 --- a/test/unit/player.js +++ b/test/unit/player.js @@ -601,3 +601,20 @@ test('should honor disabled inactivity timeout', function() { clock.restore(); }); + +test('should clear pending errors on disposal', function() { + var clock = sinon.useFakeTimers(), player; + + player = PlayerTest.makePlayer(); + player.src({ + src: 'http://example.com/movie.unsupported-format', + type: 'video/unsupported-format' + }); + player.dispose(); + try { + clock.tick(5000); + } catch (e) { + return ok(!e, 'threw an error: ' + e.message); + } + ok(true, 'did not throw an error after disposal'); +});