Bug 1272304 - Add disabled state to screen sharing permission r?florian draft
authorJonathan Kingston <jkt@mozilla.com>
Sat, 12 Aug 2017 18:54:24 +0100
changeset 667130 7d2a3eced3df45f0cadb97290a561fdc700e692e
parent 666415 66d994e5bceba2e1e16397a286f3a3ff515f28e3
child 732298 358f5c2f226df2883a63ac577f974f6f5dd36706
push id80615
push userbmo:jkt@mozilla.com
push dateTue, 19 Sep 2017 18:26:21 +0000
reviewersflorian
bugs1272304
milestone57.0a1
Bug 1272304 - Add disabled state to screen sharing permission r?florian MozReview-Commit-ID: LsZmQD5fWzW
browser/base/content/test/popupNotifications/browser.ini
browser/base/content/test/popupNotifications/browser_popupNotification_selection_required.js
browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js
browser/base/content/test/webrtc/browser_devices_get_user_media_unprompted_access.js
browser/locales/en-US/chrome/browser/browser.properties
browser/modules/webrtcUI.jsm
toolkit/modules/PopupNotifications.jsm
--- a/browser/base/content/test/popupNotifications/browser.ini
+++ b/browser/base/content/test/popupNotifications/browser.ini
@@ -11,14 +11,16 @@ skip-if = (os == "linux" && (debug || as
 [browser_popupNotification_3.js]
 skip-if = (os == "linux" && (debug || asan))
 [browser_popupNotification_4.js]
 skip-if = (os == "linux" && (debug || asan))
 [browser_popupNotification_5.js]
 skip-if = true # bug 1332646
 [browser_popupNotification_checkbox.js]
 skip-if = (os == "linux" && (debug || asan))
+[browser_popupNotification_selection_required.js]
+skip-if = (os == "linux" && (debug || asan))
 [browser_popupNotification_keyboard.js]
 skip-if = (os == "linux" && (debug || asan))
 [browser_popupNotification_no_anchors.js]
 skip-if = (os == "linux" && (debug || asan))
 [browser_reshow_in_background.js]
 skip-if = (os == "linux" && (debug || asan))
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/popupNotifications/browser_popupNotification_selection_required.js
@@ -0,0 +1,48 @@
+/* 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/. */
+
+function test() {
+  waitForExplicitFinish();
+
+  ok(PopupNotifications, "PopupNotifications object exists");
+  ok(PopupNotifications.panel, "PopupNotifications panel exists");
+
+  setup();
+}
+
+function promiseElementVisible(element) {
+  // HTMLElement.offsetParent is null when the element is not visisble
+  // (or if the element has |position: fixed|). See:
+  // https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent
+  return BrowserTestUtils.waitForCondition(() => element.offsetParent !== null,
+                                          "Waiting for element to be visible");
+}
+
+var gNotification;
+
+var tests = [
+  // Test that passing selection required prevents the button from clicking
+  { id: "require_selection_check",
+    run() {
+      this.notifyObj = new BasicNotification(this.id);
+      this.notifyObj.options.checkbox = {
+        label: "This is a checkbox",
+      };
+      gNotification = showNotification(this.notifyObj);
+    },
+    async onShown(popup) {
+      checkPopup(popup, this.notifyObj);
+      let notification = popup.childNodes[0];
+      notification.setAttribute("invalidselection", true);
+      await promiseElementVisible(notification.checkbox);
+      EventUtils.synthesizeMouseAtCenter(notification.checkbox, {});
+      ok(notification.button.disabled, "should be disabled when invalidselection");
+      notification.removeAttribute("invalidselection");
+      EventUtils.synthesizeMouseAtCenter(notification.checkbox, {});
+      ok(!notification.button.disabled, "should not be disabled when invalidselection is not present");
+      triggerMainCommand(popup);
+    },
+    onHidden() { }
+  },
+];
--- a/browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js
+++ b/browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js
@@ -25,30 +25,33 @@ var gTests = [
     let promise = promisePopupNotificationShown("webRTC-shareDevices");
     await promiseRequestDevice(false, true, null, "screen");
     await promise;
     await expectObserverCalled("getUserMedia:request");
 
     is(PopupNotifications.getNotification("webRTC-shareDevices").anchorID,
        "webRTC-shareScreen-notification-icon", "anchored to device icon");
     checkDeviceSelectors(false, false, true);
-    let iconclass =
-      PopupNotifications.panel.firstChild.getAttribute("iconclass");
+    let notification = PopupNotifications.panel.firstChild;
+    let iconclass = notification.getAttribute("iconclass");
     ok(iconclass.includes("screen-icon"), "panel using screen icon");
 
     let menulist =
       document.getElementById("webRTC-selectWindow-menulist");
     let count = menulist.itemCount;
     ok(count >= 3,
-       "There should be the 'No Screen' item, a separator and at least one screen");
+       "There should be the 'Select Screen' item, a separator and at least one screen");
 
     let noScreenItem = menulist.getItemAtIndex(0);
-    ok(noScreenItem.hasAttribute("selected"), "the 'No Screen' item is selected");
+    ok(noScreenItem.hasAttribute("selected"), "the 'Select Screen' item is selected");
+    is(menulist.selectedItem, noScreenItem, "'Select Screen' is the selected item");
     is(menulist.value, -1, "no screen is selected by default");
-    is(menulist.selectedItem, noScreenItem, "'No Screen' is the selected item");
+    ok(noScreenItem.disabled, "'Select Screen' item is disabled");
+    ok(notification.button.disabled, "Allow button is disabled");
+    ok(notification.hasAttribute("invalidselection"), "Notification is marked as invalid");
 
     let separator = menulist.getItemAtIndex(1);
     is(separator.localName, "menuseparator", "the second item is a separator");
 
     ok(document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should be hidden while there's no selection");
     ok(document.getElementById("webRTC-preview").hidden,
        "the preview area is hidden");
@@ -64,18 +67,19 @@ var gTests = [
     menulist.getItemAtIndex(2).doCommand();
     ok(!document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should now be visible");
     await promiseWaitForCondition(() => !document.getElementById("webRTC-preview").hidden, 100);
     ok(!document.getElementById("webRTC-preview").hidden,
        "the preview area is visible");
     ok(!document.getElementById("webRTC-previewWarning").hidden,
        "the scary warning is visible");
+    ok(!notification.button.disabled, "Allow button is enabled");
 
-    // Select the 'No Screen' item again, the preview should be hidden.
+    // Select the 'Select Screen' item again, the preview should be hidden.
     menulist.getItemAtIndex(0).doCommand();
     ok(document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should now be hidden");
     ok(document.getElementById("webRTC-preview").hidden,
        "the preview area is hidden");
 
     // Select the first screen again so that we can have a stream.
     menulist.getItemAtIndex(2).doCommand();
@@ -98,17 +102,17 @@ var gTests = [
     await promise;
     await expectObserverCalled("getUserMedia:request");
 
     is(PopupNotifications.getNotification("webRTC-shareDevices").anchorID,
        "webRTC-shareScreen-notification-icon", "anchored to device icon");
     checkDeviceSelectors(false, false, true);
 
     await promiseMessage(permissionError, () => {
-      PopupNotifications.panel.firstChild.button.click();
+      activateSecondaryAction(kActionDeny);
     });
 
     await expectObserverCalled("getUserMedia:response:deny");
     SitePermissions.remove(null, "screen", gBrowser.selectedBrowser);
     await closeStream();
   }
 },
 
@@ -118,30 +122,33 @@ var gTests = [
     let promise = promisePopupNotificationShown("webRTC-shareDevices");
     await promiseRequestDevice(false, true, null, "window");
     await promise;
     await expectObserverCalled("getUserMedia:request");
 
     is(PopupNotifications.getNotification("webRTC-shareDevices").anchorID,
        "webRTC-shareScreen-notification-icon", "anchored to device icon");
     checkDeviceSelectors(false, false, true);
-    let iconclass =
-      PopupNotifications.panel.firstChild.getAttribute("iconclass");
+    let notification = PopupNotifications.panel.firstChild;
+    let iconclass = notification.getAttribute("iconclass");
     ok(iconclass.includes("screen-icon"), "panel using screen icon");
 
     let menulist =
       document.getElementById("webRTC-selectWindow-menulist");
     let count = menulist.itemCount;
     ok(count >= 3,
-       "There should be the 'No Window' item, a separator and at least one window");
+       "There should be the 'Select Window' item, a separator and at least one window");
 
     let noWindowItem = menulist.getItemAtIndex(0);
-    ok(noWindowItem.hasAttribute("selected"), "the 'No Window' item is selected");
+    ok(noWindowItem.hasAttribute("selected"), "the 'Select Window' item is selected");
+    is(menulist.selectedItem, noWindowItem, "'Select Window' is the selected item");
     is(menulist.value, -1, "no window is selected by default");
-    is(menulist.selectedItem, noWindowItem, "'No Window' is the selected item");
+    ok(noWindowItem.disabled, "'Select Window' item is disabled");
+    ok(notification.button.disabled, "Allow button is disabled");
+    ok(notification.hasAttribute("invalidselection"), "Notification is marked as invalid");
 
     let separator = menulist.getItemAtIndex(1);
     is(separator.localName, "menuseparator", "the second item is a separator");
 
     ok(document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should be hidden");
     ok(document.getElementById("webRTC-preview").hidden,
        "the preview area is hidden");
@@ -163,17 +170,17 @@ var gTests = [
     ok(document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should still be hidden");
     await promiseWaitForCondition(() => !document.getElementById("webRTC-preview").hidden);
     ok(!document.getElementById("webRTC-preview").hidden,
        "the preview area is visible");
     ok(!document.getElementById("webRTC-previewWarning").hidden,
        "the scary warning is visible");
 
-    // Select the 'No Window' item again, the preview should be hidden.
+    // Select the 'Select Window' item again, the preview should be hidden.
     menulist.getItemAtIndex(0).doCommand();
     ok(document.getElementById("webRTC-preview").hidden,
        "the preview area is hidden");
 
     // If we have a non-scary window, select it and verify the warning isn't displayed.
     // A non-scary window may not always exist on test slaves.
     if (typeof nonScaryIndex == "number") {
       menulist.getItemAtIndex(nonScaryIndex).doCommand();
@@ -224,30 +231,33 @@ var gTests = [
       return;
     }
 
     await expectObserverCalled("getUserMedia:request");
 
     is(PopupNotifications.getNotification("webRTC-shareDevices").anchorID,
        "webRTC-shareScreen-notification-icon", "anchored to device icon");
     checkDeviceSelectors(false, false, true);
-    let iconclass =
-      PopupNotifications.panel.firstChild.getAttribute("iconclass");
+    let notification = PopupNotifications.panel.firstChild;
+    let iconclass = notification.getAttribute("iconclass");
     ok(iconclass.includes("screen-icon"), "panel using screen icon");
 
     let menulist =
       document.getElementById("webRTC-selectWindow-menulist");
     let count = menulist.itemCount;
     ok(count >= 3,
-       "There should be the 'No Application' item, a separator and at least one application");
+       "There should be the 'Select Application' item, a separator and at least one application");
 
     let noApplicationItem = menulist.getItemAtIndex(0);
-    ok(noApplicationItem.hasAttribute("selected"), "the 'No Application' item is selected");
+    ok(noApplicationItem.hasAttribute("selected"), "the 'Select Application' item is selected");
+    is(menulist.selectedItem, noApplicationItem, "'Select Application' is the selected item");
     is(menulist.value, -1, "no app is selected by default");
-    is(menulist.selectedItem, noApplicationItem, "'No Application' is the selected item");
+    ok(noApplicationItem.disabled, "'Select Application' item is disabled");
+    ok(notification.button.disabled, "Allow button is disabled");
+    ok(notification.hasAttribute("invalidselection"), "Notification is marked as invalid");
 
     let separator = menulist.getItemAtIndex(1);
     is(separator.localName, "menuseparator", "the second item is a separator");
 
     ok(document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should be hidden");
     ok(document.getElementById("webRTC-preview").hidden,
        "the preview area is hidden");
@@ -307,17 +317,17 @@ var gTests = [
     let iconclass =
       PopupNotifications.panel.firstChild.getAttribute("iconclass");
     ok(iconclass.includes("screen-icon"), "panel using screen icon");
 
     let menulist =
       document.getElementById("webRTC-selectWindow-menulist");
     let count = menulist.itemCount;
     ok(count >= 3,
-       "There should be the 'No Screen' item, a separator and at least one screen");
+       "There should be the 'Select Screen' item, a separator and at least one screen");
 
     // Select a screen, a preview with a scary warning should appear.
     menulist.getItemAtIndex(2).doCommand();
     ok(!document.getElementById("webRTC-all-windows-shared").hidden,
        "the 'all windows will be shared' warning should now be visible");
     await promiseWaitForCondition(() => !document.getElementById("webRTC-preview").hidden);
     ok(!document.getElementById("webRTC-preview").hidden,
        "the preview area is visible");
@@ -335,39 +345,16 @@ var gTests = [
                      "expected screen and microphone to be shared");
 
     await indicator;
     await checkSharingUI({audio: true, screen: "Screen"});
     await closeStream();
   }
 },
 
-
-{
-  desc: "getUserMedia screen: clicking through without selecting a screen denies",
-  run: async function checkClickThroughDenies() {
-    let promise = promisePopupNotificationShown("webRTC-shareDevices");
-    await promiseRequestDevice(false, true, null, "screen");
-    await promise;
-    await expectObserverCalled("getUserMedia:request");
-    checkDeviceSelectors(false, false, true);
-
-    await promiseMessage(permissionError, () => {
-      PopupNotifications.panel.firstChild.button.click();
-    });
-
-    await expectObserverCalled("getUserMedia:response:deny");
-    await expectObserverCalled("recording-window-ended");
-    await checkNotSharing();
-    SitePermissions.remove(null, "screen", gBrowser.selectedBrowser);
-    SitePermissions.remove(null, "camera", gBrowser.selectedBrowser);
-  }
-},
-
-
 {
   desc: "getUserMedia screen, user clicks \"Don't Allow\"",
   run: async function checkDontShare() {
     let promise = promisePopupNotificationShown("webRTC-shareDevices");
     await promiseRequestDevice(false, true, null, "screen");
     await promise;
     await expectObserverCalled("getUserMedia:request");
     checkDeviceSelectors(false, false, true);
--- a/browser/base/content/test/webrtc/browser_devices_get_user_media_unprompted_access.js
+++ b/browser/base/content/test/webrtc/browser_devices_get_user_media_unprompted_access.js
@@ -48,17 +48,17 @@ var gTests = [
     await promise;
     await expectObserverCalled("getUserMedia:request");
 
     is(PopupNotifications.getNotification("webRTC-shareDevices").anchorID,
        "webRTC-shareScreen-notification-icon", "anchored to device icon");
     checkDeviceSelectors(false, false, true);
 
     await promiseMessage(permissionError, () => {
-      PopupNotifications.panel.firstChild.button.click();
+      activateSecondaryAction(kActionDeny);
     });
 
     await expectObserverCalled("getUserMedia:response:deny");
     SitePermissions.remove(null, "screen", gBrowser.selectedBrowser);
     SitePermissions.remove(null, "camera", gBrowser.selectedBrowser);
     SitePermissions.remove(null, "microphone", gBrowser.selectedBrowser);
 
     // After closing all streams, gUM(audio+camera) causes a prompt.
@@ -141,17 +141,17 @@ var gTests = [
     await promise;
     await expectObserverCalled("getUserMedia:request");
 
     is(PopupNotifications.getNotification("webRTC-shareDevices").anchorID,
        "webRTC-shareScreen-notification-icon", "anchored to device icon");
     checkDeviceSelectors(false, false, true);
 
     await promiseMessage(permissionError, () => {
-      PopupNotifications.panel.firstChild.button.click();
+      activateSecondaryAction(kActionDeny);
     });
 
     await expectObserverCalled("getUserMedia:response:deny");
     SitePermissions.remove(null, "screen", gBrowser.selectedBrowser);
     SitePermissions.remove(null, "camera", gBrowser.selectedBrowser);
     SitePermissions.remove(null, "microphone", gBrowser.selectedBrowser);
 
     // gUM(camera) returns a stream without prompting.
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@@ -638,19 +638,19 @@ getUserMedia.shareFirefoxWarning.message
 # LOCALIZATION NOTE(getUserMedia.shareScreen.learnMoreLabel): NB: inserted via innerHTML, so please don't use <, > or & in this string.
 getUserMedia.shareScreen.learnMoreLabel = Learn More
 getUserMedia.selectWindow.label=Window to share:
 getUserMedia.selectWindow.accesskey=W
 getUserMedia.selectScreen.label=Screen to share:
 getUserMedia.selectScreen.accesskey=S
 getUserMedia.selectApplication.label=Application to share:
 getUserMedia.selectApplication.accesskey=A
-getUserMedia.noApplication.label = No Application
-getUserMedia.noScreen.label = No Screen
-getUserMedia.noWindow.label = No Window
+getUserMedia.pickApplication.label = Select Application
+getUserMedia.pickScreen.label = Select Screen
+getUserMedia.pickWindow.label = Select Window
 getUserMedia.shareEntireScreen.label = Entire screen
 # LOCALIZATION NOTE (getUserMedia.shareMonitor.label):
 # %S is screen number (digits 1, 2, etc)
 # Example: Screen 1, Screen 2,..
 getUserMedia.shareMonitor.label = Screen %S
 # LOCALIZATION NOTE (getUserMedia.shareApplicationWindowCount.label):
 # Semicolon-separated list of plural forms.
 # See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
--- a/browser/modules/webrtcUI.jsm
+++ b/browser/modules/webrtcUI.jsm
@@ -536,34 +536,45 @@ function prompt(aBrowser, aRequest) {
         // when the list is rebuilt with a different content, so we remove
         // the value attribute explicitly.
         menupopup.parentNode.removeAttribute("value");
 
         for (let device of devices)
           addDeviceToList(menupopup, device.name, device.deviceIndex);
       }
 
+      function checkDisabledWindowMenuItem() {
+        let list = doc.getElementById("webRTC-selectWindow-menulist");
+        let item = list.selectedItem;
+        let notificationElement = doc.getElementById("webRTC-shareDevices-notification");
+        if (!item || item.hasAttribute("disabled")) {
+          notificationElement.setAttribute("invalidselection", "true");
+        } else {
+          notificationElement.removeAttribute("invalidselection");
+        }
+      }
+
       function listScreenShareDevices(menupopup, devices) {
         while (menupopup.lastChild)
           menupopup.removeChild(menupopup.lastChild);
 
         let type = devices[0].mediaSource;
         let typeName = type.charAt(0).toUpperCase() + type.substr(1);
 
         let label = doc.getElementById("webRTC-selectWindow-label");
         let gumStringId = "getUserMedia.select" + typeName;
         label.setAttribute("value",
                            stringBundle.getString(gumStringId + ".label"));
         label.setAttribute("accesskey",
                            stringBundle.getString(gumStringId + ".accesskey"));
 
-        // "No <type>" is the default because we can't pick a
+        // "Select <type>" is the default because we can't pick a
         // 'default' window to share.
         addDeviceToList(menupopup,
-                        stringBundle.getString("getUserMedia.no" + typeName + ".label"),
+                        stringBundle.getString("getUserMedia.pick" + typeName + ".label"),
                         "-1");
         menupopup.appendChild(doc.createElement("menuseparator"));
 
         // Build the list of 'devices'.
         let monitorIndex = 1;
         for (let i = 0; i < devices.length; ++i) {
           let device = devices[i];
 
@@ -594,17 +605,19 @@ function prompt(aBrowser, aRequest) {
           item.deviceId = device.id;
           if (device.scary)
             item.scary = true;
         }
 
         // Always re-select the "No <type>" item.
         doc.getElementById("webRTC-selectWindow-menulist").removeAttribute("value");
         doc.getElementById("webRTC-all-windows-shared").hidden = true;
+
         menupopup._commandEventListener = event => {
+          checkDisabledWindowMenuItem();
           let video = doc.getElementById("webRTC-previewVideo");
           if (video.stream) {
             video.stream.getTracks().forEach(t => t.stop());
             video.stream = null;
           }
 
           let deviceId = event.target.deviceId;
           if (deviceId == undefined) {
@@ -670,31 +683,38 @@ function prompt(aBrowser, aRequest) {
 
       function addDeviceToList(menupopup, deviceName, deviceIndex, type) {
         let menuitem = doc.createElement("menuitem");
         menuitem.setAttribute("value", deviceIndex);
         menuitem.setAttribute("label", deviceName);
         menuitem.setAttribute("tooltiptext", deviceName);
         if (type)
           menuitem.setAttribute("devicetype", type);
+
+        if (deviceIndex == "-1")
+          menuitem.setAttribute("disabled", true);
+
         menupopup.appendChild(menuitem);
         return menuitem;
       }
 
       doc.getElementById("webRTC-selectCamera").hidden = !videoDevices.length || sharingScreen;
       doc.getElementById("webRTC-selectWindowOrScreen").hidden = !sharingScreen || !videoDevices.length;
       doc.getElementById("webRTC-selectMicrophone").hidden = !audioDevices.length || sharingAudio;
 
       let camMenupopup = doc.getElementById("webRTC-selectCamera-menupopup");
       let windowMenupopup = doc.getElementById("webRTC-selectWindow-menupopup");
       let micMenupopup = doc.getElementById("webRTC-selectMicrophone-menupopup");
-      if (sharingScreen)
+      if (sharingScreen) {
         listScreenShareDevices(windowMenupopup, videoDevices);
-      else
+        checkDisabledWindowMenuItem();
+      } else {
         listDevices(camMenupopup, videoDevices);
+        doc.getElementById("webRTC-shareDevices-notification").removeAttribute("invalidselection");
+      }
 
       if (!sharingAudio)
         listDevices(micMenupopup, audioDevices);
 
       this.mainAction.callback = function(aState) {
         let remember = aState && aState.checkboxChecked;
         let allowedDevices = [];
         let perms = Services.perms;
--- a/toolkit/modules/PopupNotifications.jsm
+++ b/toolkit/modules/PopupNotifications.jsm
@@ -766,16 +766,17 @@ PopupNotifications.prototype = {
       if (popupnotification)
         gNotificationParents.set(popupnotification, popupnotification.parentNode);
       else
         popupnotification = doc.createElementNS(XUL_NS, "popupnotification");
 
       popupnotification.setAttribute("label", n.message);
       popupnotification.setAttribute("id", popupnotificationID);
       popupnotification.setAttribute("popupid", n.id);
+      popupnotification.setAttribute("oncommand", "PopupNotifications._onCommand(event);");
       if (Services.prefs.getBoolPref("privacy.permissionPrompts.showCloseButton")) {
         popupnotification.setAttribute("closebuttoncommand", "PopupNotifications._onButtonEvent(event, 'secondarybuttoncommand');");
       } else {
         popupnotification.setAttribute("closebuttoncommand", `PopupNotifications._dismiss(event, ${TELEMETRY_STAT_DISMISSAL_CLOSE_BUTTON});`);
       }
       if (n.mainAction) {
         popupnotification.setAttribute("buttonlabel", n.mainAction.label);
         popupnotification.setAttribute("buttonaccesskey", n.mainAction.accessKey);
@@ -885,44 +886,30 @@ 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 = {}) {
-    if (state.disableMainAction) {
+    if (state.disableMainAction ||
+        notification.hasAttribute("invalidselection")) {
       notification.setAttribute("mainactiondisabled", "true");
     } else {
       notification.removeAttribute("mainactiondisabled");
     }
     if (state.warningLabel) {
       notification.setAttribute("warninglabel", state.warningLabel);
       notification.removeAttribute("warninghidden");
     } else {
       notification.setAttribute("warninghidden", "true");
     }
   },
 
-  _onCheckboxCommand(event) {
-    let notificationEl = getNotificationFromElement(event.originalTarget);
-    let checked = notificationEl.checkbox.checked;
-    let notification = notificationEl.notification;
-
-    // Save checkbox state to be able to persist it when re-opening the doorhanger.
-    notification._checkboxChecked = checked;
-
-    if (checked) {
-      this._setNotificationUIState(notificationEl, notification.options.checkbox.checkedState);
-    } else {
-      this._setNotificationUIState(notificationEl, notification.options.checkbox.uncheckedState);
-    }
-  },
-
   _showPanel: function PopupNotifications_showPanel(notificationsToShow, anchorElement) {
     this.panel.hidden = false;
 
     notificationsToShow = notificationsToShow.filter(n => {
       if (anchorElement != n.anchorElement) {
         return false;
       }
 
@@ -1411,16 +1398,49 @@ PopupNotifications.prototype = {
         this._remove(notificationObj);
       } else {
         notificationObj.dismissed = true;
         this._fireCallback(notificationObj, NOTIFICATION_EVENT_DISMISSED);
       }
     }, this);
   },
 
+  _onCheckboxCommand(event) {
+    let notificationEl = getNotificationFromElement(event.originalTarget);
+    let checked = notificationEl.checkbox.checked;
+    let notification = notificationEl.notification;
+
+    // Save checkbox state to be able to persist it when re-opening the doorhanger.
+    notification._checkboxChecked = checked;
+
+    if (checked) {
+      this._setNotificationUIState(notificationEl, notification.options.checkbox.checkedState);
+    } else {
+      this._setNotificationUIState(notificationEl, notification.options.checkbox.uncheckedState);
+    }
+    event.stopPropagation();
+  },
+
+  _onCommand(event) {
+    // Ignore events from buttons as they are submitting and so don't need checks
+    if (event.originalTarget.localName == "button") {
+      return;
+    }
+    let notificationEl = event.target;
+    // Find notification like getNotificationFromElement but some nodes are non-anon
+    while (notificationEl) {
+      if (notificationEl.localName == "popupnotification") {
+        break;
+      }
+      notificationEl =
+        notificationEl.ownerDocument.getBindingParent(notificationEl) || notificationEl.parentNode;
+    }
+    this._setNotificationUIState(notificationEl);
+  },
+
   _onButtonEvent(event, type, notificationEl = null) {
     if (!notificationEl) {
       notificationEl = getNotificationFromElement(event.originalTarget);
     }
 
     if (!notificationEl)
       throw "PopupNotifications._onButtonEvent: couldn't find notification element";