Fix the double loadstart event and fix it good #2605
Conversation
| // watch for that also. | ||
| let loadstartFired = false; | ||
| let setLoadstartFired = function() { loadstartFired = true; }; | ||
| this.on('loadstart', setLoadstartFired); |
There was a problem hiding this comment.
maybe this.one?
Also, probably would've been a lot simpler with promises :P
There was a problem hiding this comment.
one seemed unnecessary since we're always removing it on ready anyway. I guess it felt better to clean it up more manually, since not cleaning it up always would have really bad side effects.
|
@misteroneill would you be able to verify this? (Sorry @pat, end of a long day) |
|
@heff no worries, it happens :) |
|
After attempting to verify: if you bind a listener before the player is ready, both "loadstart" and "ready" are fired twice. If you bind a listener on ready, both are fired once. Here's the test code I used: var vid = document.getElementById("vid1");
var player = videojs(vid).ready(function() {
console.log('ready callback!');
});
player.on([
'loadstart',
'ready'
], function(event) {
console.log(event.type);
});Results in: |
|
Thanks @misteroneill, these should be fixed now, if you get another second to try it. We had double ready triggers in the player. Not exactly sure how that happened. |
|
@heff Yeah, looking good now. 👍 |
- Fixed some issue comments - Fixed a double ready event closes videojs#2605 fixes videojs#2588
|
Thanks @misteroneill! |
fixes #2588 @pat can you confirm?
Took me a day and half to fully wrap my brain around this. The code comments give some insight into the complexity.
With these changes you can now init a player on a video element of any loading state and the player will emit all the loading events, from loadstart to canplaythrough, to catch up to the current state.