From 64baf2d7298a26550a3ef6fa549893e2e4f5576c Mon Sep 17 00:00:00 2001 From: Panos Astithas Date: Sun, 20 Nov 2016 18:40:22 +0100 Subject: [PATCH] Bug 1282768 - Part 2 - Move the secondary actions of doorhanger notifications to their own menu button. r=paolo MozReview-Commit-ID: 2cnufF7wZJG --HG-- extra : rebase_source : 6bb9c9f8bd52bd8e6f54cbc2b0ef2760fee25c3a --- .../browser_popupNotification.js | 44 +++++++ .../browser_popupNotification_2.js | 16 --- .../browser_popupNotification_4.js | 17 --- .../browser_popupNotification_5.js | 6 +- .../content/test/popupNotifications/head.js | 48 ++++---- browser/base/content/test/webrtc/head.js | 10 +- browser/components/nsBrowserGlue.js | 1 - .../content/PresentationDevicePrompt.jsm | 3 +- browser/modules/test/browser_PermissionUI.js | 13 +-- .../test/browser_permissionsPromptDeny.js | 2 +- dom/indexedDB/test/head.js | 21 +--- .../browser/browser_permission_dismiss.js | 10 +- .../passwordmgr/test/browser/head.js | 5 +- toolkit/content/widgets/notification.xml | 43 +++---- .../en-US/chrome/global/notification.dtd | 2 - toolkit/modules/PopupNotifications.jsm | 108 ++++++++++-------- toolkit/themes/osx/global/notification.css | 5 - .../themes/shared/popupnotification.inc.css | 64 +++++++---- 18 files changed, 225 insertions(+), 193 deletions(-) diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification.js b/browser/base/content/test/popupNotifications/browser_popupNotification.js index bd1813a8d3f1..4388c7c0bf92 100644 --- a/browser/base/content/test/popupNotifications/browser_popupNotification.js +++ b/browser/base/content/test/popupNotifications/browser_popupNotification.js @@ -47,6 +47,50 @@ var tests = [ 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); diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification_2.js b/browser/base/content/test/popupNotifications/browser_popupNotification_2.js index 027cfe9e1117..2bb42334a21e 100644 --- a/browser/base/content/test/popupNotifications/browser_popupNotification_2.js +++ b/browser/base/content/test/popupNotifications/browser_popupNotification_2.js @@ -187,22 +187,6 @@ var tests = [ 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() { diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification_4.js b/browser/base/content/test/popupNotifications/browser_popupNotification_4.js index e22cfa326f3f..def3d9fe6570 100644 --- a/browser/base/content/test/popupNotifications/browser_popupNotification_4.js +++ b/browser/base/content/test/popupNotifications/browser_popupNotification_4.js @@ -160,23 +160,6 @@ var tests = [ 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() { diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification_5.js b/browser/base/content/test/popupNotifications/browser_popupNotification_5.js index 61550ad07bb1..9e796e93c7cf 100644 --- a/browser/base/content/test/popupNotifications/browser_popupNotification_5.js +++ b/browser/base/content/test/popupNotifications/browser_popupNotification_5.js @@ -101,7 +101,7 @@ var tests = [ 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"); @@ -131,9 +131,9 @@ var tests = [ 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"); diff --git a/browser/base/content/test/popupNotifications/head.js b/browser/base/content/test/popupNotifications/head.js index 6baab7c41a05..4fbaba2a2c5d 100644 --- a/browser/base/content/test/popupNotifications/head.js +++ b/browser/base/content/test/popupNotifications/head.js @@ -213,25 +213,24 @@ function checkPopup(popup, notifyObj) { is(notification.getAttribute("buttonaccesskey"), notifyObj.mainAction.accessKey, "main action accesskey matches"); } - let actualSecondaryActions = + 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"); + } + // Additional secondary actions appear as menu items. + let actualExtraSecondaryActions = 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"); + 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"); }); } @@ -272,13 +271,20 @@ function triggerSecondaryCommand(popup, index) { 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 diff --git a/browser/base/content/test/webrtc/head.js b/browser/base/content/test/webrtc/head.js index 472cbbcecb9c..c09d4cd3c925 100644 --- a/browser/base/content/test/webrtc/head.js +++ b/browser/base/content/test/webrtc/head.js @@ -305,13 +305,19 @@ 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 diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index e07cdacd613a..433510e30360 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -2856,7 +2856,6 @@ var E10SAccessibilityCheck = { learnMoreURL: Services.urlFormatter.formatURLPref("app.support.e10sAccessibilityUrl"), persistent: true, persistWhileVisible: true, - hideNotNow: true, }; notification = diff --git a/browser/extensions/presentation/content/PresentationDevicePrompt.jsm b/browser/extensions/presentation/content/PresentationDevicePrompt.jsm index dcde6826c702..883ffff34fa3 100644 --- a/browser/extensions/presentation/content/PresentationDevicePrompt.jsm +++ b/browser/extensions/presentation/content/PresentationDevicePrompt.jsm @@ -76,7 +76,6 @@ PresentationPermissionPrompt.prototype = { }, get popupOptions() { return { - hideNotNow: true, removeOnDismissal: true, popupIconURL: kNotificationPopupIcon, // Icon shown on prompt content eventCallback: (aTopic, aNewBrowser) => { @@ -162,7 +161,7 @@ PresentationPermissionPrompt.prototype = { this._isResponded = true; this.request.cancel(Cr.NS_ERROR_NOT_AVAILABLE); }, - dismiss: true, // For hideNotNow. + dismiss: true, }]; }, // PRIVATE APIs diff --git a/browser/modules/test/browser_PermissionUI.js b/browser/modules/test/browser_PermissionUI.js index 006bc5e66401..767c97024549 100644 --- a/browser/modules/test/browser_PermissionUI.js +++ b/browser/modules/test/browser_PermissionUI.js @@ -63,21 +63,18 @@ function clickMainAction() { } /** - * 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; } @@ -274,7 +271,7 @@ add_task(function* test_with_permission_key() { // 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, @@ -429,7 +426,7 @@ add_task(function* test_no_request() { // 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"); diff --git a/dom/indexedDB/test/browser_permissionsPromptDeny.js b/dom/indexedDB/test/browser_permissionsPromptDeny.js index e7132e004a16..ce6760915ac5 100644 --- a/dom/indexedDB/test/browser_permissionsPromptDeny.js +++ b/dom/indexedDB/test/browser_permissionsPromptDeny.js @@ -37,7 +37,7 @@ add_task(function test1() { }); registerPopupEventHandler("popupshown", function () { ok(true, "prompt shown"); - triggerSecondaryCommand(this, 0); + triggerSecondaryCommand(this); }); registerPopupEventHandler("popuphidden", function () { ok(true, "prompt hidden"); diff --git a/dom/indexedDB/test/head.js b/dom/indexedDB/test/head.js index f561d29083e7..19d908fc981d 100644 --- a/dom/indexedDB/test/head.js +++ b/dom/indexedDB/test/head.js @@ -50,28 +50,13 @@ function triggerMainCommand(popup) 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) diff --git a/dom/notification/test/browser/browser_permission_dismiss.js b/dom/notification/test/browser/browser_permission_dismiss.js index de655870bdcf..ef1e15e74441 100644 --- a/dom/notification/test/browser/browser_permission_dismiss.js +++ b/dom/notification/test/browser/browser_permission_dismiss.js @@ -14,20 +14,18 @@ const TEST_URL = "http://mochi.test:8888/browser/dom/notification/test/browser/n * @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(); } } diff --git a/toolkit/components/passwordmgr/test/browser/head.js b/toolkit/components/passwordmgr/test/browser/head.js index 3ebb13713180..583fded3f94d 100644 --- a/toolkit/components/passwordmgr/test/browser/head.js +++ b/toolkit/components/passwordmgr/test/browser/head.js @@ -118,9 +118,12 @@ function clickDoorhangerButton(aPopup, aButtonIndex) { 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(); } } diff --git a/toolkit/content/widgets/notification.xml b/toolkit/content/widgets/notification.xml index 3f79864dd8a3..f1fdf0d964de 100644 --- a/toolkit/content/widgets/notification.xml +++ b/toolkit/content/widgets/notification.xml @@ -502,7 +502,7 @@ @@ -513,27 +513,25 @@ - + - - - - - - - - - - + + + + + + + + @@ -549,6 +547,9 @@ document.getAnonymousElementByAttribute(this, "anonid", "button"); + + document.getAnonymousElementByAttribute(this, "anonid", "secondarybutton"); + document.getAnonymousElementByAttribute(this, "anonid", "menupopup"); diff --git a/toolkit/locales/en-US/chrome/global/notification.dtd b/toolkit/locales/en-US/chrome/global/notification.dtd index 75aea0b98664..34ee873be888 100644 --- a/toolkit/locales/en-US/chrome/global/notification.dtd +++ b/toolkit/locales/en-US/chrome/global/notification.dtd @@ -4,8 +4,6 @@ - - diff --git a/toolkit/modules/PopupNotifications.jsm b/toolkit/modules/PopupNotifications.jsm index 62dcb7d053af..908402c6db69 100644 --- a/toolkit/modules/PopupNotifications.jsm +++ b/toolkit/modules/PopupNotifications.jsm @@ -31,7 +31,6 @@ 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; @@ -352,6 +351,8 @@ PopupNotifications.prototype = { * 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: @@ -372,11 +373,6 @@ PopupNotifications.prototype = { * (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 @@ -411,10 +407,6 @@ PopupNotifications.prototype = { 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); @@ -597,8 +589,7 @@ PopupNotifications.prototype = { // 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; } @@ -687,18 +678,16 @@ PopupNotifications.prototype = { 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) { @@ -729,17 +718,26 @@ PopupNotifications.prototype = { } 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); @@ -749,15 +747,14 @@ PopupNotifications.prototype = { 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; @@ -788,11 +785,14 @@ PopupNotifications.prototype = { }, _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"); } @@ -1277,7 +1277,7 @@ PopupNotifications.prototype = { let notification = notificationEl.notification; - if (type == "buttonpopupshown") { + if (type == "dropmarkerpopupshown") { notification._recordTelemetryStat(TELEMETRY_STAT_OPEN_SUBMENU); return; } @@ -1287,34 +1287,46 @@ PopupNotifications.prototype = { 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; } diff --git a/toolkit/themes/osx/global/notification.css b/toolkit/themes/osx/global/notification.css index 5172ec57e522..5bdb0cd88a9c 100644 --- a/toolkit/themes/osx/global/notification.css +++ b/toolkit/themes/osx/global/notification.css @@ -110,11 +110,6 @@ notification[type="info"]:not([value="translation"]) .close-icon:not(:hover) { 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; } diff --git a/toolkit/themes/shared/popupnotification.inc.css b/toolkit/themes/shared/popupnotification.inc.css index 982d40d98c82..ff3590b37b2b 100644 --- a/toolkit/themes/shared/popupnotification.inc.css +++ b/toolkit/themes/shared/popupnotification.inc.css @@ -43,13 +43,11 @@ .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); @@ -57,41 +55,67 @@ 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; - min-height: 40px; + padding: 0; min-width: 0; + min-height: 40px; border: none; } -.popup-notification-button:hover { +.popup-notification-button:hover:not([disabled]) { outline: 1px solid var(--arrowpanel-dimmed); - background-color: #0675d3; + background-color: var(--arrowpanel-dimmed); } -.popup-notification-button:hover:active { +.popup-notification-button:hover:active:not([disabled]) { outline: 1px solid var(--arrowpanel-dimmed-further); - background-color: #0568ba; + background-color: var(--arrowpanel-dimmed-further); box-shadow: 0 1px 0 hsla(210,4%,10%,.05) inset; } -.popup-notification-dropmarker { - padding: 0 15px; +.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[default]:hover:active:not([disabled]) { + background-color: #0568ba; } -/* prevent double border on windows when focused */ .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; +} + .popup-notification-dropmarker > .button-box > hbox { display: none; } @@ -100,6 +124,8 @@ /* 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 { @@ -107,9 +133,5 @@ height: 16px; list-style-image: url(chrome://global/skin/icons/menubutton-dropmarker.svg); filter: url(chrome://global/skin/filters.svg#fill); - fill: white; -} - -.popup-notification-button[disabled] { - opacity: 0.5; + fill: currentColor; }