1932263 - Clicking on a notification has no effect if background script in MV3 extension is suspended

Status: VERIFIED FIXED

(WebExtensions :: General, defect, P1)

Description

[Alex Eng]

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

Steps to reproduce:

If the user clicks on a notification from an MV3 extension while the background script is suspended, and an event handler is defined for the click action on the notification, nothing happens.

I have created an example MV3 extension that illustrates the problem. Full source code can be found here: https://github.com/aecreations/mv3-notification-demo

The extension manifest has notifications listed in the permissions key.
The browser.notifications.onClicked.addListener function is defined in the top-level of the background script, not inside a function or method.

Steps to reproduce:

1. Clone the above GitHub repository to a folder on your local machine. Then launch Firefox Developer Edition and install the extension as a temporary add-on
2. A notification should appear immediately upon installation. Do NOT click on it!
3. Wait for the extension's background script to be suspended.
4. Once the background script is suspended, click on the notification. On Windows, it can be found in the Action Center; on macOS, it can be found in the sidebar that appears when clicking on the date and time on the right-hand side of the desktop menu bar.

Actual results:

Clicking on the notification in step 4 has no effect.

Expected results:

Clicking on the notification in step 4 should open an extension page in a popup window.

The behaviour should be the same regardless of whether the notification was clicked as soon as it appeared, or later after the background script is suspended, in which case it should cause it to be restarted so that it can respond to the click action on the notification.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Rob Wu [:robwu]]

Thanks for your report. I thought that we fixed this for all APIs, but apparently that is not the case:
https://searchfox.org/mozilla-central/rev/81ac0fe96e1bd6020408c2ae7eae6e5b5114d1db/toolkit/components/extensions/parent/ext-notifications.js#87

We should implement persistent listeners for notifications.

I wondered whethere there are other cases we overlooked, and I checked all results for events without ExtensionAPIPersistent super class nor a special primeListeners at https://searchfox.org/mozilla-central/search?q=class+extends+ExtensionAPI+%7B&path=parent%2F&case=false&regexp=false

Out of all, we only have the privileged-only geckoProfiler that does not have a method override: https://searchfox.org/mozilla-central/rev/81ac0fe96e1bd6020408c2ae7eae6e5b5114d1db/toolkit/components/extensions/parent/ext-geckoProfiler.js#71

Comment 3

[Rob Wu [:robwu]]

I'm going to fix this since the notifications API is one of the most popular APIs among extensions, and not waking up the event page is disruptive to the functionality of extensions depending on this.

I am also going to request uplifts, at least for ESR128.

Comment 4

[Rob Wu [:robwu]]

Attachment: Bug 1932263 - Support persistent listeners in notifications API

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/472e9951f8a9
Support persistent listeners in notifications API r=zombie

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/472e9951f8a9

Comment 7

[Rob Wu [:robwu]]

Attachment: Bug 1932263 - Support persistent listeners in notifications API

Original Revision: https://phabricator.services.mozilla.com/D232859

Comment 8

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: User interactions with extension notifications may fail to be detected by the extension. The affected extension or browser may be perceived as non-responsive/buggy.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Manual QA not needed, fully covered by automated tests
• Risk associated with taking this patch: Low
• Explanation of risk level: Issue well understood, and fix is very targeted and covered by unit tests.
• String changes made/needed: none
• Is Android affected?: yes

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9445236 [details]
Bug 1932263 - Support persistent listeners in notifications API

This needs rebasing for ESR128.

Comment 10

[Rob Wu [:robwu]]

Rebased for ESR128. Merge conflict was caused by bug 1901282 (https://hg.mozilla.org/mozilla-central/rev/1d375c0daabb37a3d846fe0ff3edcc3a5edc50e3).

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/b3cb8a82c809