Bug 1265797 - WebExtensions Notification observer should not remove the notificationId on alertshow, r?aswan
Note that I looked at trying to create a test to test this bug and which would prove it was fixed by the patch,
but there is already a test in place [1] that would catch this if the timing was right.
It looks like most of the time the call to clear() happens before the observer reacts to the `alertshow` topic
and only if the opposite is true would the bug manifest itself.
All of this is to say that I believe that the fix is a good one, but not one for which we can accurately test.
[1] https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/test/mochitest/test_ext_notifications.html#91
MozReview-Commit-ID: GLqHkHEnDwd
--- a/toolkit/components/extensions/ext-notifications.js
+++ b/toolkit/components/extensions/ext-notifications.js
@@ -49,29 +49,32 @@ Notification.prototype = {
// This will fail if the OS doesn't support this function.
}
notificationsMap.get(this.extension).delete(this.id);
},
observe(subject, topic, data) {
let notifications = notificationsMap.get(this.extension);
+ function emitAndDelete(event) {
+ notifications.emit(event, data);
+ notifications.delete(this.id);
+ }
+
// Don't try to emit events if the extension has been unloaded
if (!notifications) {
return;
}
if (topic === "alertclickcallback") {
- notifications.emit("clicked", data);
+ emitAndDelete("clicked");
}
if (topic === "alertfinished") {
- notifications.emit("closed", data);
+ emitAndDelete("closed");
}
-
- notifications.delete(this.id);
},
};
/* eslint-disable mozilla/balanced-listeners */
extensions.on("startup", (type, extension) => {
let map = new Map();
EventEmitter.decorate(map);
notificationsMap.set(extension, map);