Skip to content

Commit

Permalink
Fixes #1481
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dmlap authored and heff committed Sep 5, 2014
1 parent 88f59d7 commit d385f82
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

--------------------

Expand Down
9 changes: 7 additions & 2 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
});
}
};

Expand Down
4 changes: 2 additions & 2 deletions test/unit/mediafaker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
17 changes: 17 additions & 0 deletions test/unit/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

0 comments on commit d385f82

Please sign in to comment.