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

Check a component is a function before new-ing #2814

Closed
wants to merge 1 commit into from

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented Nov 13, 2015

If third-party code placed a non-Component onto the video.js object and then that same name was referenced in player options, we would try to new that third-party object whether it was a function or not. Check ahead of time and skip things that are obviously not Components.

@pam
Copy link

pam commented Nov 13, 2015

Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: 7c22e6d54ecb098f009407365a0e2112bd1094a4
Build details: https://travis-ci.org/pam/video.js/builds/91049141

(Please note that this is a fully automated comment.)

// component class can be instantiated.
if (typeof ComponentClass !== 'function') {
return null;
};
Copy link
Member

Choose a reason for hiding this comment

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

no semicolon

If third-party code placed a non-Component onto the video.js object and then that same name was referenced in player options, we would try to new that third-party object whether it was a function or not. Check ahead of time and skip things that are obviously not Components.
@dmlap dmlap force-pushed the check-components-before-newing branch from 7c22e6d to 625f01c Compare November 15, 2015 19:04
@pam
Copy link

pam commented Nov 15, 2015

Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: 625f01c
Build details: https://travis-ci.org/pam/video.js/builds/91256996

(Please note that this is a fully automated comment.)

dmlap added a commit to dmlap/videojs-contrib-hls that referenced this pull request Nov 15, 2015
Separate out the source handler object from the HLS namespace. Now, videojs.Hls is an object that is used to organize exports from this library. The actual source handler prototype is now HlsHandler. Intended to be defensive about the backwards-compatibility concerns in videojs/video.js#2814.
@dmlap dmlap closed this in 3852663 Nov 16, 2015
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