1847096 - Clicking notification without an awake service worker always opens a new window with the root path of the origin

Status: NEW

(Core :: DOM: Notifications, defect, P2)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

What we currently do is just opening the origin; e.g. https://app.slack.com for all Slack notifications. We do not trigger notificationclick event on service worker, but we should.

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

So, the code path differs quite from bug 1874511 because we can't depend on Activated event in this case.

1. Windows invokes the Firefox notification server as described in launchArg generated from ToastNotificationHandler::GetLaunchArgument. This includes host/port pair via launchUrl generated by ToastNotificationHandler::ActionArgsJSONString, which then is served as action parameter.
2. The server then again invokes Firefox with --notification-windowsAction.
3. BrowserContentHandler calls handleWindowsTag, which fails with tagWasHandled: false as ToastNotification::VerifyTagPresentOrFallback fails to find any match with mActiveHandlers, which is currently only initiated by ToastNotification::ShowAlert.
4. The failure fallback either opens a tab or a new window with notificationData.launchUrl depending on the current browser state.

(These steps actually happen for both cases per the comment, but is ignored when tag is handled. Or not, see bug 1880863.)

I think we need to reload notification handlers from the notification DB, as I don't think that's happening right now. Each notification in DB has alertName for unique match.

Comment 2

[David]

Would you please remove the "when the browser is closed" from the title? Since there seem to be no conditions at all where it Does run the service worker.

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

"Notification click" is the condition, but sure I can clarify.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Actually this is a defect, so rephrasing the title again to match that.

Comment 5

[David]

That new title places this even farther from the issue I reported which was referred here in bug 1874511, comment 8

I Never click notifications when the browser is closed, it's always open, and clicking them when it's open Always goes to the front page (the main domain) instead of the correct address (as mentioned in comment 0 here) if it goes anywhere at all. Not going anywhere is covered now by bug 1874511.

Including 'when closed' in the title narrows the scope way smaller than the issue is. It's a major user facing problem which significantly reduces the utility of the browser.
Unless I'm the only one seeing this? I have a few extensions but it's a relatively new profile.

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

If it always fails then we need more info, can you try:

1. Open https://simple-push-demo.vercel.app/
2. Enable push
3. Click "Send a push message" immediately when notification shows

Clicking it should open https://web.dev/articles/push-notifications-overview .

Comment 7

[David]

Very interesting. I restarted the browser for an update a few days ago and when it came back this issue didn't happen - it started going to the correct page for every notification that opened at all, including that test page. I'll keep an eye on it, it seems like it's either not the 100% repro I was seeing before or it's been fixed entirely (while open. I still haven't tested the closed case.)

I'll keep the needsinfo on while I monitor the behavior.

Comment 8

[David]

Sorry I didn't get back to you about this. I have seen it happen (when the browser is open) just a few times since then, but it's also difficult to test now because Bug 1874511 is such a common repro.

So it still happens, but rarely anymore.

Comment 9

[Jens Stutte [:jstutte]]

Do we have enough information to set the severity here? Thank you!

Comment 10

[Nick Alexander :nalexander [he/him]]

krosylight: my WNS PoC ends up invoking notifyPushWithData at about the same place where native notifications are handled.

It might be possible to arrange for some Web Push specific data to round-trip through the toast notification system to support this use case, if you'd like it to. The issue might be identifying when a toast notification "originates" from a Web Push -- but I think that's more or less always here. Food for thought.

Comment 11

[Jens Stutte [:jstutte]]

(for comment 10)

Comment 12

[Kagami Rosylight [:saschanaz] (they/them)]

It should be possible to pass a service worker identifier to each notification so that the command would include it as a parameter. I'm not quite sure we should do that, because:

• I'm not sure it's exactly safe, as some malware could scan the notification center and craft a command with the identifier to mock a user click, which then would run JS with malware crafted arguments. But maybe we can't do much for such an infected machine, as there would be hundreds more evil things to do for a malware?
• Do we know the same can be done for Linux and macOS, i.e. can we store the same amount of information into libnotify and macOS notification system?
  • Can you share how you did it for Windows? Bug 1803416 only has the proxy binary part, and per my understanding we hash the information here to fit the system limitation. My plan was to use this hashed tag everywhere (in other backends and notification store), but open for better ideas!

Please feel free to correct me if I got something wrong 👍🏻