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

Invoke the loadstart handler manually if the tech already triggered the ... #1208

Closed
wants to merge 1 commit into from

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented May 14, 2014

...event. Fixes #1207.

Check if the player already has a currentSrc on load and call the loadstart handler explicitly. This ensures that the first play listener is properly registered before playback begins.

@heff, @seniorflexdeveloper: This deserves some tests but I wanted to open the PR to get some feedback.

…he event. Fixes videojs#1207.

Check if the player already has a currentSrc on load and call the loadstart handler explicitly. This ensures that the first play listener is properly registered before playback begins.
@heff
Copy link
Member

heff commented May 14, 2014

This was reported in #1193 also.

I'm trying to understand why this isn't fixed by this line:
https://github.com/videojs/video.js/blob/master/src/js/media/html5.js#L32

I think there's a deeper issue here and I don't think I understand it fully yet. I'm wondering if this would be fixed by making the ready event smarter on the player. Right now the techs just trigger the ready event on the player when they're ready, but it's possible that should only be triggered after:

  • the tech is ready
  • the player init has finished running
  • the player's children have been added
    That may have nothing to do with this specific issue though...

@gkatsev
Copy link
Member

gkatsev commented May 14, 2014

I'm trying to understand why this isn't fixed by this line:
https://github.com/videojs/video.js/blob/master/src/js/media/html5.js#L32

Maybe the event listener still hasn't been added at that point?

@heff
Copy link
Member

heff commented May 14, 2014

@gkatsev, yeah it could simply be that we need to move the event listener attaching before the Component init. Need to set up a reduced test case for this.

heff added a commit to heff/video.js that referenced this pull request May 14, 2014
@heff heff closed this in bad5130 May 14, 2014
@heff
Copy link
Member

heff commented May 14, 2014

Fixed and in master. Please try it out.

@dmlap dmlap deleted the hotfix/no-has-started branch May 15, 2014 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vjs-has-started not being added on init
3 participants