Bug 572967: don't remove notifications once dismissed, and instead allow them to be re-opened from the anchor icon, r=dolske, a=Mossop
☠☠ backed out by f2d6cc57c80c ☠ ☠
authorGavin Sharp <gavin@gavinsharp.com>
Wed, 23 Jun 2010 12:53:09 -0400
changeset 48761 b46982cc0c0a82dd8fdbb9f9b57da382c5bd515f
parent 48760 da01163529c3fa13feb617eb5fc1ebba328bb5d8
child 48762 3b59889221bcb6d8ffc27500070dd26e1f08691b
push id14803
push usergsharp@mozilla.com
push dateTue, 03 Aug 2010 04:18:08 +0000
treeherdermozilla-central@bdb98838d6f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske, Mossop
bugs572967
milestone2.0b3pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 572967: don't remove notifications once dismissed, and instead allow them to be re-opened from the anchor icon, r=dolske, a=Mossop
browser/base/content/browser.css
browser/base/content/browser.js
browser/base/content/browser.xul
browser/themes/gnomestripe/browser/browser.css
browser/themes/pinstripe/browser/browser.css
browser/themes/winstripe/browser/browser.css
toolkit/content/PopupNotifications.jsm
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -320,14 +320,15 @@ window[chromehidden~="toolbar"] toolbar:
   margin: 0;
 }
 
 
 /* notification anchors should only be visible when their associated
    notifications are */
 .notification-anchor-icon {
   display: none;
+  -moz-user-focus: normal;
 }
 
 #notification-popup-box[anchorid="geo-notification-icon"] > #geo-notification-icon,
 #notification-popup-box[anchorid="addons-notification-icon"] > #addons-notification-icon {
   display: -moz-box;
 }
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -154,19 +154,23 @@ XPCOMUtils.defineLazyGetter(this, "Weave
   Cu.import("resource://services-sync/service.js", tmp);
   return tmp.Weave;
 });
 #endif
 
 XPCOMUtils.defineLazyGetter(this, "PopupNotifications", function () {
   let tmp = {};
   Cu.import("resource://gre/modules/PopupNotifications.jsm", tmp);
-  return new tmp.PopupNotifications(gBrowser,
-                                    document.getElementById("notification-popup"),
-                                    document.getElementById("notification-popup-box"));
+  try {
+    return new tmp.PopupNotifications(gBrowser,
+                                      document.getElementById("notification-popup"),
+                                      document.getElementById("notification-popup-box"));
+  } catch (ex) {
+    Cu.reportError(ex);
+  }
 });
 
 let gInitialPages = [
   "about:blank",
   "about:privatebrowsing",
   "about:sessionrestore"
 ];
 
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -674,18 +674,18 @@
                  ontextentered="this.handleCommand(param);"
                  ontextreverted="return this.handleRevert();"
                  pageproxystate="invalid"
                  onsearchbegin="LocationBarHelpers._searchBegin();"
                  onsearchcomplete="LocationBarHelpers._searchComplete();"
                  onfocus="document.getElementById('identity-box').style.MozUserFocus= 'normal'"
                  onblur="setTimeout(function() document.getElementById('identity-box').style.MozUserFocus = '', 0);">
           <box id="notification-popup-box" hidden="true" align="center">
-            <image id="geo-notification-icon" class="notification-anchor-icon"/>
-            <image id="addons-notification-icon" class="notification-anchor-icon"/>
+            <image id="geo-notification-icon" class="notification-anchor-icon" role="button"/>
+            <image id="addons-notification-icon" class="notification-anchor-icon" role="button"/>
           </box>
           <!-- Use onclick instead of normal popup= syntax since the popup
                code fires onmousedown, and hence eats our favicon drag events.
                We only add the identity-box button to the tab order when the location bar
                has focus, otherwise pressing F6 focuses it instead of the location bar -->
           <box id="identity-box" role="button"
                onclick="gIdentityHandler.handleIdentityButtonEvent(event);"
                onkeypress="gIdentityHandler.handleIdentityButtonEvent(event);">
--- a/browser/themes/gnomestripe/browser/browser.css
+++ b/browser/themes/gnomestripe/browser/browser.css
@@ -1002,16 +1002,20 @@ toolbar[iconsize="small"] #fullscreen-bu
   margin: 0 3px;
 }
 
 .notification-anchor-icon {
   width: 16px;
   height: 16px;
 }
 
+.notification-anchor-icon:focus {
+  outline: 1px dotted -moz-DialogText;
+}
+
 #geo-notification-icon {
   list-style-image: url(chrome://browser/skin/Geolocation-16.png);
 }
 
 #addons-notification-icon {
   list-style-image: url(chrome://mozapps/skin/extensions/extensionGeneric-16.png);
 }
 
--- a/browser/themes/pinstripe/browser/browser.css
+++ b/browser/themes/pinstripe/browser/browser.css
@@ -1858,16 +1858,21 @@ toolbarbutton.chevron > .toolbarbutton-m
 }
 
 .notification-anchor-icon {
   width: 16px;
   height: 16px;
   margin: 0 2px;
 }
 
+.notification-anchor-icon:focus {
+  -moz-box-shadow: 0 0 3px 1px -moz-mac-focusring inset,
+                   0 0 3px 2px -moz-mac-focusring;
+}
+
 #geo-notification-icon {
   list-style-image: url(chrome://browser/skin/Geolocation-16.png);
 }
 
 #addons-notification-icon {
   list-style-image: url(chrome://mozapps/skin/extensions/extensionGeneric-16.png);
 }
 
--- a/browser/themes/winstripe/browser/browser.css
+++ b/browser/themes/winstripe/browser/browser.css
@@ -1608,16 +1608,21 @@ toolbarbutton.bookmark-item[dragover="tr
   margin: 0 3px;
 }
 
 .notification-anchor-icon {
   width: 16px;
   height: 16px;
 }
 
+.notification-anchor-icon:focus {
+  outline: 1px dotted -moz-DialogText;
+  outline-offset: -3px;
+}
+
 #geo-notification-icon {
   list-style-image: url(chrome://browser/skin/Geolocation-16.png);
 }
 
 #addons-notification-icon {
   list-style-image: url(chrome://mozapps/skin/extensions/extensionGeneric-16.png);
 }
 
--- a/toolkit/content/PopupNotifications.jsm
+++ b/toolkit/content/PopupNotifications.jsm
@@ -64,17 +64,17 @@ Notification.prototype = {
    */
   remove: function Notification_remove() {
     this.owner.remove(this);
   },
 
   get anchorElement() {
     let anchorElement = null;
     if (this.anchorID)
-      anchorElement = this.owner.window.document.getElementById(this.anchorID);
+      anchorElement = this.owner.iconBox.querySelector("#"+this.anchorID);
 
     if (!anchorElement)
       anchorElement = this.owner.iconBox;
 
     return anchorElement;
   }
 };
 
@@ -84,27 +84,42 @@ Notification.prototype = {
  * @param tabbrowser
  *        window's <xul:tabbrowser/>. Used to observe tab switching events and
  *        for determining the active browser element.
  * @param panel
  *        The <xul:panel/> element to use for notifications. The panel is
  *        populated with <popupnotification> children and displayed it as
  *        needed.
  * @param iconBox
- *        Optional reference to a container element that should be hidden or
- *        unhidden when notifications are hidden or shown. Used as a fallback
- *        popup anchor if notifications do not specify anchor IDs.
+ *        Reference to a container element that should be hidden or
+ *        unhidden when notifications are hidden or shown. It should be the
+ *        parent of anchor elements whose IDs are passed to show().
+ *        It is used as a fallback popup anchor if notifications specify
+ *        invalid or non-existent anchor IDs.
  */
 function PopupNotifications(tabbrowser, panel, iconBox) {
+  if (!(tabbrowser instanceof Ci.nsIDOMXULElement))
+    throw "Invalid tabbrowser";
+  if (!(iconBox instanceof Ci.nsIDOMXULElement))
+    throw "Invalid iconBox";
+  if (!(panel instanceof Ci.nsIDOMXULElement))
+    throw "Invalid panel";
+
   this.window = tabbrowser.ownerDocument.defaultView;
   this.panel = panel;
   this.iconBox = iconBox;
   this.tabbrowser = tabbrowser;
 
   let self = this;
+  this.iconBox.addEventListener("click", function (event) {
+    self._onIconBoxCommand(event);
+  }, false);
+  this.iconBox.addEventListener("keypress", function (event) {
+    self._onIconBoxCommand(event);
+  }, false);
   this.panel.addEventListener("popuphidden", function (event) {
     self._onPopupHidden(event);
   }, true);
 
   function updateFromListeners() {
     // setTimeout(..., 0) needed, otherwise openPopup from "activate" event
     // handler results in the popup being hidden again for some reason...
     self.window.setTimeout(function () {
@@ -166,16 +181,19 @@ PopupNotifications.prototype = {
    *        dropdown menu.
    * @param options
    *        An options JavaScript object holding additional properties for the
    *        notification. The following properties are currently supported:
    *        persistence: An integer. The notification will not automatically
    *                     dismiss for this many page loads.
    *        timeout:     A time in milliseconds. The notification will not
    *                     automatically dismiss before this time.
+   *        dismissed:   Whether the notification should be added as a dismissed
+   *                     notification. Dismissed notifications can be activated
+   *                     by clicking on their anchorElement.
    * @returns the Notification object corresponding to the added notification.
    */
   show: function PopupNotifications_show(browser, id, message, anchorID,
                                          mainAction, secondaryActions, options) {
     function isInvalidAction(a) {
       return !a || !(typeof(a.callback) == "function") || !a.label || !a.accessKey;
     }
 
@@ -188,16 +206,18 @@ PopupNotifications.prototype = {
     if (mainAction && isInvalidAction(mainAction))
       throw "PopupNotifications_show: invalid mainAction";
     if (secondaryActions && secondaryActions.some(isInvalidAction))
       throw "PopupNotifications_show: invalid secondaryActions";
 
     let notification = new Notification(id, message, anchorID, mainAction,
                                         secondaryActions, browser, this, options);
 
+    if (options && options.dismissed)
+      notification.dismissed = true;
 
     let existingNotification = this.getNotification(id, browser);
     if (existingNotification)
       this._remove(existingNotification);
 
     let notifications = this._getNotificationsForBrowser(browser);
     notifications.push(notification);
 
@@ -361,60 +381,88 @@ PopupNotifications.prototype = {
   },
 
   /**
    * Updates the notification state in response to window activation or tab
    * selection changes.
    */
   _update: function PopupNotifications_update(anchor) {
     let anchorElement, notificationsToShow = [];
-    if (this._currentNotifications.length > 0) {
+    let haveNotifications = this._currentNotifications.length > 0;
+    if (haveNotifications) {
       // Only show the notifications that have the passed-in anchor (or the
       // first notification's anchor, if none was passed in). Other
       // notifications will be shown once these are dismissed.
       anchorElement = anchor || this._currentNotifications[0].anchorElement;
 
-      if (this.iconBox) {
-        this.iconBox.hidden = false;
-        this.iconBox.setAttribute("anchorid", anchorElement.id);
-      }
+      this.iconBox.hidden = false;
+      this.iconBox.setAttribute("anchorid", anchorElement.id);
 
+      // Also filter out notifications that have been dismissed.
       notificationsToShow = this._currentNotifications.filter(function (n) {
-        return n.anchorElement == anchorElement;
+        return !n.dismissed && n.anchorElement == anchorElement;
       });
     }
 
     if (notificationsToShow.length > 0) {
       this._showPanel(notificationsToShow, anchorElement);
     } else {
       // Notify observers that we're not showing the popup (useful for testing)
       this._notify("updateNotShowing");
 
       this._hidePanel();
 
-      if (this.iconBox)
+      // Only hide the iconBox if we actually have no notifications (as opposed
+      // to not having any showable notifications)
+      if (this.iconBox && !haveNotifications)
         this.iconBox.hidden = true;
     }
   },
 
   _getNotificationsForBrowser: function PopupNotifications_getNotifications(browser) {
     if (browser.popupNotifications)
       return browser.popupNotifications;
 
     return browser.popupNotifications = [];
   },
 
+  _onIconBoxCommand: function PopupNotifications_onIconBoxCommand(event) {
+    // Left click, space or enter only
+    let type = event.type;
+    if (type == "click" && event.button != 0)
+      return;
+
+    if (type == "keypress" &&
+        !(event.charCode == Ci.nsIDOMKeyEvent.DOM_VK_SPACE ||
+          event.keyCode == Ci.nsIDOMKeyEvent.DOM_VK_RETURN))
+      return;
+
+    if (this._currentNotifications.length == 0)
+      return;
+
+    let anchor = event.originalTarget;
+
+    // Mark notifications anchored to this anchor as un-dismissed
+    this._currentNotifications.forEach(function (n) {
+      if (n.anchorElement == anchor)
+        n.dismissed = false;
+    });
+
+    // ...and then show them.
+    this._update(anchor);
+  },
+
   _onPopupHidden: function PopupNotifications_onPopupHidden(event) {
     if (event.target != this.panel || this._ignoreDismissal)
       return;
 
-    // Remove notifications being dismissed
+    // Mark notifications as dismissed
     Array.forEach(this.panel.childNodes, function (nEl) {
       let notificationObj = nEl.notification;
-      this._remove(notificationObj);
+      notificationObj.dismissed = true;
     }, this);
 
     this._update();
   },
 
   _onButtonCommand: function PopupNotifications_onButtonCommand(event) {
     // Need to find the associated notification object, which is a bit tricky
     // since it isn't associated with the button directly - this is kind of