If you do:

browser.permissions.request({
  permissions: ["notifications"]
})

then

browser.notifications.onClicked.addListener(onClicked);

and finally

browser.permissions.remove({
  permissions: ["notifications"]
})

the listener will not be removed.

Because, after you do

browser.permissions.remove({
  permissions: ["notifications"]
})

browser.notifications is undefined and the listener added by the add-on should also be removed. Because the add-on can no longer use browser.notifications. So it doesn't make sense, to keep the listener alive.

Should

browser.permissions.remove({
  permissions: ["notifications"]
})

also remove the listener?

I have a test add-on attached that shows the problem:
It should not log the message "already has listener" to the console.

First, please don't needinfo multiple people without asking specific questions and waiting a reasonable time for a reply.

(In reply to kernp25 from comment #0)

browser.notifications is undefined and the listener added by the add-on should also be removed. Because the add-on can no longer use browser.notifications. So it doesn't make sense, to keep the listener alive.

I'm not sure "it doesn't make sense" is a strong enough reason to write and maintain more code to clean up a listener in this case -- it appears to me that the listener won't ever actually be invoked so does it do any actual harm to keep it around? If you want to propose a change to the current behavior, please include a more detailed explanation of why the change would be valuable and direct a question to a current member of the addons engineering team (I think :robwu has been the most active developer of the notifications api, he would be a good place to start)

This is not specific to the notifications API, but a bindings issue.

The console shows the following error:

map.keys is not a function ExtensionChild.jsm:1033

https://searchfox.org/mozilla-central/rev/524bed6dfbc5ae21c62632d83b7573448b29e0ac/toolkit/components/extensions/ExtensionChild.jsm#1033

To fix this, we should iterate over map.listeners.keys() instead of map.keys(), and add a regression test. Use .hasListener to check whether a listener has been removed and/or trigger the event to check whether the event has been dispatched.

This is actually fixed in the patch for bug 1630419, and the test in that patch would fail without the fix.

in bug 1630419