Bug 587587 - Always dismiss popup notifications when they are hidden, and add "removeOnDismissal" option to show() [r=gavin, a=blocking-betaN]

browser/base/content/test/browser_popupNotification.js

@@ -198,6 +198,8 @@ var tests = [
     },
     onHidden: function (popup) {
       ok(this.notifyObj.mainActionClicked, "mainAction was clicked");
+      ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
+      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
     }
   },
   { // Test #1
@@ -211,6 +213,8 @@ var tests = [
     },
     onHidden: function (popup) {
       ok(this.notifyObj.secondaryActionClicked, "secondaryAction was clicked");
+      ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
+      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
     }
   },
   { // Test #2
@@ -259,9 +263,8 @@ var tests = [
     },
     onHidden: function (popup) {
       // actually remove the notification to prevent it from reappearing
-      ok(!wrongBrowserNotificationObject.dismissalCallbackTriggered, "dismissal callback wasn't called");
+      ok(wrongBrowserNotificationObject.dismissalCallbackTriggered, "dismissal callback triggered due to tab switch");
       wrongBrowserNotification.remove();
-      ok(!wrongBrowserNotificationObject.dismissalCallbackTriggered, "dismissal callback wasn't called after remove()");
       ok(wrongBrowserNotificationObject.removedCallbackTriggered, "removed callback triggered");
       wrongBrowserNotification = null;
     }
@@ -290,6 +293,8 @@ var tests = [
       this.notification2.remove();
     },
     onHidden: function (popup) {
+      ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
+      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
     }
   },
   // Test that two notifications with different IDs are displayed
@@ -592,7 +597,7 @@ var tests = [
     }
   },
   // Test notification when chrome is hidden
-  { // Test #18
+  { // Test #19
     run: function () {
       this.oldSelectedTab = gBrowser.selectedTab;
       gBrowser.selectedTab = gBrowser.addTab("about:blank");
@@ -617,6 +622,24 @@ var tests = [
       gBrowser.selectedTab = this.oldSelectedTab;
     }
   },
+  // Test notification is removed when dismissed if removeOnDismissal is true
+  { // Test #20
+    run: function () {
+      this.notifyObj = new basicNotification();
+      this.notifyObj.addOptions({
+        removeOnDismissal: true
+      });
+      this.notification = showNotification(this.notifyObj);
+    },
+    onShown: function (popup) {
+      checkPopup(popup, this.notifyObj);
+      dismissNotification(popup);
+    },
+    onHidden: function (popup) {
+      ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
+      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
+    }
+  },
 ];
 
 function showNotification(notifyObj) {

toolkit/content/PopupNotifications.jsm

@@ -220,6 +220,11 @@ PopupNotifications.prototype = {
    *                              and re-shown)
    *        neverShow:   Indicate that no popup should be shown for this
    *                     notification. Useful for just showing the anchor icon.
+   *        removeOnDismissal:
+   *                     Notifications with this parameter set to true will be
+   *                     removed when they would have otherwise been dismissed
+   *                     (i.e. any time the popup is closed due to user
+   *                     interaction).
    * @returns the Notification object corresponding to the added notification.
    */
   show: function PopupNotifications_show(browser, id, message, anchorID,
@@ -483,7 +488,9 @@ PopupNotifications.prototype = {
       // Notify observers that we're not showing the popup (useful for testing)
       this._notify("updateNotShowing");
 
-      this._hidePanel();
+      // Dismiss the panel if needed. _onPopupHidden will ensure we never call
+      // a dismissal handler on a notification that's been removed.
+      this._dismiss();
 
       // Only hide the iconBox if we actually have no notifications (as opposed
       // to not having any showable notifications)
@@ -537,11 +544,27 @@ PopupNotifications.prototype = {
     if (event.target != this.panel || this._ignoreDismissal)
       return;
 
+    let browser = this.panel.firstChild &&
+                  this.panel.firstChild.notification.browser;
+    if (!browser)
+      return;
+
+    let notifications = this._getNotificationsForBrowser(browser);
     // Mark notifications as dismissed and call dismissal callbacks
     Array.forEach(this.panel.childNodes, function (nEl) {
       let notificationObj = nEl.notification;
-      notificationObj.dismissed = true;
-      this._fireCallback(notificationObj, "dismissed");
+      // Never call a dismissal handler on a notification that's been removed.
+      if (notifications.indexOf(notificationObj) == -1)
+        return;
+
+      // Do not mark the notification as dismissed or fire "dismissed" if the
+      // notification is removed. 
+      if (notificationObj.options.removeOnDismissal)
+        this._remove(notificationObj);
+      else {
+        notificationObj.dismissed = true;
+        this._fireCallback(notificationObj, "dismissed");
+      }
     }, this);
 
     this._update();