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

video.min.js errors in IE8, must use video.js #3064

Closed
sethborg opened this issue Feb 2, 2016 · 15 comments
Closed

video.min.js errors in IE8, must use video.js #3064

sethborg opened this issue Feb 2, 2016 · 15 comments
Assignees

Comments

@sethborg
Copy link
Contributor

sethborg commented Feb 2, 2016

Something's busted about video.min.js since it breaks IE8 support. Could someone look into this or if you aren't going to fix it, at least update the docs with an html comment in the source code so people know

Use the below to see the error, change video.min.js to video.js and see how it works fine

<!DOCTYPE html>
<html lang="en">
<head>
<link href="http://vjs.zencdn.net/5.5.3/video-js.css" rel="stylesheet">
<script src="http://vjs.zencdn.net/ie8/1.1.1/videojs-ie8.min.js" ></script>
<!-- must use video.js, IE8 errors if you use video.min.js -->
<script src="http://vjs.zencdn.net/5.5.3/video.min.js" ></script>
</head>

<body>
<video controls="controls" class="video-js vjs-fluid vjs-default-skin vjs-big-play-centered" preload="auto" data-setup='{}'>
<source src="http://clips.vorwaerts-gmbh.de/VfE_html5.mp4" type="video/mp4">
<p class="vjs-no-js">
To view this video please enable JavaScript, and consider upgrading to a web browser that
<a href="http://videojs.com/html5-video-support/" target="_blank">supports HTML5 video</a>
</p>
</video>
</body>
</html>

@gkatsev
Copy link
Member

gkatsev commented Feb 2, 2016

Can you elaborate on what you mean by ie8 being broken? Are there any errors in the console? What behavior are you seeing and what behavior are you expecting to see?
Thanks.

@sethborg
Copy link
Contributor Author

sethborg commented Feb 2, 2016

Yeah console error and the Flash video player never loads because of it

@gkatsev
Copy link
Member

gkatsev commented Feb 2, 2016

What error are you seeing?

@sethborg
Copy link
Contributor Author

sethborg commented Feb 2, 2016

Nothing useful, that's why I didn't include it:

Script error
video.min.js
Code: 0
URI: http://vjs.zencdn.net/5.5.3/video.min.js
Line: 0
Char: 0

@gkatsev
Copy link
Member

gkatsev commented Feb 2, 2016

Can't know unless I ask. A lot of times errors do contain useful information but the people reporting the issue don't think it's useful so they don't provide it. It's generally best to provide such information even if at first glance it seems not useful.
Thanks, we'll investigate.

@kingprimex
Copy link

Have you tried to use compatibility mode set to IE9? For example, JWPlayer 7 flash source not working on IE8 or lower! I suspect the same behaviour with VideoJS
P.S.1 Right click -> Inspect Element.
P.S.2 More on the issue about selecting different compatibility mode here: http://stackoverflow.com/questions/14611264/x-ua-compatible-content-ie-9-ie-8-ie-7-ie-edge
videojs

@sethborg
Copy link
Contributor Author

sethborg commented Feb 3, 2016

I tried adding <meta http-equiv="X-UA-Compatible" content="IE=Edge" /> to the page but it didn't change anything. Probably because that applies to the DOM not javascript.

There's some issue with your minimizing function that makes it incompatible with IE8.

@sethborg
Copy link
Contributor Author

sethborg commented Jun 6, 2016

I just tested this in IE8 on 5.10.0 and this bug still exists. See also #3142

What I've found is that I must use video.js not video.min.js if I'm using javascript initialization for the video player or it errors on line 13611 which I've marked below.

Here are all the test cases:

http://ivcmiami.com/testvideojs/min_js_setup.htm
http://ivcmiami.com/testvideojs/min_data_setup.htm
http://ivcmiami.com/testvideojs/no_min_js_setup.htm
http://ivcmiami.com/testvideojs/no_min_data_setup.htm

  // Check if any media elements exist
  if (mediaEls && mediaEls.length > 0) {

    for (var i = 0, e = mediaEls.length; i < e; i++) {
      var mediaEl = mediaEls[i];

      // Check if element exists, has getAttribute func.
      // IE seems to consider typeof el.getAttribute == 'object' instead of 'function' like expected, at least when loading the player immediately.
      if (mediaEl && mediaEl.getAttribute) {

        // Make sure this player hasn't already been set up.
        if (mediaEl['player'] === undefined) {
          var options = mediaEl.getAttribute('data-setup');

          // Check if data-setup attr exists.
          // We only auto-setup if they've added the data-setup attr.
          if (options !== null) {
            // Create new video.js instance.
            var player = videojs(mediaEl);     <- IT ERRORS HERE
          }
        }

        // If getAttribute isn't defined, we need to wait for the DOM.
      } else {
          autoSetupTimeout(1);
          break;
        }
    }

    // No videos were found, so keep looping unless page is finished loading.
  } else if (!_windowLoaded) {
      autoSetupTimeout(1);
    }

@gkatsev
Copy link
Member

gkatsev commented Jun 6, 2016

Have a reduced test case? When I was testing the fix (#3233), it was working fine for me in IE8 as well.

@sethborg
Copy link
Contributor Author

sethborg commented Jun 6, 2016

@gkatsev
Copy link
Member

gkatsev commented Jun 6, 2016

@sethborg looks like the minified version is erroring out when you pass in a player id as a string to it. This is actually occurring because the minifier is doing weird stuff that is causing issues.
specially, it turns the main videojs function from (simplified):

var videojs = function(id, options, ready) {
   return videojs.getPlayers()[id];

to something like

var videojs = function vjs(id, options, ready) {
  return vjs.getPlayers()[id];

However, in IE8, vjs and videojs are not equal inside the function and thus it doesn't have the propery getPlayers that we expect.
Reopening and investigating why our minifier produces this code since it is also technically 2 characters longer than necessary.

@gkatsev gkatsev reopened this Jun 6, 2016
@gkatsev
Copy link
Member

gkatsev commented Jun 6, 2016

Also, interestingly enough, if you pass in an element reference instead of a string id, it works just fine. I guess because it doesn't enter the main block that does videojs.getPlayers()

@gkatsev
Copy link
Member

gkatsev commented Jun 6, 2016

Looks like the issue isn't uglify but rather that babel produces a named function expression that then uglify changes the names of.

gkatsev added a commit that referenced this issue Jun 6, 2016
This is a fix for IE8 where the name of a named function
expression is different from the variable that function is
assigned to. This is an issue because our version of babel outputs
functions like that.
Fixes #3064 again.
@gkatsev
Copy link
Member

gkatsev commented Jun 7, 2016

Fixed in 5.10.5.

@gkatsev gkatsev closed this as completed Jun 7, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@gkatsev @kingprimex @sethborg and others