Bug 1336066 - Avoid reshowing a persistent notification that has not been dismissed when clicking the anchor icon. r=johannh, a=gchang
authorFlorian Quèze <florian@queze.net>
Mon, 13 Feb 2017 13:01:34 +0100
changeset 376235 ce2403f3f23bf34c6708b9162502459823ca9af5
parent 376234 3a3e07e22ea8a9f6523172c6e85ff363b9b1a53d
child 376236 5b366c35d4a51cf1a73f3dee6cf40eb4cbc8c9db
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh, gchang
bugs1336066
milestone53.0a2
Bug 1336066 - Avoid reshowing a persistent notification that has not been dismissed when clicking the anchor icon. r=johannh, a=gchang
browser/base/content/test/popupNotifications/browser_popupNotification_5.js
toolkit/modules/PopupNotifications.jsm
--- a/browser/base/content/test/popupNotifications/browser_popupNotification_5.js
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification_5.js
@@ -66,19 +66,20 @@ var tests = [
   { id: "Test#3",
     *run() {
       let notifyObj = new BasicNotification(this.id);
       notifyObj.options.dismissed = true;
 
       let win = yield BrowserTestUtils.openNewBrowserWindow();
 
       // Open the notification in the original window, now in the background.
-      showNotification(notifyObj);
+      let notification = showNotification(notifyObj);
       let anchor = document.getElementById("default-notification-icon");
       is(anchor.getAttribute("showing"), "true", "the anchor is shown");
+      notification.remove();
 
       yield BrowserTestUtils.closeWindow(win);
       yield waitForWindowReadyForPopupNotifications(window);
 
       goNext();
     }
   },
   // Test that persistent doesn't allow the notification to persist after
@@ -325,9 +326,78 @@ var tests = [
       ok(!this.notification2.options.persistent, "notification 2 is not persistent");
       ok(this.notification3.options.persistent, "notification 3 is persistent");
 
       this.notification1.remove();
       this.notification2.remove();
       this.notification3.remove();
     }
   },
+  // Test clicking the anchor icon.
+  // Clicking the anchor of an already visible persistent notification should
+  // focus the main action button, but not cause additional showing/shown event
+  // callback calls.
+  // Clicking the anchor of a dismissed notification should show it, even when
+  // the currently displayed notification is a persistent one.
+  { id: "Test#11",
+    *run() {
+      function clickAnchor(notifyObj) {
+        let anchor = document.getElementById(notifyObj.anchorID);
+        EventUtils.synthesizeMouseAtCenter(anchor, {});
+      }
+
+      let popup = PopupNotifications.panel;
+
+      let notifyObj1 = new BasicNotification(this.id);
+      notifyObj1.id += "_1";
+      notifyObj1.anchorID = "default-notification-icon";
+      notifyObj1.options.persistent = true;
+      let shown = waitForNotificationPanel();
+      let notification1 = showNotification(notifyObj1);
+      yield shown;
+      checkPopup(popup, notifyObj1);
+      ok(!notifyObj1.dismissalCallbackTriggered,
+         "Should not have dismissed the notification");
+      notifyObj1.shownCallbackTriggered = false;
+      notifyObj1.showingCallbackTriggered = false;
+
+      // Click the anchor. This should focus the primary button, but
+      // not call event callbacks on the notification object.
+      clickAnchor(notifyObj1);
+      is(document.activeElement, popup.childNodes[0].button);
+      ok(!notifyObj1.dismissalCallbackTriggered,
+         "Should not have dismissed the notification");
+      ok(!notifyObj1.shownCallbackTriggered,
+         "Should have triggered the shown event again");
+      ok(!notifyObj1.showingCallbackTriggered,
+         "Should have triggered the showing event again");
+
+      // Add another notification.
+      let notifyObj2 = new BasicNotification(this.id);
+      notifyObj2.id += "_2";
+      notifyObj2.anchorID = "geo-notification-icon";
+      notifyObj2.options.dismissed = true;
+      let notification2 = showNotification(notifyObj2);
+
+      // Click the anchor of the second notification, this should dismiss the
+      // first notification.
+      shown = waitForNotificationPanel();
+      clickAnchor(notifyObj2);
+      yield shown;
+      checkPopup(popup, notifyObj2);
+      ok(notifyObj1.dismissalCallbackTriggered,
+         "Should have dismissed the first notification");
+
+      // Click the anchor of the first notification, it should be shown again.
+      shown = waitForNotificationPanel();
+      clickAnchor(notifyObj1);
+      yield shown;
+      checkPopup(popup, notifyObj1);
+      ok(notifyObj2.dismissalCallbackTriggered,
+         "Should have dismissed the second notification");
+
+      // Cleanup.
+      notification1.remove();
+      notification2.remove();
+      goNext();
+    }
+  },
 ];
--- a/toolkit/modules/PopupNotifications.jsm
+++ b/toolkit/modules/PopupNotifications.jsm
@@ -1222,17 +1222,20 @@ PopupNotifications.prototype = {
     // active notifications for the previous anchor as dismissed
     if (this.panel.state != "closed" && anchor != this._currentAnchorElement) {
       this._dismissOrRemoveCurrentNotifications();
     }
 
     // Ensure we move focus into the panel because it's opened through user interaction:
     this.panel.removeAttribute("noautofocus");
 
-    this._reshowNotifications(anchor);
+    // Avoid reshowing notifications that are already shown and have not been dismissed.
+    if (this.panel.state == "closed" || anchor != this._currentAnchorElement) {
+      this._reshowNotifications(anchor);
+    }
 
     // If the user re-selects the current notification, focus it.
     if (anchor == this._currentAnchorElement && this.panel.firstChild) {
       this.panel.firstChild.button.focus();
     }
   },
 
   _reshowNotifications: function PopupNotifications_reshowNotifications(anchor, browser) {