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

Too many globals added by videojs #603

Closed
jelbourn opened this issue Jun 26, 2013 · 10 comments
Closed

Too many globals added by videojs #603

jelbourn opened this issue Jun 26, 2013 · 10 comments

Comments

@jelbourn
Copy link
Contributor

Right now, videojs seems to add 5 properties to the global object.
Example: http://jsbin.com/izovid/7/edit

Some of these properties are minified by the closure compiler (in the example, "xd", "yd", "pd")

This is problematic for anyone that includes the minified videojs script in a project that has been independently minified with the closure compiler because it is possible that the minified properties collide and overwrite.

Ideally, the library would only touch the global object with the single "videojs" property.

@heff
Copy link
Member

heff commented Jun 26, 2013

Hmm, that's not right.

On Jun 26, 2013, at 8:25 AM, Jeremy Elbourn notifications@github.com wrote:

Right now, videojs seems to add 5 properties to the global object.
Example: http://jsbin.com/izovid/7/edit

Some of these properties are minified by the closure compiler (in the example, "xd", "yd", "pd")

This is problematic for anyone that includes the minified videojs script in a project that has been independently minified with the closure compiler because it is possible that the minified properties collide and overwrite.

Ideally, the library would only touch the global object with the single "videojs" property.


Reply to this email directly or view it on GitHub.

@heff
Copy link
Member

heff commented Jun 26, 2013

xd and yd come from this minification oversight: https://github.com/videojs/video.js/blob/master/src/js/core.js#L55
Those need to be turned into string keys.

pd seems to come from goog.base.js (part of closure compiler). pd isn't used anywhere after it's set, so CC is doing a bad job of removing that. My best guess is it's this var.
https://github.com/videojs/video.js/blob/master/build/compiler/goog.base.js#L397

vdata is part of the event system and gets added to any element that events get tracked on.

@jelbourn
Copy link
Contributor Author

Sounds like it's only that minification oversight that's problematic, then. In my case, a conflict with "xd" was causing random elements to be turned into video players.

@heff
Copy link
Member

heff commented Jun 26, 2013

Heh, play all the things! Thanks for pointing that out.

@heff
Copy link
Member

heff commented Jun 26, 2013

The first one is a pretty easy fix. Did you want to make a pull request and contribute some code?

@jelbourn
Copy link
Contributor Author

Perhaps another time; I'm not familiar with the testing framework and would have to get the whole environment set-up.

@heff
Copy link
Member

heff commented Jun 26, 2013

No problem, though it's probably easier than you think. (i.e. make the commit in github; let Travis run the tests on the pull request since no new tests are needed).

@heff
Copy link
Member

heff commented Mar 5, 2014

Confirming that this is still an open issue as of 4.4.2
http://jsbin.com/izovid/15/edit

"Globals added: "
["Xd", "Wd", "vdata1393978974754", "Fd", "videojs", "_V_"]

@heff
Copy link
Member

heff commented Aug 5, 2014

Should be all good now! :) (in version 4.7.0 forward)

@mreinstein
Copy link
Contributor

@heff @gkatsev This still seems to be a problem in 6.10:

http://jsbin.com/nuveduhutu/7/edit?html,js,console,output

"Globals added: "
[ "vdata1532308500384", "vttjs", "WebVTT", "videojs" ]

can we re-open this issue please?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants