Proxy ios webkit events into fullscreenchange #3644
Conversation
|
This is failing the test due to the firefox issue which is fixed here |
| }; | ||
|
|
||
| this.on('webkitbeginfullscreen', webkitbeginFn); | ||
| this.on('dispose', () => this.off('webkitbeginfullscreen', webkitbeginFn)); |
There was a problem hiding this comment.
I don't think webkitendfullscreen will be disposed correctly if we dispose the player while in full screen for some reason
There was a problem hiding this comment.
Yup, you're correct. I'll update it.
There was a problem hiding this comment.
@brandonocasey if you can take another look now, that would be great
| * @method proxyWebkitFullscreen_ | ||
| */ | ||
| proxyWebkitFullscreen_() { | ||
| if ('webkitDisplayingFullscreen' in this.el_) { |
There was a problem hiding this comment.
I would do if (!'webkitDisplayingFullscreen' in this.el_) if that works and return at the top
There was a problem hiding this comment.
Yeah, that would be a bit nicer.
| this.on('webkitbeginfullscreen', beginFn); | ||
| this.on('dispose', () => { | ||
| this.off('webkitbeginfullscreen', beginFn); | ||
| this.off('webkitendfullscreen', endFn); |
There was a problem hiding this comment.
will this trigger an error if we are not in full screen? aka this.one('webkitendfullscreen', endFn); is already triggered and gone or was never added in the first place(in the case of never going to full screen)
There was a problem hiding this comment.
Nope, neither of these should error out if it's already been removed.
Previously, the ios webkit events (webkitbeginfullscreen and webkitendfullscreen) were only proxied into the fullscreenchange event when requestFullscreen was called directly. So, on iPhones, we would never be able to get this event. This makes it so these events are always proxied.
…t' from var names
gkatsev
force-pushed
the
proper-ios-fullscreenchange
branch
from
c73de48
to
cf01084
Compare
|
I've made the nesting change. I've also rebased against master, so, hopefully, the tests will pass without the firefox issue. |
Description
Previously, the ios webkit events (webkitbeginfullscreen and
webkitendfullscreen) were only proxied into the fullscreenchange event
when requestFullscreen was called directly. So, on iPhones, we would
never be able to get this event. This makes it so these events are
always proxied.
Requirements Checklist