1178365 - Can't exit fullscreen videos

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

I noticed this last week, but couldn't file a bug. For whatever reason, it's impossible to exit fullscreen once you enter it when playing a video.

Comment 1

[:Margaret Leibovic]

Kevin, can we get a regression range here?

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

I bisected, pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=98820360ab66&tochange=0496b5b3e9ef

The most likely thing here looks like bug 1105939

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

The inbound range I got is: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e423430262c8&tochange=59d11e593eca

It does not include bug 1105939, but I also don't see anything else obvious in there.

Comment 5

[:Margaret Leibovic]

I just realized I recently reviewed a patch for bug 1161802, perhaps I messed that up? Or maybe there was a corresponding Android change needed for bug 1105939 that didn't happen.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

I re-ran bisect and got a different Nightly range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5787d784485e&tochange=bfd82015df48

Bug 1161802 is in that range, so that does seem likely.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Fix DOM fullscreen support on Android, regressed by bug 1161802

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

We were missing this new widget listener callback, so the fullscreen change never completed.

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

So on Android, we do not have sizemode, and the GeckoAppShell::SetFullScreen will synchronously change the size of the viewport, right? FullscreenChanged callback is supposed to be called after size change.

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #10)
> So on Android, we do not have sizemode, and the GeckoAppShell::SetFullScreen
> will synchronously change the size of the viewport, right? FullscreenChanged
> callback is supposed to be called after size change.

It's not synchronous, no. What badness will happen if the size changes later? I can work on a patch to wait on that before calling the callback.

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

I'm trying to ensure the viewport size to be stable after that callback, so that content can rely on the window size when they get fullscreenchange event. It would also make our fullscreen tests more reliable, though they are currently not enabled on Android.

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Fix DOM fullscreen support on Android

Now we wait for an actual size change before notifying the widget listener

Comment 14

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8629977 [details] [diff] [review]
Fix DOM fullscreen support on Android

Review of attachment 8629977 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4ac17101a35

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f4ac17101a35

Comment 17

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8629977 [details] [diff] [review]
Fix DOM fullscreen support on Android

Approval Request Comment
[Feature/regressing bug #]: bug 1161802
[User impact if declined]: Unable to enter/exit fullscreen mode (incl. video)
[Describe test coverage new/current, TreeHerder]: Nightly
[Risks and why]: low, simply adjusts to widget changes made in other platforms
[String/UUID change made/needed]: none

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8629977 [details] [diff] [review]
Fix DOM fullscreen support on Android

Fix was verified on nightly. Let's land it on mozilla-aurora branch.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/25666390d98c

Comment 21

[Teodora Vermesan (:TeoVermesan)]

Pressing "Back" after entering full screen for http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm video, will display correctly the page, so:
Verified as fixed using:
Device: Nexus 4 (Android 5.1)
Build: Firefox for Android 42.0a1 (2015-07-19)

Comment 22

[Flaviu Cos]

The bug is still reproducible in build 42.0a2 (2015-08-10) using Asus Transformer Pad (Android 4.2.1).