1559697 - chrome.notifications extension API: Clicking a notification makes it impossible to clear on macOS

Status: VERIFIED FIXED

(WebExtensions :: Frontend, defect)

Description

[eritbh]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

I'm writing a WebExtension which uses the chrome.notifications API and testing it on macOS Mojave 10.14.4, on Firefox Developer Edition 68.0b10. Essentially, the following code does not work:

chrome.notifications.onClicked.addListener(notificationId => {
  chrome.notifications.getAll(allNotifications => {
    console.log(notificationId, allNotifications); // notificationId is not a key of allNotifications
    chrome.notifications.clear(notificationId, success => {
      console.log(success); // always false
    });
  });
});

I've created a sample WebExtension which can be cloned and loaded as a temporary extension to illustrate the issue: https://github.com/Geo1088/chrome-notifications-test-case

The same code works as intended on Windows 10.

Actual results:

• Calling chrome.notifications.getAll from within a chrome.notifications.onClicked listener results in the clicked tab's ID being absent from the returned object.
• Calling chrome.notifications.clear from within a chrome.notifications.onClicked listener results in no visible action and a value of false being passed to the callback.

Expected results:

• Calling chrome.notifications.getAll from within a chrome.notifications.onClicked listener should result in the clicked tab's ID being present in the returned object.
• Calling chrome.notifications.clear from within a chrome.notifications.onClicked listener should result in the notification being cleared and the value true being passed to the callback.

Comment 1

[Luciana queirolo]

Hi,

Please, check if the issue is reproducible on latest Nightly version, here is the link where you can download it https://www.mozilla.org/en-US/firefox/nightly/all/
Could you also attached a screen recording for this please?

Comment 2

[eritbh]

Reproduced on a clean install of Nightly 69.0a1 build 20190619214046 on macOS 10.14.4. Attaching a screen recording of the issue momentarily.

Comment 3

[eritbh]

Attachment: Screen Recording 2019-06-19 at 21.46.52.mov

Screen recording of the bug being reproduced in Firefox Nightly 69.0a1 build 20190619214046 on macOS 10.14.4.

The test extension is loaded through the debugger and a notification is created. After opening the inspector, the notification is not clicked, and the appropriate console output is displayed. The notification is not observed in the notification center after the ten-second timeout.

The extension is then removed and re-added through about:debugging, and this time the notification is clicked. The bugged console output is observed, and the notification remains in notification center even after the ten-second timeout.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:ddurst, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 5

[Rob Wu [:robwu]]

Not a regression. This behavior already existed since the initial implementation in bug 1256635.

On Linux, Windows and Android, clicking the native notification will automatically close the notification.
On all platforms (including macOS), if the pref alerts.useSystemBackend is set to false, then the same behavior occurs.

The current exception is macOS: I tested in Firefox 67.0.4, Firefox Nightly, Safari 12.1.1, Google Chrome 75.0.3770.90, and the web platform's Notification API is similar to the extension's chrome.notifications API (in Chrome): When the notification is shown, clicking it does not dismiss the notification. The click event can thus be fired repeatedly.

Compared to other browsers, and the web platform's Notification API on desktop, the notification from the browser.notifications API does not have a close button.

Comment 6

[Rob Wu [:robwu]]

Attachment: Bug 1559697 - Keep track of notification until it is really closed

On Windows, Linux, Android, the backend for native notifications closes
the notification notification upon click.

On macOS, the notification sticks around, unless the user closes the
notification. This behavior is also observed in the Notification API
on the web platform, across different browsers.

Comment 7

[Rob Wu [:robwu]]

Attachment: Bug 1559697 - Show "Close" button on extension notification on macOS

Comment 8

[Shane Caraveo (:mixedpuppy)]

IIUC our implementation was not 100% compatible with Chrome or with web notifications. This patch addresses that (and almost as a side effect fixes the reported issue). As well it seems that extensions must explicitly close the notification now.

Given the time that it has worked this way, do we know that this wont break any existing extensions on AMO?

Comment 9

[Rob Wu [:robwu]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #8)

As well it seems that extensions must explicitly close the notification now.

The second patch only adds a close button on macOS that was previously not visible. The invisibility of the button prevented users from closing the notification.

Given the time that it has worked this way, do we know that this wont break any existing extensions on AMO?

The first patch changes the behavior of notifications.getAll, and does not change the behavior of events (i.e. onClicked/onClosed), and the unit tests that I added (that explicitly check the order of events), already pass before my fix if I remove the checkNotificationCount checks. So I don't expect significant breakage.

Comment 10

[Rob Wu [:robwu]]

Attachment: Bug 1559697 - Clear notification if backend is not supported

Comment 11

[Shane Caraveo (:mixedpuppy)]

This will need an STR for QE to test the close button.

Comment 12

[Rob Wu [:robwu]]

STR:

1. Install an add-on that uses notifications: https://addons.mozilla.org/en-US/firefox/addon/display-_anchors/
2. Click on the extension button (while still on AMO).
3. Observe a desktop notification from the extension.
4. Put the mouse pointer on the notification.

Expected:

• A "Close" button should appear. Clicks at the left (not on the button) are ignored, clicking on the "Close" button should close the notification.

Actual (before patch):

• "Close" button does not appear. Clicks on the notification are ignored.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53effd672f63
Keep track of notification until it is really closed r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/359eff208383
Show "Close" button on extension notification on macOS r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/f0cdc7a25e47
Clear notification if backend is not supported r=mixedpuppy

Comment 14

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/53effd672f63
https://hg.mozilla.org/mozilla-central/rev/359eff208383
https://hg.mozilla.org/mozilla-central/rev/f0cdc7a25e47

Comment 15

[Madalin Cotetiu]

I have verified this issue on MacOS 10.14.1 Using FF70.0a1 (20190712095934) using the steps from comment 12. The following behavior was observed:

• after clicking the extension button the notification is displayed, when hovering the notification the close button is displayed.
• Clicking the close button closes the notification.

The only thing that is not as in the expected results mentioned in the comment 12 is that clicking anywhere on the notification (not on the close button) closes the notification.

@Rob
Should we log a new bug for this issue?

Comment 16

[Rob Wu [:robwu]]

(In reply to Madalin Cotetiu from comment #15)

I have verified this issue on MacOS 10.14.1

The only thing that is not as in the expected results mentioned in the comment 12 is that clicking anywhere on the notification (not on the close button) closes the notification.

@Rob
Should we log a new bug for this issue?

No. On macOS 10.14.5 I observed that the notification doesn't disappear on click, and hence I found the need to add a close button.
If the notification is somehow clickable on your machine, then that's fine (it means that this bug is not as severe as I initially thought).

Comment 17

[Madalin Cotetiu]

Based on the above comment I'm marking the bug as verified fixed. Thanks for the response Rob!