Don't use minifiable player methods in fullscreenToggle #851
Conversation
If the video element has a id attribute when video.js was created, it used that as the player id. If the video did not have an id, however, the player would get an auto-generated component id that started with "undefined_..." and the player element received a id like "vjs_video_...". This change modifies the player init order so that the video element is given an autogenerated id early in the process, which is then picked up by the player component when it gets around to initialization. This unifies the player component id and tag id and is more consistent with the structure created when a video element with a user-defined id is passed in.
Remove qunit files directly from the repo and use npm to load them instead. Add the qunit-toolbar to test runner pages so you can run tests without the "helpful" try-catch blocks inside the browser.
availble -> available
Use string literals to lookup fullscreen-related methods on the player object in the fullscreenToggle component. Otherwise, closure compiler replaces them with minified method names and makes it impossible to supply a simpler "player" object with customized fullscreen logic for the fullscreenToggle to interact with.
| @@ -23,11 +23,11 @@ vjs.FullscreenToggle.prototype.buildCSSClass = function(){ | |||
| }; | |||
|
|
|||
| vjs.FullscreenToggle.prototype.onClick = function(){ | |||
| if (!this.player_.isFullScreen) { | |||
| this.player_.requestFullScreen(); | |||
| if (!this.player_['isFullScreen']) { | |||
There was a problem hiding this comment.
This may fix that iOS fullscreen control issue (I forget the issue number offhand) but my actual use-case is for custom controls during ad playback.
There was a problem hiding this comment.
yeah, just don't like that we had to do this whole string thing.
There was a problem hiding this comment.
Yeah, that's ugly. I actually think we could set up an externs file for the player like we do with Flash.
https://github.com/videojs/video.js/blob/master/src/js/media/flash.externs.js
Externs are meant for external projects, but we're running into inter-component issues here, and I think it would avoid the duplicate methods that exporting creates. Want to try that?
There was a problem hiding this comment.
Opened an externs-based PR: #853
Instead of using string literals, create an externs file for the player and use that to ensure fullscreen functionality is never minified.
|
Bad merge. I'll re-open a clean PR. |
Use string literals to lookup fullscreen-related methods on the player object in the fullscreenToggle component. Otherwise, closure compiler replaces them with minified method names and makes it impossible to supply a simpler "player" object with customized fullscreen logic for the fullscreenToggle to interact with.
This PR includes a typo fix and the changes from Pull #850. I can open a clean PR if those changes require re-think.