Bug 1282768 - Allow some notifications to display the secondary action outside of the drop-down menu draft
authorPanos Astithas <past@mozilla.com>
Thu, 18 Aug 2016 01:05:41 +0300
changeset 402004 0cf2c63f237edbe66cd3ab7a3083efb6374f0d8e
parent 402003 cb81b0c15b605f4e6bea3081223d8671453f5b61
child 528623 792883176e851148cc395c7451ea8c20416d701f
push id26603
push userbmo:past@mozilla.com
push dateWed, 17 Aug 2016 22:07:14 +0000
bugs1282768
milestone51.0a1
Bug 1282768 - Allow some notifications to display the secondary action outside of the drop-down menu MozReview-Commit-ID: 2cnufF7wZJG
browser/base/content/test/popupNotifications/browser_popupNotification_4.js
browser/base/content/test/popupNotifications/head.js
browser/components/nsBrowserGlue.js
browser/themes/linux/browser.css
browser/themes/osx/browser.css
browser/themes/shared/downloads/downloads.inc.css
browser/themes/windows/browser.css
toolkit/content/widgets/notification.xml
toolkit/modules/PopupNotifications.jsm
toolkit/themes/shared/popupnotification.inc.css
--- a/browser/base/content/test/popupNotifications/browser_popupNotification_4.js
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification_4.js
@@ -143,33 +143,16 @@ var tests = [
       });
 
       checkPopup(win.PopupNotifications.panel, notifyObj);
       ok(notifyObj.swappingCallbackTriggered, "the swapping callback was triggered");
       win.close();
       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/head.js
+++ b/browser/base/content/test/popupNotifications/head.js
@@ -212,24 +212,16 @@ function checkPopup(popup, notifyObj) {
        "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");
-  }
-  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");
   });
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -2992,17 +2992,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/themes/linux/browser.css
+++ b/browser/themes/linux/browser.css
@@ -28,16 +28,18 @@
 
   --toolbarbutton-active-background: rgba(154,154,154,.5) linear-gradient(rgba(255,255,255,.7), rgba(255,255,255,.4));
   --toolbarbutton-active-bordercolor: rgba(0,0,0,.3);
   --toolbarbutton-active-boxshadow: 0 1px 1px rgba(0,0,0,.1) inset, 0 0 1px rgba(0,0,0,.3) inset;
 
   --toolbarbutton-checkedhover-backgroundcolor: rgba(200,200,200,.5);
 
   --panel-separator-color: ThreeDShadow;
+  --panel-background-color: hsla(210,4%,10%,.07);
+  --panel-background-active-color: hsla(210,4%,10%,.12);
 
   --urlbar-separator-color: ThreeDShadow;
 }
 
 #menubar-items {
   -moz-box-orient: vertical; /* for flex hack */
 }
 
--- a/browser/themes/osx/browser.css
+++ b/browser/themes/osx/browser.css
@@ -39,16 +39,18 @@
   --urlbar-dropmarker-url: url("chrome://browser/skin/urlbar-history-dropmarker.png");
   --urlbar-dropmarker-region: rect(0, 11px, 14px, 0);
   --urlbar-dropmarker-active-region: rect(0, 22px, 14px, 11px);
   --urlbar-dropmarker-2x-url: url("chrome://browser/skin/urlbar-history-dropmarker@2x.png");
   --urlbar-dropmarker-2x-region: rect(0, 22px, 28px, 0);
   --urlbar-dropmarker-active-2x-region: rect(0, 44px, 28px, 22px);
 
   --panel-separator-color: hsla(210,4%,10%,.14);
+  --panel-background-color: hsla(210,4%,10%,.07);
+  --panel-background-active-color: hsla(210,4%,10%,.12);
 
   --urlbar-separator-color: hsla(0,0%,16%,.2);
 }
 
 #urlbar:-moz-lwtheme:not([focused="true"]),
 .searchbar-textbox:-moz-lwtheme:not([focused="true"]) {
   opacity: .9;
 }
--- a/browser/themes/shared/downloads/downloads.inc.css
+++ b/browser/themes/shared/downloads/downloads.inc.css
@@ -32,17 +32,17 @@
 #emptyDownloads {
   padding: 10px 20px;
   /* The panel can be wider than this description after the blocked subview is
      shown, so center the text. */
   text-align: center;
 }
 
 .downloadsPanelFooter {
-  background-color: hsla(210,4%,10%,.07);
+  background-color: var(--panel-background-color);
   border-top: 1px solid var(--panel-separator-color);
 }
 
 .downloadsPanelFooter > toolbarseparator {
   margin: 0;
   border: 0;
   min-width: 0;
   border-left: 1px solid var(--panel-separator-color);
@@ -54,23 +54,23 @@
   background-color: transparent;
   color: inherit;
   margin: 0;
   padding: 0;
   min-height: 40px;
 }
 
 .downloadsPanelFooterButton:hover {
-  outline: 1px solid hsla(210,4%,10%,.07);
-  background-color: hsla(210,4%,10%,.07);
+  outline: 1px solid var(--panel-background-color);
+  background-color: var(--panel-background-color);
 }
 
 .downloadsPanelFooterButton:hover:active {
-  outline: 1px solid hsla(210,4%,10%,.12);
-  background-color: hsla(210,4%,10%,.12);
+  outline: 1px solid var(--panel-background-active-color);
+  background-color: var(--panel-background-active-color);
   box-shadow: 0 1px 0 hsla(210,4%,10%,.05) inset;
 }
 
 .downloadsPanelFooterButton[default] {
   background-color: #0996f8;
   color: white;
 }
 
--- a/browser/themes/windows/browser.css
+++ b/browser/themes/windows/browser.css
@@ -41,16 +41,18 @@
   --urlbar-dropmarker-hover-region: rect(0px, 22px, 14px, 11px);
   --urlbar-dropmarker-active-region: rect(0px, 33px, 14px, 22px);
   --urlbar-dropmarker-2x-url: url("chrome://browser/skin/urlbar-history-dropmarker@2x.png");
   --urlbar-dropmarker-2x-region: rect(0, 22px, 28px, 0);
   --urlbar-dropmarker-hover-2x-region: rect(0, 44px, 28px, 22px);
   --urlbar-dropmarker-active-2x-region: rect(0, 66px, 28px, 44px);
 
   --panel-separator-color: ThreeDLightShadow;
+  --panel-background-color: hsla(210,4%,10%,.07);
+  --panel-background-active-color: hsla(210,4%,10%,.12);
 
   --urlbar-separator-color: ThreeDLightShadow;
 }
 
 #nav-bar[brighttext] {
   --toolbarbutton-hover-background: rgba(255,255,255,.25);
   --toolbarbutton-hover-bordercolor: rgba(255,255,255,.5);
 
--- a/toolkit/content/widgets/notification.xml
+++ b/toolkit/content/widgets/notification.xml
@@ -495,36 +495,24 @@
                      xbl:inherits="onclick=learnmoreclick,href=learnmoreurl">&learnMore;</xul:label>
           <xul:checkbox
             anonid="alwaysremembercheckbox"
             label="Always remember my decision"
             xbl:inherits="hidden=hideremember"
             checked="false"/>
         </xul:vbox>
       </xul:hbox>
-      <xul:hbox pack="end" 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,onpopupshown=buttonpopupshown,label=buttonlabel,accesskey=buttonaccesskey"/>
-          <xul:toolbarseparator/>
-          <xul:button type="menu"
-                      class="popup-notification-button popup-notification-dropmarker">
-            <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:hbox pack="center" class="popup-notification-button-container">
+        <xul:button anonid="secondarybutton" flex="1"
+                    class="popup-notification-button popup-notification-button-secondary"
+                    xbl:inherits="oncommand=secondarybuttoncommand,onpopupshown=buttonpopupshown,label=secondarybuttonlabel,accesskey=secondarybuttonaccesskey"/>
+        <xul:button anonid="button" flex="1"
+                    class="popup-notification-button"
+                    default="true"
+                    xbl:inherits="oncommand=buttoncommand,onpopupshown=buttonpopupshown,label=buttonlabel,accesskey=buttonaccesskey"/>
       </xul:hbox>
     </content>
     <resources>
       <stylesheet src="chrome://global/skin/notification.css"/>
     </resources>
     <implementation>
       <field name="alwaysremembercheckbox" readonly="true">
         document.getAnonymousElementByAttribute(this, "anonid", "alwaysremembercheckbox");
--- a/toolkit/modules/PopupNotifications.jsm
+++ b/toolkit/modules/PopupNotifications.jsm
@@ -337,21 +337,16 @@ PopupNotifications.prototype = {
    *                     removed when they would have otherwise been dismissed
    *                     (i.e. any time the popup is closed due to user
    *                     interaction).
    *        alwaysRemember:
    *                     A boolean indicating whether to show the
    *                     "Always remember my choice" checkbox. The value of this
    *                     checkbox is passed to the callback provided by the
    *                     mainAction and secondaryActions.
-   *        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.
    *        popupIconURL:
    *                     A string. URL of the image to be displayed in the popup.
    *                     Normally specified in CSS using list-style-image and the
    *                     .popup-notification-icon[popupid=...] selector.
    *        learnMoreURL:
    *                     A string URL. Setting this property will make the
    *                     prompt display a "Learn More" link that, when clicked,
    *                     opens the URL in a new tab.
@@ -371,20 +366,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);
@@ -651,17 +642,16 @@ PopupNotifications.prototype = {
       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("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("learnmoreclick");
         popupnotification.removeAttribute("menucommand");
@@ -690,44 +680,27 @@ PopupNotifications.prototype = {
           popupnotification.removeAttribute("origin");
         }
       } else
         popupnotification.removeAttribute("origin");
 
       popupnotification.notification = n;
 
       if (n.secondaryActions) {
-        let telemetryStatId = TELEMETRY_STAT_ACTION_2;
-
-        n.secondaryActions.forEach(function (a) {
-          let item = doc.createElementNS(XUL_NS, "menuitem");
-          item.setAttribute("label", a.label);
-          item.setAttribute("accesskey", a.accessKey);
-          item.notification = n;
-          item.action = a;
+        // TODO: stop gap until we change the API to only accept a single secondary action.
+        if (n.secondaryActions.length > 1) {
+          Cu.reportError("Only the first secondary action in a popup notification will be used!");
+        }
 
-          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);
+        let a = n.secondaryActions[0];
+        popupnotification.setAttribute("secondarybuttonlabel", a.label);
+        popupnotification.setAttribute("secondarybuttonaccesskey", a.accessKey);
+        popupnotification.setAttribute("secondarybuttoncommand", "PopupNotifications._onButtonEvent(event, 'secondarybuttoncommand');");
 
         popupnotification.setAttribute("hideremember", !n.options.alwaysRemember);
-
-        if (n.options.hideNotNow) {
-          popupnotification.setAttribute("hidenotnow", "true");
-        }
-        else if (n.secondaryActions.length) {
-          let closeItemSeparator = doc.createElementNS(XUL_NS, "menuseparator");
-          popupnotification.appendChild(closeItemSeparator);
-        }
       }
 
       this.panel.appendChild(popupnotification);
 
       // The popupnotification may be hidden if we got it from the chrome
       // document rather than creating it ad hoc.
       popupnotification.hidden = false;
     }, this);
@@ -1152,73 +1125,57 @@ PopupNotifications.prototype = {
       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;
+      }
     }
 
-    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;
+    let action = notification.mainAction;
+    let telemetryStatId = TELEMETRY_STAT_ACTION_1;
+
+    if (type == "secondarybuttoncommand") {
+      action = notification.secondaryActions[0];
+      telemetryStatId = TELEMETRY_STAT_ACTION_2;
     }
 
-    notification._recordTelemetryStat(TELEMETRY_STAT_ACTION_1);
+    notification._recordTelemetryStat(telemetryStatId);
 
     try {
       let alwaysRemember = notificationEl.alwaysremembercheckbox.checked;
-      notification.mainAction.callback.call(null, alwaysRemember);
+      action.callback.call(null, alwaysRemember);
     } catch (error) {
       Cu.reportError(error);
     }
 
-    if (notification.mainAction.dismiss) {
+    if (action.dismiss) {
       this._dismiss();
       return;
     }
 
     this._remove(notification);
     this._update();
   },
 
-  _onMenuCommand: function PopupNotifications_onMenuCommand(event) {
-    let target = event.originalTarget;
-    if (!target.action || !target.notification)
-      throw "menucommand target has no associated action/notification";
-
-    let notificationEl = target.parentElement;
-    event.stopPropagation();
-
-    target.notification._recordTelemetryStat(target.action.telemetryStatId);
-
-    try {
-      let alwaysRemember = notificationEl.alwaysremembercheckbox.checked;
-      target.action.callback.call(null, alwaysRemember);
-    } catch (error) {
-      Cu.reportError(error);
-    }
-
-    if (target.action.dismiss) {
-      this._dismiss();
-      return;
-    }
-
-    this._remove(target.notification);
-    this._update();
-  },
-
   _notify: function PopupNotifications_notify(topic) {
     Services.obs.notifyObservers(null, "PopupNotifications-" + topic, "");
   },
 };
--- a/toolkit/themes/shared/popupnotification.inc.css
+++ b/toolkit/themes/shared/popupnotification.inc.css
@@ -40,79 +40,55 @@
   margin-bottom: .3em !important;
 }
 
 .popup-notification-learnmore-link {
   margin-top: .5em !important;
 }
 
 .popup-notification-button-container {
-  background-color: hsla(210,4%,10%,.07);
+  background-color: var(--panel-background-color);
   border-top: 1px solid var(--panel-separator-color);
 }
 
-.popup-notification-button-wrapper {
-  background-color: #0996f8;
-}
-
-.popup-notification-button-wrapper > toolbarseparator {
-  border: 0;
-  border-left: 1px solid var(--panel-separator-color);
-  margin: 7px 0 7px;
-  /* This is to override the linux !important */
-  -moz-appearance: none !important;
-  min-width: 0;
-}
-
-.popup-notification-button-wrapper:hover > toolbarseparator {
-  margin: 0;
-}
-
 .popup-notification-button {
   -moz-appearance: none;
-  background-color: transparent;
+  background-color: #0996f8;
   color: white;
   margin: 0;
   padding: 0 18px;
   min-height: 40px;
   min-width: 0;
   border: none;
 }
 
 .popup-notification-button:hover {
-  outline: 1px solid hsla(210,4%,10%,.07);
+  outline: 1px solid var(--panel-background-color);
   background-color: #0675d3;
 }
 
 .popup-notification-button:hover:active {
-  outline: 1px solid hsla(210,4%,10%,.12);
+  outline: 1px solid var(--panel-background-active-color);
   background-color: #0568ba;
   box-shadow: 0 1px 0 hsla(210,4%,10%,.05) inset;
 }
 
-.popup-notification-dropmarker {
-  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;
-}
-
-.popup-notification-dropmarker > .button-box > .button-menu-dropmarker > .dropmarker-icon {
-  width: 16px;
-  height: 16px;
-  list-style-image: url(chrome://global/skin/icons/menubutton-dropmarker-white.svg);
-}
-
 .popup-notification-button[disabled] {
   opacity: 0.5;
 }
+
+.popup-notification-button-secondary {
+  background-color: transparent;
+  color: inherit;
+}
+
+.popup-notification-button-secondary:hover {
+  background-color: var(--panel-background-color);
+}
+
+.popup-notification-button-secondary:hover:active {
+  background-color: var(--panel-background-active-color);
+}