fix(fs): make sure there's only one fullscreenchange event#5686
Conversation
|
Looks like this works great everywhere except IE11. |
|
Firefox has problems with closing. |
misteroneill
left a comment
misteroneill
left a comment
There was a problem hiding this comment.
👍
Verified in Chrome, Firefox, Safari, Edge, and IE11.
|
I tried this on a lot of different browsers and devices, including GC and FF from before and after the move to unprefixed. It worked everywhere, except on Android and desktop Firefox before unprefixed, where the event didn't fire. Note that depending on the Android device, that might be the latest release available. |
|
@nackd that is firefox on android? Also, thanks for checking! |
|
Firefox on Android, yes. |
|
In case there are doubts yet: besides Firefox on Android, it also failed to fire on desktop Firefox (I tried Firefox 61, but I presume it fails with 62 and 63 too). |
|
Thanks! Yeah, it's likely the same issue. I'll take a look. |
|
@nackd I just pushed a change (c026af8) that I think fixes older firefox, would you be able to give it a try again? |
|
Sure, but only on Thursday, @gkatsev. |
|
@nackd that's fine. We're more or less all on break for the week anyway :) |
|
@nackd hope you had a good new year, did you have a chance to take a look at this PR? Thanks! |
|
Thanks, @gkatsev, happy new year! I'm busier than I anticipated, but I plan to have a look at this tomorrow or during the weekend. |
|
I was finally able to get to this and it seems to work great, be it prefixed, unprefixed, desktop or Android Firefox. Thanks a lot, @gkatsev! |
|
Awesome, glad to hear! |
Before the fullscreen API was un-prefixed, we listened to triggered a `fullscreenchange` event on the player manually. This worked fine because a prefixed `fullscreenchange` event was triggered on the player element itself. But when it was unprefixed, we ended up with two events now, the unprefixed event and the one we triggered. Instead, we should listen the event the browser supports and re-trigger it as the unprefixed event as necessary. We also make sure that the handler gets calls when the document level fullscreenchange handler gets called so that it is cancelled properly. Fixes #5685.
Before the fullscreen API was un-prefixed, we listened to triggered a `fullscreenchange` event on the player manually. This worked fine because a prefixed `fullscreenchange` event was triggered on the player element itself. But when it was unprefixed, we ended up with two events now, the unprefixed event and the one we triggered. Instead, we should listen the event the browser supports and re-trigger it as the unprefixed event as necessary. We also make sure that the handler gets calls when the document level fullscreenchange handler gets called so that it is cancelled properly. Fixes #5685.
3 participants
Before the fullscreen API was un-prefixed, we listened to triggered a
fullscreenchangeevent on the player manually. This worked finebecause a prefixed
fullscreenchangeevent was triggered on the playerelement itself. But when it was unprefixed, we ended up with two events
now, the unprefixed event and the one we triggered.
Instead, we should listen the event the browser supports and re-trigger
it as the unprefixed event as necessary.
We also make sure that the handler gets calls when the document level
fullscreenchange handler gets called so that it is cancelled properly.
Fixes #5685.