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

5.0.0-rc.12 - wait for components to register before autoSetup #2341

Open
eXon opened this issue Jul 11, 2015 · 8 comments
Open

5.0.0-rc.12 - wait for components to register before autoSetup #2341

eXon opened this issue Jul 11, 2015 · 8 comments
Labels
pinned Things that stalebot shouldn't close automatically

Comments

@eXon
Copy link
Contributor

eXon commented Jul 11, 2015

Video.js doesn't wait on the page to be loaded before doing the setup on the videojs instances (video with data-setup).

Thus, it doesn't wait for my Youtube component to register and if you are unlucky (or fake a really slow network), it will throw this error:

VIDEOJS: ERROR: The "Youtube" tech is undefined. Skipped browser support check for that tech._logType @ log.js:79115.log.error @ log.js:31selectSource @ player.js:1655sourceList_ @ player.js:1770src @ player.js:1717MediaLoader @ loader.js:48addChild @ component.js:392handleAdd @ component.js:528(anonymous function) @ component.js:546(anonymous function) @ component.js:546initChildren @ component.js:46Player @ player.js:181videojs @ video.js:94autoSetup @ setup.js:65
log.js:79 VIDEOJS: ERROR: (CODE:4 MEDIA_ERR_SRC_NOT_SUPPORTED) No compatible source was found for this video. MediaError {code: 4, message: "No compatible source was found for this video."}

Here is how I am doing it:

<!DOCTYPE html>
<html>
<head>
  <link type="text/css" rel="stylesheet" href="../node_modules/video.js/dist/video-js.min.css" />

  <script async src="../node_modules/video.js/dist/video.min.js"></script>
  <script async src="../src/Youtube.js"></script>
</head>
<body>
  <video
    id="vid1"
    class="video-js vjs-default-skin"
    controls
    autoplay
    loop
    preload="auto"
    width="640" height="264"
    data-setup='{ "techOrder": ["youtube"], "sources": [{ "type": "video/youtube", "src": "https://www.youtube.com/watch?v=xjS6SftYQaQ"}] }'
  >
  </video>
</body>
</html>

The async is very important to make sure the loading order is intact (or it will throw videojs undefined). If I put my scripts at the end of the page, I have the same problem.

I am not very familiar with the way the autosetup work. Should we wait for the document ready state before? It might be necessary.

What do you guys think?

@heff
Copy link
Member

heff commented Jul 14, 2015

If you remove async from both script tags it should work. Alternatively you could not use the data-setup attribute and instead use the videojs() function to init the player.

I'm not sure if there's an easy way we can make the data-setup magic smarter, or if it's worth it. Does document ready wait for async scripts?

@eXon
Copy link
Contributor Author

eXon commented Jul 14, 2015

If I remove async (in the head), I get videojs is undefined (they all load in parallel). If I have the async attribute (or at the end of the page), the autoSetup is triggered before the component is registred. I'm not sure what more we can do outside waiting for the entire page to be loaded. This is a major problem.

Maybe we can wait for the page to be ready before loading the tech? I don't know.

@heff
Copy link
Member

heff commented Jul 14, 2015

If I remove async (in the head), I get videojs is undefined (they all load in parallel).

That doesn't seem right to me. By definition <script> files are executed in order. They may load in parallel but the browsers should run them in the order they're defined in the page. This jsbin example works at least.
http://jsbin.com/vobola/edit?html,console,output

I think we do need a better system for including plugins in the load process though. Some way of informing data-setup to wait. Either that or deprecating the data-setup method.

@qpSHiNqp
Copy link

The same issue to my case, and the example doesn't work at least on Mac OS X Chrome 47.0.2526.80 (64-bit), iOS 9.2 Safari and Mac OS X Safari ver. 9.0.1 (10601.2.7.2).

This jsbin example works at least.
http://jsbin.com/vobola/edit?html,console,output

Confirmed the issue also with videojs 4.12.11.

I guess, because video.js execution / evaluation takes much time, autoSetupTimeout comes before the parser reaches Youtube tech and the tech is registered, though scripts are non-async and they are executed in-order.

I need a workaround for this issue but I don't want the data-setup method to be deprecated because initializing with videojs() method is a little complicated for non-specialized engineers.

@hartman
Copy link
Contributor

hartman commented Dec 10, 2015

@qpSHiNqp This is most likely simply because your resources are not present. the naming of all these things have changed over time quite often.

The following just works for me: 4.12.15 and youtube tech plugin v1.2.13
http://jsbin.com/xupeciwera/1/edit?html,console,output

And here an updated jsbin with 5.4.4 (prerelease) and youtube tech 2.0.3
http://jsbin.com/peyucoliwa/1/edit?html,console,output

@qpSHiNqp
Copy link

@hartman Oh, I wasn't aware of that! Thank you and I confirmed those examples work fine.
However, in my case this still matters though my plugin resources are certainly present and loaded slightly after videojs main component.

@apatinoyu
Copy link

Did Someone find a solution?

@hartman
Copy link
Contributor

hartman commented Aug 18, 2016

If you need to do async loading, then you should also do the tracking to make sure those components are loaded and executed in order and then use the videojs() setup function.

This is usually where AMD and CommonJS come into the picture. Without those technologies or ones like them, you should usually not use the async attribute on your scripts.

There is a very nice article by Jake Archibald on html5rocks that gives lots of background about how script loading works.

@gkatsev gkatsev added the pinned Things that stalebot shouldn't close automatically label Jul 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Things that stalebot shouldn't close automatically
Projects
None yet
Development

No branches or pull requests

6 participants