1419463 - standalone apps don't seem to focus when service workers call client.focus()

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), enhancement, P2)

Description

[Ben Kelly [:bkelly, not reviewing]]

There is a DOM API which allows service workers to focus an existing window/tab.  This works by dispatching a chrome event "DOMWindowFocus".

Does this work with standalone sites installed to the homescreen in fennec?

I've noticed a couple times I clicked a telegram push notification that did nothing while I had the standalone telegram site open.  Normally it will focus the tab if it exists or open a new window.

Here is the code in fennec that taps into the DOMWindowFocus event:

https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#4311

Comment 1

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

I'm sure we don't handle this right in the standalone scenario. Dylan, we'll need to have some GV API for this, probably in the ContentDelegate. onFocusRequested() maybe? This would cause us to send an intent if we are backgrounded.

Comment 2

[Dylan Roeh (:droeh) (he/him)]

Attachment: Add onFocusRequest to ContentListener

This adds onFocusRequest to ContentListener, listens for "DOMWindowFocus" in GeckoViewContent.js, and calls onFocusRequest when it is received. Also implements onFocusRequest for both PWAs and custom tabs.

Comment 3

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

Comment on attachment 8944454 [details] [diff] [review]
Add onFocusRequest to ContentListener

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

I have questions.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoSession.java
@@ +1196,4 @@
>          void onTitleChange(GeckoSession session, String title);
>  
>          /**
> +        * A page has called WindowClient.focus

Probably not good to reference WindowClient here since it doesn't really mean anything in GeckoView.

One question I have is whether or not this would be called if someone just does window.focus() in a normal page. Would we want to ignore those? Are they guarded by user activity in Gecko?

Comment 4

[Dylan Roeh (:droeh) (he/him)]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> Probably not good to reference WindowClient here since it doesn't really
> mean anything in GeckoView.

Right, I was a little bit hesitant as to how to clearly explain when this gets called. Should I go with something a bit more like "A service worker has requested focus for this page"?

> One question I have is whether or not this would be called if someone just
> does window.focus() in a normal page. Would we want to ignore those? Are
> they guarded by user activity in Gecko?

It doesn't look to me like it would get called in that case; the "DOMWindowFocus" event is fired by nsContentUtils::DispatchFocusChromeEvent(), which is only called by nsDocShell::InternalLoad(), WaitForLoad() in ClientOpenWindowUtils.cpp, and ClientSource::Focus().

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d34b2165a74
Add onFocusRequest to ContentListener API. r=snorp

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2d34b2165a74

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Is it possible to add a test for this?

Comment 8

[Dylan Roeh (:droeh) (he/him)]

(In reply to Ben Kelly [:bkelly] from comment #7)
> Is it possible to add a test for this?

I think the best approach would be to add a test for the GeckoView functionality here; we should be making a major push to add a lot of GV testing soon.

Comment 9

[Jovan Gerodetti]

This still doesn't work 100%. See Bug 1435083