Closed
Bug 1412678
Opened 7 years ago
Closed 7 years ago
Progressive Web Apps break fullscreen mode, so do Custom Tabs
Categories
(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P2)
Tracking
(fennec+, firefox-esr52 unaffected, firefox57 unaffected, firefox58 fixed, firefox59 verified)
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
fennec | + | --- |
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | fixed |
firefox59 | --- | verified |
People
(Reporter: mark.paxman99, Assigned: droeh)
References
Details
Attachments
(1 file, 1 obsolete file)
684 bytes,
patch
|
snorp
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:58.0) Gecko/20100101 Firefox/58.0 Build ID: 20171027220059 Steps to reproduce: In Fennec Nightly 2017-10-29 (earlier versions too) Navigate to www.theguardian.com/uk Tap the house icon in the toolbar to create a Progressive Web App on your desktop In Fennec, navigate to https://arstechnica.com/gaming/2017/10/assassins-creed-origins-review-a-living-breathing-ancient-world/ There is a video about half way down. Play the video and then try to enter full screen using the button bottom right of the video. It works OK Go to your Android desktop and open the Guardian PWA Go back to Ars Technica in Fennec and try to play the video in full screen Full Screen video button doesn't work Kill the PWA from the Android task manager (square icon then swipe it away) Full Screen video button now works as expected I guess the same would be true for any PWA and any video. The same is true of a Fennec Custom Tab but here to get full screen functionality back you have to kill the whole of Fennec not just the Custom Tab The same is true for the WebExtension function document.documentElement.requestFullscreen(); it does nothing when a Fennec PWA or Fennec Custom Tab is open. I think.
Comment 1•7 years ago
|
||
I was able to reproduce this with PWA's, but not with a Gmail custom tab. Devices: Samsung Galaxy Note 4 (Android 5.0.1) Huawei P9 Lite (Android 6.0) build: Nightly 58 (2017-10-29).
Blocks: progressive-apps
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
status-firefox58:
--- → affected
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → All
I replicated the broken Custom Tabs behaviour just now on:- Sony Xperia X Compact (Android 7.1.1) I used an app called SearchBar Ex and it was a PITA to get it to use a Fennec Custom Tab rather than a Chrome one. In SearchBar Ex I set 'Enable Internal Browser' to ON but it took a lot more fiddling after that. I don't know how I got it to work in the end, much rebooting phone etc. I am really struggling to get apps to use Fennec Custom Tabs. SearchBar Ex was reluctant to do so. AquaMail always uses Chrome Custom Tabs even though I have Fennec Nightly as my default browser and Fennec Custom Tabs set to ON. Maybe there is another issue with getting apps to use Fennec Custom Tabs, or maybe I am just an idiot (more than likely). If there is anything fancy I need to do, please let me know!
Comment 3•7 years ago
|
||
You did nothing wrong, setting Fennec as your default browser and Custom tabs ON is all you need to do. However, I've just installed the SearchBar Ex app and even though Chrome was the default browser, it asked me which browser I want to use with it. So you can choose Nightly there as default, but it doesn't open a custom tab, just regular browser tabs. That is until you switch 'Enable Internal Browser' to ON. But, 'Enable Internal Browser' specifies that it will use Chrome custom tabs by default, so it's forcing that instead of using your default browser (Fennec). I don't know about AquaMail, couldn't figure out why it uses only Chrome's custom tabs, but it's probably sort of the same thing.
I did get SearchBar Ex to use Fennec Custom Tabs, it's doing so now [ I can send video if you don't believe me ;) ]. I have no idea how I got it to work but I'm reluctant to fiddle with it while this bug is open, I want to leave it to test any fixes. You are right about Gmail - I opened a Fennec Custom Tab from Gmail and it did not break Fennec browser fullscreen. But SearchBar Ex still does. So for me it seems to depend on which app opens the Fennec Custom Tab.
Summary: Progressive Web Apps break fullscreen mode, so do Compact Tabs → Progressive Web Apps break fullscreen mode, so do Custom Tabs
Dylan can you take a look please?
Flags: needinfo?(droeh)
tracking-fennec: ? → +
Priority: -- → P2
Assignee | ||
Comment 6•7 years ago
|
||
Right, so this basically comes down to the requirement that a fullscreen request is only honored if it comes from the active window, and we weren't tracking which window was active with GeckoView. The concept of an active window doesn't necessarily make sense on Android, but removing that entirely proved to be a bit much. Consequently, here's a patch that ensures the window most recently interacted with is the active window, which should fix this issue and generally ensure somewhat better behavior in situations with multiple GeckoViews.
Comment on attachment 8932925 [details] [diff] [review] Ensure the window most recently interacted with is active Review of attachment 8932925 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/android/nsWindow.cpp @@ +496,5 @@ > PostInputEvent([input, guid, blockId, status] (nsWindow* window) { > + // Ensure most recently touched window is active. > + nsIWidgetListener* listener = window->GetWidgetListener(); > + if (listener) { > + listener->WindowActivated(); I'm worried that calling this on every input is going to be a perf issue. Can you break these blocks out into their own function and only call WindowActivated if we're not already the active window?
Attachment #8932925 -
Flags: review?(snorp) → review-
Comment 8•7 years ago
|
||
Comment on attachment 8932925 [details] [diff] [review] Ensure the window most recently interacted with is active Looks like this will fix bug 1419000 as well. We really should be calling `BringToFront()` though, preferably from inside `nsWindow::UserActivity()`. e.g., > if (FindTopLevel() != nsWindow::TopWindow()) { > BringToFront(); > }
Assignee | ||
Comment 9•7 years ago
|
||
Updated per feedback.
Attachment #8932925 -
Attachment is obsolete: true
Attachment #8933323 -
Flags: review?(snorp)
Comment on attachment 8933323 [details] [diff] [review] Ensure the window most recently interacted with is active Review of attachment 8933323 [details] [diff] [review]: ----------------------------------------------------------------- This is much nicer :)
Attachment #8933323 -
Flags: review?(snorp) → review+
Comment 11•7 years ago
|
||
Pushed by droeh@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6f0efd8680 Ensure the window most recently interacted with is the active window. r=snorp
![]() |
||
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ee6f0efd8680
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•7 years ago
|
status-firefox57:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 13•7 years ago
|
||
Please request Beta approval on this when you get a chance.
Flags: needinfo?(droeh)
Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8933323 [details] [diff] [review] Ensure the window most recently interacted with is active Approval Request Comment [Feature/Bug causing the regression]: [User impact if declined]: PWAs and custom tabs will interfere with fullscreen in Fennec [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Not very [Why is the change risky/not risky?]: This updates behavior so that the window the user is interacting with is always considered the active window [String changes made/needed]: None
Flags: needinfo?(droeh)
Attachment #8933323 -
Flags: approval-mozilla-beta?
Comment 15•7 years ago
|
||
Comment on attachment 8933323 [details] [diff] [review] Ensure the window most recently interacted with is active Fix a PWA related issue. Beta58+.
Attachment #8933323 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
![]() |
||
Comment 16•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/0c07f502b744
Comment 17•7 years ago
|
||
Verified on Nightly 59 (2017-12-05). Devices: Google Pixel (Android 8.0) Samsung Galaxy Note 4 (Android 5.0.1)
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•