Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ignore progress events #143

Merged
merged 2 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const defaults = {
message: '',
timeout: 45 * 1000,
dismiss: defaultDismiss,
progressDisabled: false,
errors: {
'1': {
type: 'MEDIA_ERR_ABORTED',
Expand Down Expand Up @@ -194,10 +193,6 @@ const initPlugin = function(player, options) {
resetMonitor();
}
});

if (!options.progressDisabled) {
healthcheck('progress', resetMonitor);
}
};

const onPlayNoSource = function() {
Expand Down Expand Up @@ -309,10 +304,9 @@ const initPlugin = function(player, options) {
}
};

reInitPlugin.disableProgress = function(disabled) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be a breaking change. It probably should just be a no-op now.

options.progressDisabled = disabled;
onPlayStartMonitor();
};
// no-op API
// TODO: remove in a major version
reInitPlugin.disableProgress = () => {};

player.on('play', onPlayStartMonitor);
player.on('play', onPlayNoSource);
Expand All @@ -338,7 +332,7 @@ const errors = function(options) {
initPlugin(this, videojs.mergeOptions(defaults, options));
};

['extend', 'getAll', 'disableProgress'].forEach(k => {
['extend', 'getAll'].forEach(k => {
errors[k] = function() {
videojs.log.warn(
`The errors.${k}() method is not available until the plugin has been initialized!`
Expand Down
27 changes: 3 additions & 24 deletions test/plugin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,9 @@ QUnit.test('no progress for 45 seconds is an error', function(assert) {
assert.strictEqual(this.player.error().type, 'PLAYER_ERR_TIMEOUT');
});

QUnit.test('when progress watching is disabled, progress within 45 seconds is an error', function(assert) {
QUnit.test('progress events are ignored during timeout', function(assert) {
let errors = 0;

this.player.errors.disableProgress(true);

this.player.on('error', function() {
errors++;
});
Expand All @@ -155,8 +153,6 @@ QUnit.test('when progress watching is disabled, progress within 45 seconds is an
assert.strictEqual(errors, 1, 'emitted an error');
assert.strictEqual(this.player.error().code, -2, 'error code is -2');
assert.strictEqual(this.player.error().type, 'PLAYER_ERR_TIMEOUT');

this.player.errors.disableProgress(false);
});

QUnit.test('Flash API is unavailable when using Flash is an error', function(assert) {
Expand Down Expand Up @@ -217,7 +213,7 @@ QUnit.test('when dispose is triggered should not throw error ', function(assert)
this.player = videojs(this.video);
});

QUnit.test('progress clears player timeout errors', function(assert) {
QUnit.test('progress does not clear player timeout errors', function(assert) {
let errors = 0;

this.player.on('error', function() {
Expand All @@ -233,7 +229,7 @@ QUnit.test('progress clears player timeout errors', function(assert) {
assert.strictEqual(this.player.error().type, 'PLAYER_ERR_TIMEOUT');

this.player.trigger('progress');
assert.strictEqual(this.player.error(), null, 'error removed');
assert.strictEqual(this.player.error().code, -2, 'error code is -2');
});

QUnit.test('reinitialising plugin during playback starts timeout handler', function(assert) {
Expand Down Expand Up @@ -308,23 +304,6 @@ QUnit.test('timing out multiple times only throws a single error', function(asse
assert.strictEqual(errors, 1, 'only one error fired');
});

QUnit.test('progress events while playing reset the player timeout', function(assert) {
let errors = 0;

this.player.on('error', function() {
errors++;
});
this.player.src(sources);
this.player.trigger('play');
// stalled for awhile
this.clock.tick(44 * 1000);
// but playback resumes!
this.player.trigger('progress');
this.clock.tick(44 * 1000);

assert.strictEqual(errors, 0, 'no errors emitted');
});

QUnit.test('no signs of playback triggers a player timeout', function(assert) {
let errors = 0;

Expand Down