Bug 1282768 - Part 2 - Move the secondary actions of doorhanger notifications to their own menu button. r=paolo draft
authorPanos Astithas <past@mozilla.com>
Sun, 20 Nov 2016 18:40:22 +0100
changeset 441674 84b7ab2a16141c736dc0f6b84387fb1be4775b59
parent 441673 b5ccef53f8be35d8bd70ff5c7f9fbc74a631e954
child 441675 daf79cf7ab46e2605c1c86c0d2f0f6c9cd8c972e
push id36489
push userpaolo.mozmail@amadzone.org
push dateSun, 20 Nov 2016 17:49:09 +0000
reviewerspaolo
bugs1282768
milestone53.0a1
Bug 1282768 - Part 2 - Move the secondary actions of doorhanger notifications to their own menu button. r=paolo MozReview-Commit-ID: 2cnufF7wZJG
browser/base/content/test/popupNotifications/browser_popupNotification.js
browser/base/content/test/popupNotifications/browser_popupNotification_2.js
browser/base/content/test/popupNotifications/browser_popupNotification_4.js
browser/base/content/test/popupNotifications/browser_popupNotification_5.js
browser/base/content/test/popupNotifications/head.js
browser/base/content/test/webrtc/head.js
browser/components/nsBrowserGlue.js
browser/extensions/presentation/content/PresentationDevicePrompt.jsm
browser/modules/test/browser_PermissionUI.js
dom/indexedDB/test/browser_permissionsPromptDeny.js
dom/indexedDB/test/head.js
dom/notification/test/browser/browser_permission_dismiss.js
toolkit/components/passwordmgr/test/browser/head.js
toolkit/content/widgets/notification.xml
toolkit/locales/en-US/chrome/global/notification.dtd
toolkit/modules/PopupNotifications.jsm
toolkit/themes/osx/global/notification.css
toolkit/themes/shared/popupnotification.inc.css
--- a/browser/base/content/test/popupNotifications/browser_popupNotification.js
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification.js
@@ -42,16 +42,60 @@ var tests = [
       triggerSecondaryCommand(popup, 0);
     },
     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");
     }
   },
+  { id: "Test#2b",
+    run: function () {
+      this.notifyObj = new BasicNotification(this.id);
+      this.notifyObj.secondaryActions.push({
+        label: "Extra Secondary Action",
+        accessKey: "E",
+        callback: () => this.extraSecondaryActionClicked = true,
+      });
+      showNotification(this.notifyObj);
+    },
+    onShown: function (popup) {
+      checkPopup(popup, this.notifyObj);
+      triggerSecondaryCommand(popup, 1);
+    },
+    onHidden: function (popup) {
+      ok(this.extraSecondaryActionClicked, "extra secondary action was clicked");
+      ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
+      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
+    }
+  },
+  { id: "Test#2c",
+    run: function () {
+      this.notifyObj = new BasicNotification(this.id);
+      this.notifyObj.secondaryActions.push({
+        label: "Extra Secondary Action",
+        accessKey: "E",
+        callback: () => ok(false, "unexpected callback invocation"),
+      }, {
+        label: "Other Extra Secondary Action",
+        accessKey: "O",
+        callback: () => this.extraSecondaryActionClicked = true,
+      });
+      showNotification(this.notifyObj);
+    },
+    onShown: function (popup) {
+      checkPopup(popup, this.notifyObj);
+      triggerSecondaryCommand(popup, 2);
+    },
+    onHidden: function (popup) {
+      ok(this.extraSecondaryActionClicked, "extra secondary action was clicked");
+      ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback wasn't triggered");
+      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
+    }
+  },
   { id: "Test#3",
     run: function() {
       this.notifyObj = new BasicNotification(this.id);
       this.notification = showNotification(this.notifyObj);
     },
     onShown: function(popup) {
       checkPopup(popup, this.notifyObj);
       dismissNotification(popup);
--- a/browser/base/content/test/popupNotifications/browser_popupNotification_2.js
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification_2.js
@@ -182,32 +182,16 @@ var tests = [
       let promiseTopic = promiseTopicObserved("PopupNotifications-updateNotShowing");
       showNotification(notifyObj);
       yield promiseTopic;
       isnot(document.getElementById("geo-notification-icon").boxObject.width, 0,
             "geo anchor should be visible");
       goNext();
     }
   },
-  // Test notification "Not Now" menu item
-  { id: "Test#8",
-    run: function() {
-      this.notifyObj = new BasicNotification(this.id);
-      this.notification = showNotification(this.notifyObj);
-    },
-    onShown: function(popup) {
-      checkPopup(popup, this.notifyObj);
-      triggerSecondaryCommand(popup, 1);
-    },
-    onHidden: function(popup) {
-      ok(this.notifyObj.dismissalCallbackTriggered, "dismissal callback triggered");
-      this.notification.remove();
-      ok(this.notifyObj.removedCallbackTriggered, "removed callback triggered");
-    }
-  },
   // Test notification close button
   { id: "Test#9",
     run: function() {
       this.notifyObj = new BasicNotification(this.id);
       this.notification = showNotification(this.notifyObj);
     },
     onShown: function(popup) {
       checkPopup(popup, this.notifyObj);
--- a/browser/base/content/test/popupNotifications/browser_popupNotification_4.js
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification_4.js
@@ -155,33 +155,16 @@ var tests = [
                                               "The browser should be active");
       let fm = Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager);
       yield BrowserTestUtils.waitForCondition(() => fm.activeWindow == originalWindow,
                                               "The window should be active")
 
       goNext();
     }
   },
-  // the hideNotNow option
-  { id: "Test#7",
-    run: function() {
-      this.notifyObj = new BasicNotification(this.id);
-      this.notifyObj.options.hideNotNow = true;
-      this.notifyObj.mainAction.dismiss = true;
-      this.notification = showNotification(this.notifyObj);
-    },
-    onShown: function(popup) {
-      // checkPopup verifies that the Not Now item is hidden, and that no separator is added.
-      checkPopup(popup, this.notifyObj);
-      triggerMainCommand(popup);
-    },
-    onHidden: function(popup) {
-      this.notification.remove();
-    }
-  },
   // the main action callback can keep the notification.
   { id: "Test#8",
     run: function() {
       this.notifyObj = new BasicNotification(this.id);
       this.notifyObj.mainAction.dismiss = true;
       this.notification = showNotification(this.notifyObj);
     },
     onShown: function(popup) {
--- a/browser/base/content/test/popupNotifications/browser_popupNotification_5.js
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification_5.js
@@ -96,17 +96,17 @@ var tests = [
 
       yield promiseTabLoadEvent(gBrowser.selectedTab, "http://example.org/");
       yield promiseTabLoadEvent(gBrowser.selectedTab, "http://example.com/");
 
       // This code should not be executed.
       ok(false, "Should have removed the notification after navigation");
       // Properly dismiss and cleanup in case the unthinkable happens.
       this.complete = true;
-      triggerSecondaryCommand(popup, 1);
+      triggerSecondaryCommand(popup, 0);
     },
     onHidden: function(popup) {
       ok(!this.complete, "Should have hidden the notification after navigation");
       this.notification.remove();
       gBrowser.removeTab(gBrowser.selectedTab);
       gBrowser.selectedTab = this.oldSelectedTab;
     }
   },
@@ -126,19 +126,19 @@ var tests = [
     onShown: function* (popup) {
       this.complete = false;
 
       // Notification should persist after attempt to dismiss by clicking on the
       // content area.
       let browser = gBrowser.selectedBrowser;
       yield BrowserTestUtils.synthesizeMouseAtCenter("body", {}, browser)
 
-      // Notification should be hidden after dismissal via Not Now.
+      // Notification should be hidden after dismissal via Don't Allow.
       this.complete = true;
-      triggerSecondaryCommand(popup, 1);
+      triggerSecondaryCommand(popup, 0);
     },
     onHidden: function(popup) {
       ok(this.complete, "Should have hidden the notification after clicking Not Now");
       this.notification.remove();
       gBrowser.removeTab(gBrowser.selectedTab);
       gBrowser.selectedTab = this.oldSelectedTab;
     }
   },
--- a/browser/base/content/test/popupNotifications/head.js
+++ b/browser/base/content/test/popupNotifications/head.js
@@ -208,35 +208,34 @@ function checkPopup(popup, notifyObj) {
   is(notification.getAttribute("label"), notifyObj.message, "message matches");
   is(notification.id, notifyObj.id + "-notification", "id matches");
   if (notifyObj.mainAction) {
     is(notification.getAttribute("buttonlabel"), notifyObj.mainAction.label,
        "main action label matches");
     is(notification.getAttribute("buttonaccesskey"),
        notifyObj.mainAction.accessKey, "main action accesskey matches");
   }
-  let actualSecondaryActions =
-    Array.filter(notification.childNodes, child => child.nodeName == "menuitem");
-  let secondaryActions = notifyObj.secondaryActions || [];
-  let actualSecondaryActionsCount = actualSecondaryActions.length;
-  if (notifyObj.options.hideNotNow) {
-    is(notification.getAttribute("hidenotnow"), "true", "'Not Now' item hidden");
-    if (secondaryActions.length)
-      is(notification.lastChild.tagName, "menuitem", "no menuseparator");
+  if (notifyObj.secondaryActions && notifyObj.secondaryActions.length > 0) {
+    let secondaryAction = notifyObj.secondaryActions[0];
+    is(notification.getAttribute("secondarybuttonlabel"), secondaryAction.label,
+       "secondary action label matches");
+    is(notification.getAttribute("secondarybuttonaccesskey"),
+       secondaryAction.accessKey, "secondary action accesskey matches");
   }
-  else if (secondaryActions.length) {
-    is(notification.lastChild.tagName, "menuseparator", "menuseparator exists");
-  }
-  is(actualSecondaryActionsCount, secondaryActions.length,
-    actualSecondaryActions.length + " secondary actions");
-  secondaryActions.forEach(function(a, i) {
-    is(actualSecondaryActions[i].getAttribute("label"), a.label,
-       "label for secondary action " + i + " matches");
-    is(actualSecondaryActions[i].getAttribute("accesskey"), a.accessKey,
-       "accessKey for secondary action " + i + " matches");
+  // Additional secondary actions appear as menu items.
+  let actualExtraSecondaryActions =
+    Array.filter(notification.childNodes, child => child.nodeName == "menuitem");
+  let extraSecondaryActions = notifyObj.secondaryActions ? notifyObj.secondaryActions.slice(1) : [];
+  is(actualExtraSecondaryActions.length, extraSecondaryActions.length,
+     "number of extra secondary actions matches");
+  extraSecondaryActions.forEach(function(a, i) {
+    is(actualExtraSecondaryActions[i].getAttribute("label"), a.label,
+       "label for extra secondary action " + i + " matches");
+    is(actualExtraSecondaryActions[i].getAttribute("accesskey"), a.accessKey,
+       "accessKey for extra secondary action " + i + " matches");
   });
 }
 
 XPCOMUtils.defineLazyGetter(this, "gActiveListeners", () => {
   let listeners = new Map();
   registerCleanupFunction(() => {
     for (let [listener, eventName] of listeners) {
       PopupNotifications.panel.removeEventListener(eventName, listener, false);
@@ -267,23 +266,30 @@ function triggerMainCommand(popup) {
 }
 
 function triggerSecondaryCommand(popup, index) {
   let notifications = popup.childNodes;
   ok(notifications.length > 0, "at least one notification displayed");
   let notification = notifications[0];
   info("Triggering secondary command for notification " + notification.id);
 
-  notification.button.nextSibling.nextSibling.focus();
+  if (index == 0) {
+    EventUtils.synthesizeMouseAtCenter(notification.secondaryButton, {});
+    return;
+  }
+
+  // Extra secondary actions appear in a menu.
+  notification.secondaryButton.nextSibling.nextSibling.focus();
 
   popup.addEventListener("popupshown", function handle() {
     popup.removeEventListener("popupshown", handle, false);
     info("Command popup open for notification " + notification.id);
-    // Press down until the desired command is selected
-    for (let i = 0; i <= index; i++) {
+    // Press down until the desired command is selected. Decrease index by one
+    // since the secondary action was handled above.
+    for (let i = 0; i <= index - 1; i++) {
       EventUtils.synthesizeKey("VK_DOWN", {});
     }
     // Activate
     EventUtils.synthesizeKey("VK_RETURN", {});
   }, false);
 
   // One down event to open the popup
   info("Open the popup to trigger secondary command for notification " + notification.id);
--- a/browser/base/content/test/webrtc/head.js
+++ b/browser/base/content/test/webrtc/head.js
@@ -300,23 +300,29 @@ function promiseNoPopupNotification(aNam
 }
 
 const kActionAlways = 1;
 const kActionDeny = 2;
 const kActionNever = 3;
 
 function activateSecondaryAction(aAction) {
   let notification = PopupNotifications.panel.firstChild;
-  notification.button.nextSibling.nextSibling.focus();
+
+  if (aAction == kActionAlways) {
+    notification.secondaryButton.click();
+    return;
+  }
+
+  notification.secondaryButton.nextSibling.nextSibling.focus();
   let popup = notification.menupopup;
   popup.addEventListener("popupshown", function() {
     popup.removeEventListener("popupshown", arguments.callee, false);
 
     // Press 'down' as many time as needed to select the requested action.
-    while (aAction--)
+    while (--aAction)
       EventUtils.synthesizeKey("VK_DOWN", {});
 
     // Activate
     EventUtils.synthesizeKey("VK_RETURN", {});
   }, false);
 
   // One down event to open the popup
   EventUtils.synthesizeKey("VK_DOWN",
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -2851,17 +2851,16 @@ var E10SAccessibilityCheck = {
       accessKey: win.gNavigatorBundle.getString("e10s.accessibilityNotice.enableAndRestart.accesskey"),
       callback: restartCallback,
     }];
     let options = {
       popupIconURL: "chrome://browser/skin/e10s-64@2x.png",
       learnMoreURL: Services.urlFormatter.formatURLPref("app.support.e10sAccessibilityUrl"),
       persistent: true,
       persistWhileVisible: true,
-      hideNotNow: true,
     };
 
     notification =
       win.PopupNotifications.show(browser, "a11y_enabled_with_e10s",
                                   promptMessage, null, mainAction,
                                   secondaryActions, options);
   },
 };
--- a/browser/extensions/presentation/content/PresentationDevicePrompt.jsm
+++ b/browser/extensions/presentation/content/PresentationDevicePrompt.jsm
@@ -71,17 +71,16 @@ PresentationPermissionPrompt.prototype =
   get browser() {
     return this.request.chromeEventHandler;
   },
   get principal() {
     return this.request.principal;
   },
   get popupOptions() {
     return {
-      hideNotNow: true,
       removeOnDismissal: true,
       popupIconURL: kNotificationPopupIcon, // Icon shown on prompt content
       eventCallback: (aTopic, aNewBrowser) => {
         log("eventCallback: " + aTopic);
         let handler = {
           // dismissed: () => { // Won't be fired if removeOnDismissal is true.
           //   log("Dismissed by user. Cancel the request.");
           // },
@@ -157,17 +156,17 @@ PresentationPermissionPrompt.prototype =
     }, {
       label: GetString("presentation.deviceprompt.cancel.label"),
       accessKey: GetString("presentation.deviceprompt.cancel.accessKey"),
       callback: () => {
         log("Cancel selection.");
         this._isResponded = true;
         this.request.cancel(Cr.NS_ERROR_NOT_AVAILABLE);
       },
-      dismiss: true, // For hideNotNow.
+      dismiss: true,
     }];
   },
   // PRIVATE APIs
   get _domainName() {
     if (this.principal.URI instanceof Ci.nsIFileURL) {
       return this.principal.URI.path.split('/')[1];
     }
     return this.principal.URI.hostPort;
--- a/browser/modules/test/browser_PermissionUI.js
+++ b/browser/modules/test/browser_PermissionUI.js
@@ -58,31 +58,28 @@ function clickMainAction() {
   let removePromise =
     BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popuphidden");
   let popupNotification = getPopupNotificationNode();
   popupNotification.button.click();
   return removePromise;
 }
 
 /**
- * For an opened PopupNotification, clicks on a secondary action,
+ * For an opened PopupNotification, clicks on the secondary action,
  * and waits for the panel to fully close.
  *
- * @param {int} index
- *        The 0-indexed index of the secondary menuitem to choose.
  * @return {Promise}
  *         Resolves once the panel has fired the "popuphidden"
  *         event.
  */
-function clickSecondaryAction(index) {
+function clickSecondaryAction() {
   let removePromise =
     BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popuphidden");
   let popupNotification = getPopupNotificationNode();
-  let menuitems = popupNotification.children;
-  menuitems[index].click();
+  popupNotification.secondaryButton.click();
   return removePromise;
 }
 
 /**
  * Makes sure that 1 (and only 1) <xul:popupnotification> is being displayed
  * by PopupNotification, and then returns that <xul:popupnotification>.
  *
  * @return {<xul:popupnotification>}
@@ -269,17 +266,17 @@ add_task(function* test_with_permission_
       Services.perms.testPermissionFromPrincipal(principal,
                                                  kTestPermissionKey);
     Assert.equal(curPerm, Ci.nsIPermissionManager.UNKNOWN_ACTION,
                  "Should be no permission set to begin with.");
 
     // First test denying the permission request.
     Assert.equal(notification.secondaryActions.length, 1,
                  "There should only be 1 secondary action");
-    yield clickSecondaryAction(0);
+    yield clickSecondaryAction();
     curPerm = Services.perms.testPermissionFromPrincipal(principal,
                                                          kTestPermissionKey);
     Assert.equal(curPerm, Ci.nsIPermissionManager.DENY_ACTION,
                  "Should have denied the action");
     Assert.ok(denied, "The secondaryAction callback should have fired");
     Assert.ok(!allowed, "The mainAction callback should not have fired");
     Assert.ok(mockRequest._cancelled,
               "The request should have been cancelled");
@@ -424,17 +421,17 @@ add_task(function* test_no_request() {
                  secondaryAction.accessKey,
                  "The secondary action should have the right access key");
     Assert.ok(notification.options.displayURI.equals(principal.URI),
               "Should be showing the URI of the requesting page");
 
     // First test denying the permission request.
     Assert.equal(notification.secondaryActions.length, 1,
                  "There should only be 1 secondary action");
-    yield clickSecondaryAction(0);
+    yield clickSecondaryAction();
     Assert.ok(denied, "The secondaryAction callback should have fired");
     Assert.ok(!allowed, "The mainAction callback should not have fired");
 
     shownPromise =
       BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
     TestPrompt.prompt();
     yield shownPromise;
 
--- a/dom/indexedDB/test/browser_permissionsPromptDeny.js
+++ b/dom/indexedDB/test/browser_permissionsPromptDeny.js
@@ -32,17 +32,17 @@ add_task(function test1() {
   gBrowser.selectedBrowser.loadURI(testPageURL);
   yield BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
 
   registerPopupEventHandler("popupshowing", function () {
     ok(true, "prompt showing");
   });
   registerPopupEventHandler("popupshown", function () {
     ok(true, "prompt shown");
-    triggerSecondaryCommand(this, 0);
+    triggerSecondaryCommand(this);
   });
   registerPopupEventHandler("popuphidden", function () {
     ok(true, "prompt hidden");
   });
 
   yield promiseMessage("InvalidStateError", gBrowser);
 
   is(getPermission(testPageURL, "indexedDB"),
--- a/dom/indexedDB/test/head.js
+++ b/dom/indexedDB/test/head.js
@@ -45,38 +45,23 @@ function triggerMainCommand(popup)
   let notifications = popup.childNodes;
   ok(notifications.length > 0, "at least one notification displayed");
   let notification = notifications[0];
   info("triggering command: " + notification.getAttribute("buttonlabel"));
 
   EventUtils.synthesizeMouseAtCenter(notification.button, {});
 }
 
-function triggerSecondaryCommand(popup, index)
+function triggerSecondaryCommand(popup)
 {
-  info("triggering secondary command, " + index);
+  info("triggering secondary command");
   let notifications = popup.childNodes;
   ok(notifications.length > 0, "at least one notification displayed");
   let notification = notifications[0];
-
-  notification.button.nextSibling.nextSibling.focus();
-
-  popup.addEventListener("popupshown", function () {
-    popup.removeEventListener("popupshown", arguments.callee, false);
-
-    // Press down until the desired command is selected
-    for (let i = 0; i <= index; i++)
-      EventUtils.synthesizeKey("VK_DOWN", {});
-
-    // Activate
-    EventUtils.synthesizeKey("VK_RETURN", {});
-  }, false);
-
-  // One down event to open the popup
-  EventUtils.synthesizeKey("VK_DOWN", { altKey: (navigator.platform.indexOf("Mac") == -1) });
+  EventUtils.synthesizeMouseAtCenter(notification.secondaryButton, {});
 }
 
 function dismissNotification(popup)
 {
   info("dismissing notification");
   executeSoon(function () {
     EventUtils.synthesizeKey("VK_ESCAPE", {});
   });
--- a/dom/notification/test/browser/browser_permission_dismiss.js
+++ b/dom/notification/test/browser/browser_permission_dismiss.js
@@ -9,30 +9,28 @@ const TEST_URL = "http://mochi.test:8888
 /**
  * Clicks the specified web-notifications prompt button.
  *
  * @param {Number} aButtonIndex Number indicating which button to click.
  *                              See the constants in this file.
  * @note modified from toolkit/components/passwordmgr/test/browser/head.js
  */
 function clickDoorhangerButton(aButtonIndex) {
-  ok(true, "Looking for action at index " + aButtonIndex);
-
   let popup = PopupNotifications.getNotification("web-notifications");
   let notifications = popup.owner.panel.childNodes;
   ok(notifications.length > 0, "at least one notification displayed");
   ok(true, notifications.length + " notification(s)");
   let notification = notifications[0];
 
-  if (aButtonIndex == -1) {
+  if (aButtonIndex == PROMPT_ALLOW_BUTTON) {
     ok(true, "Triggering main action");
     notification.button.doCommand();
-  } else if (aButtonIndex <= popup.secondaryActions.length) {
-    ok(true, "Triggering secondary action " + aButtonIndex);
-    notification.childNodes[aButtonIndex].doCommand();
+  } else {
+    ok(true, "Triggering secondary action");
+    notification.secondaryButton.doCommand();
   }
 }
 
 /**
  * Opens a tab which calls `Notification.requestPermission()` with a callback
  * argument, calls the `task` function while the permission prompt is open,
  * and verifies that the expected permission is set.
  *
--- a/toolkit/components/passwordmgr/test/browser/head.js
+++ b/toolkit/components/passwordmgr/test/browser/head.js
@@ -113,19 +113,22 @@ function clickDoorhangerButton(aPopup, a
   let notifications = aPopup.owner.panel.childNodes;
   ok(notifications.length > 0, "at least one notification displayed");
   ok(true, notifications.length + " notification(s)");
   let notification = notifications[0];
 
   if (aButtonIndex == 0) {
     ok(true, "Triggering main action");
     notification.button.doCommand();
+  } else if (aButtonIndex == 1) {
+    ok(true, "Triggering secondary action");
+    notification.secondaryButton.doCommand();
   } else if (aButtonIndex <= aPopup.secondaryActions.length) {
     ok(true, "Triggering secondary action " + aButtonIndex);
-    notification.childNodes[aButtonIndex].doCommand();
+    notification.childNodes[aButtonIndex - 1].doCommand();
   }
 }
 
 /**
  * Checks the doorhanger's username and password.
  *
  * @param {String} username The username.
  * @param {String} password The password.
--- a/toolkit/content/widgets/notification.xml
+++ b/toolkit/content/widgets/notification.xml
@@ -497,61 +497,62 @@
               <xul:label class="popup-notification-origin header"
                          xbl:inherits="value=origin,tooltiptext=origin"
                          crop="center"/>
               <xul:description class="popup-notification-description"
                                xbl:inherits="xbl:text=label,popupid"/>
             </xul:vbox>
             <xul:toolbarbutton anonid="closebutton"
                                class="messageCloseButton close-icon popup-notification-closebutton tabbable"
-                               xbl:inherits="oncommand=closebuttoncommand"
+                               xbl:inherits="oncommand=closebuttoncommand,hidden=closebuttonhidden"
                                tooltiptext="&closeNotification.tooltip;"/>
           </xul:hbox>
           <children includes="popupnotificationcontent"/>
           <xul:label class="text-link popup-notification-learnmore-link"
                      xbl:inherits="onclick=learnmoreclick,href=learnmoreurl">&learnMore;</xul:label>
           <xul:checkbox anonid="checkbox"
                         xbl:inherits="hidden=checkboxhidden,checked=checkboxchecked,label=checkboxlabel,oncommand=checkboxcommand" />
           <xul:description class="popup-notification-warning" xbl:inherits="hidden=warninghidden,xbl:text=warninglabel"/>
         </xul:vbox>
       </xul:hbox>
-      <xul:hbox pack="end" class="popup-notification-button-container">
+      <xul:hbox class="popup-notification-button-container">
         <children includes="button"/>
-        <xul:hbox class="popup-notification-button-wrapper">
-          <xul:button anonid="button"
-                      class="popup-notification-button"
-                      default="true"
-                      xbl:inherits="oncommand=buttoncommand,label=buttonlabel,accesskey=buttonaccesskey,disabled=mainactiondisabled"/>
-          <xul:toolbarseparator/>
-          <xul:button type="menu"
-                      class="popup-notification-button popup-notification-dropmarker"
-                      xbl:inherits="onpopupshown=buttonpopupshown">
-            <xul:menupopup anonid="menupopup"
-                           position="after_end"
-                           xbl:inherits="oncommand=menucommand">
-              <children/>
-              <xul:menuitem class="menuitem-iconic popup-notification-closeitem"
-                            label="&closeNotificationItem.label;"
-                            xbl:inherits="oncommand=closeitemcommand,hidden=hidenotnow"/>
-            </xul:menupopup>
-          </xul:button>
-        </xul:hbox>
+        <xul:button anonid="secondarybutton"
+                    class="popup-notification-button"
+                    xbl:inherits="oncommand=secondarybuttoncommand,label=secondarybuttonlabel,accesskey=secondarybuttonaccesskey,hidden=secondarybuttonhidden"/>
+        <xul:toolbarseparator xbl:inherits="hidden=dropmarkerhidden"/>
+        <xul:button type="menu"
+                    class="popup-notification-button popup-notification-dropmarker"
+                    xbl:inherits="onpopupshown=dropmarkerpopupshown,hidden=dropmarkerhidden">
+          <xul:menupopup anonid="menupopup"
+                         position="after_end"
+                         xbl:inherits="oncommand=menucommand">
+            <children/>
+          </xul:menupopup>
+        </xul:button>
+        <xul:button anonid="button"
+                    class="popup-notification-button"
+                    default="true"
+                    xbl:inherits="oncommand=buttoncommand,label=buttonlabel,accesskey=buttonaccesskey,disabled=mainactiondisabled"/>
       </xul:hbox>
     </content>
     <resources>
       <stylesheet src="chrome://global/skin/notification.css"/>
     </resources>
     <implementation>
       <field name="checkbox" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "checkbox");
       </field>
       <field name="closebutton" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "closebutton");
       </field>
       <field name="button" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "button");
       </field>
+      <field name="secondaryButton" readonly="true">
+        document.getAnonymousElementByAttribute(this, "anonid", "secondarybutton");
+      </field>
       <field name="menupopup" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "menupopup");
       </field>
     </implementation>
   </binding>
 </bindings>
--- a/toolkit/locales/en-US/chrome/global/notification.dtd
+++ b/toolkit/locales/en-US/chrome/global/notification.dtd
@@ -1,11 +1,9 @@
 <!-- This Source Code Form is subject to the terms of the Mozilla Public
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <!ENTITY closeNotification.tooltip "Close this message">
 
-<!ENTITY closeNotificationItem.label "Not Now">
-
 <!ENTITY checkForUpdates "Check for updates…">
 
 <!ENTITY learnMore "Learn more…">
--- a/toolkit/modules/PopupNotifications.jsm
+++ b/toolkit/modules/PopupNotifications.jsm
@@ -26,17 +26,16 @@ const PREF_SECURITY_DELAY = "security.no
 const TELEMETRY_STAT_OFFERED = 0;
 const TELEMETRY_STAT_ACTION_1 = 1;
 const TELEMETRY_STAT_ACTION_2 = 2;
 const TELEMETRY_STAT_ACTION_3 = 3;
 const TELEMETRY_STAT_ACTION_LAST = 4;
 const TELEMETRY_STAT_DISMISSAL_CLICK_ELSEWHERE = 5;
 const TELEMETRY_STAT_DISMISSAL_LEAVE_PAGE = 6;
 const TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON = 7;
-const TELEMETRY_STAT_DISMISSAL_NOT_NOW = 8;
 const TELEMETRY_STAT_OPEN_SUBMENU = 10;
 const TELEMETRY_STAT_LEARN_MORE = 11;
 
 const TELEMETRY_STAT_REOPENED_OFFSET = 20;
 
 var popupNotificationsMap = new WeakMap();
 var gNotificationParents = new WeakMap;
 
@@ -347,16 +346,18 @@ PopupNotifications.prototype = {
    *                                 will be removed.
    *        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).
+   *        hideClose:   Indicate that the little close button in the corner of
+   *                     the panel should be hidden.
    *        checkbox:    An object that allows you to add a checkbox and
    *                     control its behavior with these fields:
    *                       label:
    *                         (required) Label to be shown next to the checkbox.
    *                       checked:
    *                         (optional) Whether the checkbox should be checked
    *                         by default. Defaults to false.
    *                       checkedState:
@@ -367,21 +368,16 @@ PopupNotifications.prototype = {
    *                             Defaults to false.
    *                           warningLabel:
    *                             (optional) A (warning) text that is shown below the
    *                             checkbox. Pass null to hide.
    *                       uncheckedState:
    *                         (optional) An object that allows you to customize
    *                         the notification state when the checkbox is not checked.
    *                         Has the same attributes as checkedState.
-   *        hideNotNow:  If true, indicates that the 'Not Now' menuitem should
-   *                     not be shown. If 'Not Now' is hidden, it needs to be
-   *                     replaced by another 'do nothing' item, so providing at
-   *                     least one secondary action is required; and one of the
-   *                     actions needs to have the 'dismiss' property set to true.
    *        popupIconClass:
    *                     A string. A class (or space separated list of classes)
    *                     that will be applied to the icon in the popup so that
    *                     several notifications using the same panel can use
    *                     different icons.
    *        popupIconURL:
    *                     A string. URL of the image to be displayed in the popup.
    *                     Normally specified in CSS using list-style-image and the
@@ -406,20 +402,16 @@ PopupNotifications.prototype = {
     if (!browser)
       throw "PopupNotifications_show: invalid browser";
     if (!id)
       throw "PopupNotifications_show: invalid ID";
     if (mainAction && isInvalidAction(mainAction))
       throw "PopupNotifications_show: invalid mainAction";
     if (secondaryActions && secondaryActions.some(isInvalidAction))
       throw "PopupNotifications_show: invalid secondaryActions";
-    if (options && options.hideNotNow &&
-        (!secondaryActions || !secondaryActions.length ||
-         !secondaryActions.concat(mainAction).some(action => action.dismiss)))
-      throw "PopupNotifications_show: 'Not Now' item hidden without replacement";
 
     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);
@@ -592,18 +584,17 @@ PopupNotifications.prototype = {
   _dismiss: function PopupNotifications_dismiss(telemetryReason) {
     if (telemetryReason) {
       this.nextDismissReason = telemetryReason;
     }
 
     // An explicitly dismissed persistent notification effectively becomes
     // non-persistent.
     if (this.panel.firstChild &&
-        (telemetryReason == TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON ||
-         telemetryReason == TELEMETRY_STAT_DISMISSAL_NOT_NOW)) {
+        telemetryReason == TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON) {
       this.panel.firstChild.notification.options.persistent = false;
     }
 
     let browser = this.panel.firstChild &&
                   this.panel.firstChild.notification.browser;
     this.panel.hidePopup();
     if (browser)
       browser.focus();
@@ -682,28 +673,26 @@ PopupNotifications.prototype = {
       popupnotification.setAttribute("label", n.message);
       popupnotification.setAttribute("id", popupnotificationID);
       popupnotification.setAttribute("popupid", n.id);
       popupnotification.setAttribute("closebuttoncommand", `PopupNotifications._dismiss(${TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON});`);
       if (n.mainAction) {
         popupnotification.setAttribute("buttonlabel", n.mainAction.label);
         popupnotification.setAttribute("buttonaccesskey", n.mainAction.accessKey);
         popupnotification.setAttribute("buttoncommand", "PopupNotifications._onButtonEvent(event, 'buttoncommand');");
-        popupnotification.setAttribute("buttonpopupshown", "PopupNotifications._onButtonEvent(event, 'buttonpopupshown');");
+        popupnotification.setAttribute("dropmarkerpopupshown", "PopupNotifications._onButtonEvent(event, 'dropmarkerpopupshown');");
         popupnotification.setAttribute("learnmoreclick", "PopupNotifications._onButtonEvent(event, 'learnmoreclick');");
         popupnotification.setAttribute("menucommand", "PopupNotifications._onMenuCommand(event);");
-        popupnotification.setAttribute("closeitemcommand", `PopupNotifications._dismiss(${TELEMETRY_STAT_DISMISSAL_NOT_NOW});event.stopPropagation();`);
       } else {
         popupnotification.removeAttribute("buttonlabel");
         popupnotification.removeAttribute("buttonaccesskey");
         popupnotification.removeAttribute("buttoncommand");
-        popupnotification.removeAttribute("buttonpopupshown");
+        popupnotification.removeAttribute("dropmarkerpopupshown");
         popupnotification.removeAttribute("learnmoreclick");
         popupnotification.removeAttribute("menucommand");
-        popupnotification.removeAttribute("closeitemcommand");
       }
 
       if (n.options.popupIconClass) {
         let classes = "popup-notification-icon " + n.options.popupIconClass;
         popupnotification.setAttribute("iconclass", classes);
       }
       if (n.options.popupIconURL)
         popupnotification.setAttribute("icon", n.options.popupIconURL);
@@ -724,45 +713,53 @@ PopupNotifications.prototype = {
           popupnotification.setAttribute("origin", uri);
         } catch (e) {
           Cu.reportError(e);
           popupnotification.removeAttribute("origin");
         }
       } else
         popupnotification.removeAttribute("origin");
 
+      if (n.options.hideClose)
+        popupnotification.setAttribute("closebuttonhidden", "true");
+
       popupnotification.notification = n;
 
-      if (n.secondaryActions) {
+      if (n.secondaryActions && n.secondaryActions.length > 0) {
         let telemetryStatId = TELEMETRY_STAT_ACTION_2;
 
-        n.secondaryActions.forEach(function(a) {
+        let secondaryAction = n.secondaryActions[0];
+        popupnotification.setAttribute("secondarybuttonlabel", secondaryAction.label);
+        popupnotification.setAttribute("secondarybuttonaccesskey", secondaryAction.accessKey);
+        popupnotification.setAttribute("secondarybuttoncommand", "PopupNotifications._onButtonEvent(event, 'secondarybuttoncommand');");
+
+        for (let i = 1; i < n.secondaryActions.length; i++) {
+          let action = n.secondaryActions[i];
           let item = doc.createElementNS(XUL_NS, "menuitem");
-          item.setAttribute("label", a.label);
-          item.setAttribute("accesskey", a.accessKey);
+          item.setAttribute("label", action.label);
+          item.setAttribute("accesskey", action.accessKey);
           item.notification = n;
-          item.action = a;
+          item.action = action;
 
           popupnotification.appendChild(item);
 
           // We can only record a limited number of actions in telemetry. If
           // there are more, the latest are all recorded in the last bucket.
           item.action.telemetryStatId = telemetryStatId;
           if (telemetryStatId < TELEMETRY_STAT_ACTION_LAST) {
             telemetryStatId++;
           }
-        }, this);
-
-        if (n.options.hideNotNow) {
-          popupnotification.setAttribute("hidenotnow", "true");
         }
-        else if (n.secondaryActions.length) {
-          let closeItemSeparator = doc.createElementNS(XUL_NS, "menuseparator");
-          popupnotification.appendChild(closeItemSeparator);
+
+        if (n.secondaryActions.length < 2) {
+          popupnotification.setAttribute("dropmarkerhidden", "true");
         }
+      } else {
+        popupnotification.setAttribute("secondarybuttonhidden", "true");
+        popupnotification.setAttribute("dropmarkerhidden", "true");
       }
 
       let checkbox = n.options.checkbox;
       if (checkbox && checkbox.label) {
         let checked = n._checkboxChecked != null ? n._checkboxChecked : !!checkbox.checked;
 
         popupnotification.setAttribute("checkboxhidden", "false");
         popupnotification.setAttribute("checkboxchecked", checked);
@@ -783,21 +780,24 @@ PopupNotifications.prototype = {
 
       // The popupnotification may be hidden if we got it from the chrome
       // document rather than creating it ad hoc.
       popupnotification.hidden = false;
     }, this);
   },
 
   _setNotificationUIState(notification, state = {}) {
-    notification.setAttribute("mainactiondisabled", state.disableMainAction || "false");
-
+    if (state.disableMainAction) {
+      notification.setAttribute("mainactiondisabled", "true");
+    } else {
+      notification.removeAttribute("mainactiondisabled");
+    }
     if (state.warningLabel) {
       notification.setAttribute("warninglabel", state.warningLabel);
-      notification.setAttribute("warninghidden", "false");
+      notification.removeAttribute("warninghidden");
     } else {
       notification.setAttribute("warninghidden", "true");
     }
   },
 
   _onCheckboxCommand(event) {
     let notificationEl = getNotificationFromElement(event.originalTarget);
     let checked = notificationEl.checkbox.checked;
@@ -1272,54 +1272,66 @@ PopupNotifications.prototype = {
     if (!notificationEl)
       throw "PopupNotifications._onButtonEvent: couldn't find notification element";
 
     if (!notificationEl.notification)
       throw "PopupNotifications._onButtonEvent: couldn't find notification";
 
     let notification = notificationEl.notification;
 
-    if (type == "buttonpopupshown") {
+    if (type == "dropmarkerpopupshown") {
       notification._recordTelemetryStat(TELEMETRY_STAT_OPEN_SUBMENU);
       return;
     }
 
     if (type == "learnmoreclick") {
       notification._recordTelemetryStat(TELEMETRY_STAT_LEARN_MORE);
       return;
     }
 
-    // Record the total timing of the main action since the notification was
-    // created, even if the notification was dismissed in the meantime.
-    let timeSinceCreated = this.window.performance.now() - notification.timeCreated;
-    if (!notification.recordedTelemetryMainAction) {
-      notification.recordedTelemetryMainAction = true;
-      notification._recordTelemetry("POPUP_NOTIFICATION_MAIN_ACTION_MS",
-                                    timeSinceCreated);
+    if (type == "buttoncommand") {
+      // Record the total timing of the main action since the notification was
+      // created, even if the notification was dismissed in the meantime.
+      let timeSinceCreated = this.window.performance.now() - notification.timeCreated;
+      if (!notification.recordedTelemetryMainAction) {
+        notification.recordedTelemetryMainAction = true;
+        notification._recordTelemetry("POPUP_NOTIFICATION_MAIN_ACTION_MS",
+                                      timeSinceCreated);
+      }
     }
 
-    let timeSinceShown = this.window.performance.now() - notification.timeShown;
-    if (timeSinceShown < this.buttonDelay) {
-      Services.console.logStringMessage("PopupNotifications._onButtonEvent: " +
-                                        "Button click happened before the security delay: " +
-                                        timeSinceShown + "ms");
-      return;
+    if (type == "buttoncommand" || type == "secondarybuttoncommand") {
+      let timeSinceShown = this.window.performance.now() - notification.timeShown;
+      if (timeSinceShown < this.buttonDelay) {
+        Services.console.logStringMessage("PopupNotifications._onButtonEvent: " +
+                                          "Button click happened before the security delay: " +
+                                          timeSinceShown + "ms");
+        return;
+      }
     }
 
-    notification._recordTelemetryStat(TELEMETRY_STAT_ACTION_1);
+    let action = notification.mainAction;
+    let telemetryStatId = TELEMETRY_STAT_ACTION_1;
+
+    if (type == "secondarybuttoncommand") {
+      action = notification.secondaryActions[0];
+      telemetryStatId = TELEMETRY_STAT_ACTION_2;
+    }
+
+    notification._recordTelemetryStat(telemetryStatId);
 
     try {
-      notification.mainAction.callback.call(undefined, {
+      action.callback.call(undefined, {
         checkboxChecked: notificationEl.checkbox.checked
       });
     } catch (error) {
       Cu.reportError(error);
     }
 
-    if (notification.mainAction.dismiss) {
+    if (action.dismiss) {
       this._dismiss();
       return;
     }
 
     this._remove(notification);
     this._update();
   },
 
--- a/toolkit/themes/osx/global/notification.css
+++ b/toolkit/themes/osx/global/notification.css
@@ -105,16 +105,11 @@ notification[type="info"]:not([value="tr
 
 %include ../../shared/popupnotification.inc.css
 
 .popup-notification-button:focus {
   outline: 2px -moz-mac-focusring solid;
   outline-offset: -2px;
 }
 
-/* This is changed due to hover transparency looking invisible when buttons are hovered */
-.popup-notification-button-wrapper {
-  --panel-separator-color: white;
-}
-
 .popup-notification-warning {
   color: #aa1b08;
 }
--- a/toolkit/themes/shared/popupnotification.inc.css
+++ b/toolkit/themes/shared/popupnotification.inc.css
@@ -38,78 +38,100 @@
 
 .popup-notification-learnmore-link {
   margin-top: .5em !important;
 }
 
 .popup-notification-button-container {
   background-color: var(--arrowpanel-dimmed);
   border-top: 1px solid var(--panel-separator-color);
+  display: flex;
+  justify-content: flex-end;
 }
 
-.popup-notification-button-wrapper {
-  background-color: #0996f8;
-}
-
-.popup-notification-button-wrapper > toolbarseparator {
+.popup-notification-button-container > toolbarseparator {
   -moz-appearance: none;
   border: 0;
   border-left: 1px solid var(--panel-separator-color);
   margin: 7px 0 7px;
   min-width: 0;
 }
 
-.popup-notification-button-wrapper:hover > toolbarseparator {
+.popup-notification-button-container:hover > toolbarseparator {
   margin: 0;
 }
 
 .popup-notification-button {
+  flex: 1;
   -moz-appearance: none;
   background-color: transparent;
-  color: white;
+  color: inherit;
   margin: 0;
-  padding: 0 18px;
+  padding: 0;
+  min-width: 0;
   min-height: 40px;
-  min-width: 0;
   border: none;
 }
 
-.popup-notification-button:hover {
+.popup-notification-button:hover:not([disabled]) {
   outline: 1px solid var(--arrowpanel-dimmed);
+  background-color: var(--arrowpanel-dimmed);
+}
+
+.popup-notification-button:hover:active:not([disabled]) {
+  outline: 1px solid var(--arrowpanel-dimmed-further);
+  background-color: var(--arrowpanel-dimmed-further);
+  box-shadow: 0 1px 0 hsla(210,4%,10%,.05) inset;
+}
+
+.popup-notification-button[disabled] {
+  background-color: var(--arrowpanel-dimmed-further);
+  color: graytext;
+}
+
+.popup-notification-button[default] {
+  flex: 0 50%;
+}
+
+.popup-notification-button[default]:not([disabled]) {
+  background-color: #0996f8;
+  color: white;
+}
+
+.popup-notification-button[default]:hover:not([disabled]) {
   background-color: #0675d3;
 }
 
-.popup-notification-button:hover:active {
-  outline: 1px solid var(--arrowpanel-dimmed-further);
+.popup-notification-button[default]:hover:active:not([disabled]) {
   background-color: #0568ba;
-  box-shadow: 0 1px 0 hsla(210,4%,10%,.05) inset;
+}
+
+.popup-notification-button > .button-box {
+  padding: 0;
+  margin: 0;
+  /* prevent double border on windows when focused */
+  border: none;
 }
 
 .popup-notification-dropmarker {
+  flex: none;
   padding: 0 15px;
 }
 
-/* prevent double border on windows when focused */
-.popup-notification-button > .button-box {
-  border: none;
-}
-
 .popup-notification-dropmarker > .button-box > hbox {
   display: none;
 }
 
 .popup-notification-dropmarker > .button-box > .button-menu-dropmarker {
   /* This is to override the linux !important */
   -moz-appearance: none !important;
   display: -moz-box;
+  padding: 0;
+  margin: 0;
 }
 
 .popup-notification-dropmarker > .button-box > .button-menu-dropmarker > .dropmarker-icon {
   width: 16px;
   height: 16px;
   list-style-image: url(chrome://global/skin/icons/menubutton-dropmarker.svg);
   filter: url(chrome://global/skin/filters.svg#fill);
-  fill: white;
+  fill: currentColor;
 }
-
-.popup-notification-button[disabled] {
-  opacity: 0.5;
-}