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.

Sign up for GitHub

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

Jump to bottom

HTML5 tech preload property is lost on minify #2191

Closed
geniusdex opened this issue May 22, 2015 · 5 comments
Closed

HTML5 tech preload property is lost on minify #2191

geniusdex opened this issue May 22, 2015 · 5 comments

Comments

@geniusdex
Copy link

What did you do? (steps to reproduce)

When attempting to call the preload() method on a player using the HTML5 tech, the method throws an error if a minified version of the source is loaded:

VIDEOJS: Video.js: preload method not defined for Html5 playback technology.

This is triggered by the following document:

<!DOCTYPE html>
<html>
    <head>
        <link href="//vjs.zencdn.net/4.12/video-js.css" rel="stylesheet">
        <script type="text/javascript" src="build/files/minified.video.js"></script>
    </head>
    <body>
        <video id="example_video_1" class="video-js vjs-default-skin" controls preload="none" width="640" height="264" poster="http://video-js.zencoder.com/oceans-clip.png">
            <source src="http://video-js.zencoder.com/oceans-clip.mp4" type='video/mp4' />
        </video>
    </body>
    <script type="text/javascript">
videojs('example_video_1', {
}, function()
{
    console.log(this.preload());
});
    </script>
</html>

What happened? (actual results)

The console log shows an error:

VIDEOJS: Video.js: preload method not defined for Html5 playback technology. TypeError: undefined is not a function {stack: (...), message: "undefined is not a function"}

What should have happened? (expected results)

The console log shows the correct preload value:

none

Video.js version (e.g. v4.3.0)

v4.12.7 (current stable)

Video.js plugins used (e.g. videojs-youtube)

None

Browsers + platforms where this is happening (e.g. Windows XP IE8)

Chromium 39.0.2171.65 Ubuntu 14.04

Findings

The following line in src/js/media/html5.js seems to cause the error:

vjs.Html5.prototype.preload = function(){ return this.el_.preload; };

Both preload values get minified away, which results in the following minified source:

s.Qa=function(){return this.c.Qa};

This results in 2 issues: Qa cannot be found by techGet when looking for preload, and Qa is not a valid property of a HTMLVideoElement.

Changing the original source line to the following fixes the error and gives the expected output:

vjs.Html5.prototype['preload'] = function(){ return this.el_['preload']; };

I am unsure if this problem exists in other places.
@mmcc
Copy link
Member

mmcc commented May 22, 2015

Thanks for the fantastic bug report! You hit the nail on the head, this was a result of aggressive mangling via Closure Compiler. We've removed Closure Compiler and switched to Uglify in master (#1586), so things like this shouldn't be [as much of] an issue in 5.0 and beyond.

Master is frozen for minor releases, but we're still patching 4.12. Want to submit a pull request to fix this? If so, please make sure to make it against the stable branch.

@geniusdex
Copy link
Author

The issue you referenced (#1586) mentions an src/js/exports.js file which I was not previously aware of. It seems to be the proper place to fix this, but I am not completely certain how this should work. For example, the autoplay attribute of the HTML5 tech has no issues with being mangled, but is not included in that file. Am I overlooking something?

As a side-question: is there more information on the time-path and proposed changes for 5.0? It might be an option for us to switch to 5.0 if its release is within reasonable time.

@geniusdex geniusdex mentioned this issue May 31, 2015
Closed
@Sattar1 Sattar1 mentioned this issue Aug 24, 2015
Closed
@heff
Copy link
Member

heff commented Sep 14, 2015

@geniusdex 5.0 should be fully released in the next month, but you can start using it now without too much pain.

It'd be great if someone can confirm this is fixed in 5.0 so we can close the issue.

@nickygerritsen
Copy link

Just tested this (http://output.jsbin.com/jijecateri) and it seems to work correctly now.

@heff
Copy link
Member

heff commented Sep 15, 2015

Awesome, thanks @nickygerritsen!

@heff heff closed this as completed Sep 15, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@heff @nickygerritsen @mmcc @geniusdex