From 4410b2013b65cf5a445c0a8941e9c0b836dcef01 Mon Sep 17 00:00:00 2001 From: David LaPalomento Date: Thu, 4 Sep 2014 11:51:05 -0400 Subject: [PATCH 1/2] 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. --- src/js/player.js | 8 ++++++-- test/unit/mediafaker.js | 4 ++-- test/unit/player.js | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index 81fa131951..bf557de793 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -1155,7 +1155,7 @@ 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 +1167,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'); +}); From 96fddbd4c9fb34589919340db07fc692448dcea8 Mon Sep 17 00:00:00 2001 From: David LaPalomento Date: Fri, 5 Sep 2014 13:57:10 -0400 Subject: [PATCH 2/2] Fix whitespace When defining variables inline with declarations, stick to one variable per line. --- src/js/player.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/js/player.js b/src/js/player.js index bf557de793..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), errorTimeout; + var sourceTech = this.selectSource(sources), + errorTimeout; if (sourceTech) { if (sourceTech.tech === this.techName) {