1848801 - Clicking Dismiss button on Windows still opens the corresponding tab

Status: VERIFIED FIXED

(Toolkit :: Alerts Service, defect)

Description

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Dismiss button should just dismiss the notification, and should not open anything.

Comment 1

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

I can't reproduce this with https://googlechrome.github.io/samples/notifications/requireInteraction.html but I saw such behavior in Google Calendar. Not sure the difference, maybe only push notifications are affected? But how?

Comment 2

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Actually it's the Chrome example that is special, because new Notification("foo", { requireInteraction: true }) is enough to reproduce the issue.

Comment 3

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:nalexander, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[RyzenInu]

This issue is also happening with me. (Clicking the dismiss button opens the notification's respective link/tab)

Firefox 117.0 (64-bit)
Windows 11 23H2 - Build 22631.2271

Comment 5

[modi-zeichnung-05]

The Chrome-example uses a service worker as one would do for Web Push, too. As far as I know, one needs to handle the event notificationclick in the service worker to focus or open the site. Doing so leads to the problem that is described in this bug, too: A click on the dismiss-button focuses / opens the site. So, Web Push is affected, too.

I believe that the problem in case of Web Push is that the event notificationclick is fired on click on the dismiss-button without a possibility for the event-handler to detect that only the dismiss-button was clicked. I have never recognized that Chrome fires the event on click on the dismiss-button. So, sites would need different handling for different browsers, but it seems that there is not even a solution for Firefox.

I don’t know if the root-cause for Web Push and not Web Push is the same.

Comment 7

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1848801 - Use system dismiss button for persistent notification r=nalexander,nrishel

Per the documentation the system should handle arguments=dismiss to fire dismiss event but somehow it still fires activate event. This patch thus handles it manually while still taking advantage of the Windows provided localization of the button.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e92a8f339bfc
Use system dismiss button for persistent notification r=nrishel,fluent-reviewers,bolsson

Comment 9

[Narcis Beleuzu [:NarcisB]]

Backed out for xpcshell failures on test_windows_alert_service.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/6a775bef9e3ba3adedc5ed068247c87e8cc9f8ad
Log link: https://treeherder.mozilla.org/logviewer?job_id=443135032&repo=autoland&lineNumber=5298

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ed25455cf4c
Use system dismiss button for persistent notification r=nrishel,fluent-reviewers,bolsson

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/1ed25455cf4c

Comment 12

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Comment on attachment 9372283 [details]
Bug 1848801 - Use system dismiss button for persistent notification r=nalexander,nrishel

Beta/Release Uplift Approval Request

• User impact if declined: (Probably is too late for beta, though)

Users getting persistent notification popup will click Dismiss button which unexpectedly opens the webpage, which is the opposite of what Dismiss is. See also bug 1863182.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Open https://www.bennish.net/web-notifications.html

2. Click Authorize
3. Open F12 devtools console and copypaste new Notification("foo", { requireInteraction: true })
4. See the resulting notification
5. Do either one of below:
   a. switch to another tab
   b. switch to another window
   c. close the current page (while not closing Firefox)
   d. close Firefox
6. Click the Dismiss button on the notification
7. See the button do not trigger anything e.g. opening Firefox, focusing the previous window, or switching to the tab.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only changes some arguments we pass to the OS.
• String changes made/needed:
• Is Android affected?: No

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9372283 [details]
Bug 1848801 - Use system dismiss button for persistent notification r=nalexander,nrishel

Fx122 is now in release, switching uplift request to release as a possible ride-along

Comment 14

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:saschanaz, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

We have a release approval nomination instead for a good reason!

Comment 16

[Daniel Bodea [:danibodea]]

I have reproduced this issue in Beta v122.0 (RC) on Windows 10.
Steps used:

1. Open https://www.bennish.net/web-notifications.html
2. Click Authorize
3. Open F12 devtools console and copypaste new Notification("foo", { requireInteraction: true })
4. See the resulting notification
5. Do either one of below:
   a. switch to another tab
   b. switch to another window
   c. close the current page (while not closing Firefox)
   d. close Firefox
   Click the Dismiss button on the notification
   See the button do not trigger anything e.g. opening Firefox, focusing the previous window, or switching to the tab.

It only reproduced in cases a and b. cases c and d do not reopen the closed browser or tab.

This issue no longer occurs in Nightly v123.0a1 (from 2024-01-16).

Comment 17

[Donal Meehan [:dmeehan]]

:saschanaz widget/windows/ToastNotificationHandler.cpp has merge conflicts with release.
If you can attach a patch rebased on release, we can include this in a dot release ride-along.

Comment 18

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1848801 - Use system dismiss button for persistent notification r=dmeehan

Per the documentation the system should handle arguments=dismiss to fire dismiss event but somehow it still fires activate event. This patch thus handles it manually while still taking advantage of the Windows provided localization of the button.

Comment 19

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Done!

Comment 20

[Donal Meehan [:dmeehan]]

Comment on attachment 9373595 [details]
Bug 1848801 - Use system dismiss button for persistent notification r=dmeehan

Approved for 122.0.1

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/c9383a7cb866

Comment 22

[Daniel Bodea [:danibodea]]

I confirm this fix in RC V122.0.1 in Windows 10. Clicking the Windows notification's "Dismiss" button does not refocus the tab, the window or reopen the browser.