Skip to content
New issue

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

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

Already on GitHub? # to your account

Add failsafe to get duration during timeupdate #20

Merged
merged 1 commit into from
Aug 13, 2021

Conversation

IllyaMoskvin
Copy link
Contributor

We noticed that sometimes, unpredictably, the 25-50-75 percentile events would not fire. We could not figure out a reliable way to reproduce the error, but if you add a breakpoint around line 55 here:

player.on('timeupdate', function() {
const curTime = +player.currentTime().toFixed(0);
const data = {
seekCount,
pauseCount,
currentTime: curTime,
duration
};
switch (curTime) {
case first:
first = false;
player.trigger('tracking:first-quarter', data);
break;
case second:
second = false;
player.trigger('tracking:second-quarter', data);
break;
case third:
third = false;
player.trigger('tracking:third-quarter', data);
break;
}
});

...and repeatedly refresh the page while trying to play a video.js player with these events bound, eventually, you may run into a situation where duration is equal to 0.

It seems that either the durationchange event never fired, or it fired before the watcher got bound. In either case, for whatever reason, the durationchange callback here:

player.on('durationchange', function() {
duration = +player.duration().toFixed(0);
if (duration > 0) {
const quarter = (duration / 4).toFixed(0);
first = +quarter;
second = +quarter * 2;
third = +quarter * 3;
}
});
};

...never ran in those bugged-out instances, so the first, second, and third variables never got set.

(I'm guessing this could be a browser bug? We are using Chrome 92 on macOS.)

This pull request aims to fix that issue by adding an additional check for duration during the timeupdate event.

I confirmed that this fix works by adding a breakpoint on the getDuration() call inside timeupdate. By doing the same refresh-page-and-hit-play, I was able to confirm that the code at the breakpoint does occasionally get called (i.e. when it would have broken before), and the percentile events do fire as expected.

Sometimes it appears that `durationchange` either doesn't trigger, or
triggers before the `durationchange` watcher gets bound. This results
in the 25-50-75 events never firing.
@spodlecki
Copy link
Owner

awesome catch! I'm not sure how this actually happens, but I love the solution

@spodlecki spodlecki merged commit 330fbf7 into spodlecki:master Aug 13, 2021
@IllyaMoskvin
Copy link
Contributor Author

Sweet! Thank you for the merge! Would you be down to publish a new version to the npm registry?

@spodlecki
Copy link
Owner

published 1.0.2 - all set. Thank you for the contribution

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants