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

fix: make sure that document.createElement exists before using #3706

Merged
merged 1 commit into from
Oct 25, 2016

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Oct 24, 2016

If you try and require videojs in an environment that doesn't implement document.createElement properly -- like in Node.js -- you could potentially get an error. This checks that window.document && window.document.createElement is available before calling createElement.
We specifically check window.document so that global module won't cause issues with it's shimming of document.createElement in `global/document.

Fixes #3665

If you try and require videojs in an environment that doesn't implement `document.createElement` properly -- like in Node.js -- you could potentially get an error. This checks that `window.document && window.document.createElement` is available before calling `createElement`.
We specifically check `window.document` so that `global` module won't cause issues with it's shimming of `document.createElement` in `global/document.

Fixes #3665
if (typeof HTMLVideoElement === 'undefined') {
if (typeof HTMLVideoElement === 'undefined' &&
window.document &&
window.document.createElement) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using document here instead of window.document? We're already importing it on line 8.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned in the description, we can't use document because global/document tries to shim the DOM API in node.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore me, then. Chalk it up to lack of sleep. 😆

@gkatsev gkatsev merged commit 49e29ba into master Oct 25, 2016
@gkatsev gkatsev deleted the naked-createEl branch October 25, 2016 18:20
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.

3 participants