Closed Bug 1248772 Opened 4 years ago Closed 4 years ago

client.openWindow() should focus the firefox web app on all platforms

Categories

(Core :: DOM: Service Workers, defect)

44 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: maherrj, Assigned: catalinb)

Details

(Whiteboard: btpp-active)

Attachments

(5 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.109 Safari/537.36
Firefox for Android

Steps to reproduce:

Call client.focus() from a trusted notificationclick event in a Service Worker


Actual results:

The browser nor the tab gains focus.


Expected results:

The browser/web-app should be foregrounded and the chosen tab/client in front
Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1194078

Sadly this appears only to be for b2g
OS: Unspecified → All
Hardware: Unspecified → All
Component: Untriaged → DOM: Push Notifications
Product: Firefox → Core
Catalin's working in this area.
Component: DOM: Push Notifications → DOM: Service Workers
Flags: needinfo?(catalin.badea392)
Whiteboard: dom-triaged btpp-followup-2016-02-29
Service Workers are not supported on b2g. Please reopen the bug if this happens on other platforms.
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Flags: needinfo?(catalin.badea392)
Resolution: --- → WONTFIX
No one said Service Workers are supported in b2g. Please pay me the courtesy of spending more than 30secs on this bug report.

The referenced bug I gave you 1194078 "client.focus() should focus the app in firefox OS" looks to be similar to mine but the solution "appears only to be for b2g".

Please note the environment details in the original report. Here they are again: -

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.109 Safari/537.36
Firefox for Android

Steps to reproduce:

Call client.focus() from a trusted notificationclick event in a Service Worker
Status: RESOLVED → UNCONFIRMED
Resolution: WONTFIX → ---
(In reply to Richard Maher from comment #4)
> No one said Service Workers are supported in b2g. Please pay me the courtesy
> of spending more than 30secs on this bug report.
> 
> The referenced bug I gave you 1194078 "client.focus() should focus the app
> in firefox OS" looks to be similar to mine but the solution "appears only to
> be for b2g".
> 
> Please note the environment details in the original report. Here they are
> again: -
> 
> User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML,
> like Gecko) Chrome/48.0.2564.109 Safari/537.36
> Firefox for Android
> 
> Steps to reproduce:
> 
> Call client.focus() from a trusted notificationclick event in a Service
> Worker

Oh, I'm sorry for being hasty, I misread comment #1.

Does the bug occur even when the browser window is in the foreground? Does it switch to the new tab?
Yes, if the browser is in the foreground it switches to the new tab.

SSCCE attached.

On Chrome you need a manifest.json file: -
{
"name": "Push It Real Good",
"gcm_sender_id": "Your project#"
}

Go to http://localhost/whereverYouPutIt/index.html

Turn on developerTools->Debug->console

Click [subscribe]

Note that the Endpoint/URL now appears in the console and you are subscribe to notifications

From the command line on you client: -

curl -X POST --insecure https://updates.push.services.mozilla.com/push/gAAAAABWwXpUlqu. . ._jol_4=

Obviously replacing my endpoint/url with what was in your console output.
Assignee: nobody → catalin.badea392
Whiteboard: dom-triaged btpp-followup-2016-02-29 → btpp-active
How'd that followup pan out?
Catalin can you please make this bug available for work again? Someone less "busy" may be able to pick it up. After all it's not rocket science.
I would rather work on it myself.

I've taken a look at the test case. First, the service worker never gets to call focus() on an existing client because matchAll() will return only window clients controlled by the service worker and youtube.com (the url used in the test case) doesn't match the SW's origin. Clicking the notification always results in an openWindow call.

After changing the test to enter the focus() branch, everything seems to work just fine.
See: https://catalinb.github.io/sw_focus_test/. 

The problem lies with openWindow(). Calling openWindow with Firefox minimized or in the background will not
bring it in the foreground. The behaviour is a bit different between windows and mac. On mac, clicking a notification will bring the most recent window to the foreground (not necessarily the window in which the new tab was added), while on windows nothing happens.

The solution is to dispatch a chrome focus event on the browser window in which the new tab is added, the same way focus() works right now.

A workaround for this is to do clients.openWindow().then(function(window) {window.focus()}), though that won't work with cross origin urls.
Summary: client.focus() should focus the firefox web app on all platforms → client.openWindow() should focus the firefox web app on all platforms
Thanks for the update!

Can't seem to load https://catalinb.github.io/manifest.json 404
> The problem lies with openWindow().

> The solution is to dispatch a chrome focus event on the browser window in 
> which the new tab is added, the same way focus() works right now.
> 
> A workaround for this is to do clients.openWindow().then(function(window) 
> {window.focus()}), 

OK.

> though that won't work with cross origin urls.

Why not?
(In reply to Richard Maher from comment #15)
> > The problem lies with openWindow().
> 
> > The solution is to dispatch a chrome focus event on the browser window in 
> > which the new tab is added, the same way focus() works right now.
> > 
> > A workaround for this is to do clients.openWindow().then(function(window) 
> > {window.focus()}), 
> 
> OK.
> 
> > though that won't work with cross origin urls.
> 
> Why not?
That's because openWindow will resolve to null when loading an url from a different origin, so there won't be a window client to call focus on.
Just backing up a bit to: -
> Calling openWindow with Firefox minimized or in the background will not
bring it in the foreground.

Can you please explain why the window is not brought to the foreground as it does with Google Chrome?

I couldn't find anything in the spec about it: -
https://www.w3.org/TR/service-workers/#clients-openwindow-method

Although the following MDN documentation seems relevant: -

https://developer.mozilla.org/en-US/docs/Web/API/Clients/openWindow
> In Firefox, the method is allowed to show popups only when called as the result of a notification click event.
(In reply to Richard Maher from comment #17)
> Just backing up a bit to: -
> > Calling openWindow with Firefox minimized or in the background will not
> bring it in the foreground.
> 
> Can you please explain why the window is not brought to the foreground as it
> does with Google Chrome?
> 
> I couldn't find anything in the spec about it: -
> https://www.w3.org/TR/service-workers/#clients-openwindow-method
> 
> Although the following MDN documentation seems relevant: -
> 
> https://developer.mozilla.org/en-US/docs/Web/API/Clients/openWindow
> > In Firefox, the method is allowed to show popups only when called as the result of a notification click event.

The spec doesn't usually deal with OS specifics, like bringing windows to the foreground. I think we should follow chrome's behaviour here.
Comment on attachment 8736430 [details] [diff] [review]
Trigger a OS window focus in ServiceWorkerClients::OpenWindow.

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

::: dom/workers/ServiceWorkerClients.cpp
@@ +509,5 @@
> +      }
> +      nsContentUtils::DispatchChromeEvent(doc,
> +                                          window->GetOuterWindow(),
> +                                          NS_LITERAL_STRING("DOMServiceWorkerFocusClient"),
> +                                          true, true);

Can you please add an nsContentUtils helper for this and use it both here and in <https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerWindowClient.cpp#98>?  r=me with that!
Attachment #8736430 - Flags: review?(ehsan) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cce5b60332cd
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Could you please check on nightly if this fixed the issue for you?
Flags: needinfo?(maherrj)
Looks good. Thank-you.
Flags: needinfo?(maherrj)
You need to log in before you can comment on or make changes to this bug.