Bug 380637, add site-specific permissions to prevent pages from overriding keyboard shortcuts. This is done by preventing the key combination from being sent to the content page, r=felipe

Users can block the overriding of shortcuts using the permissions tab of the page info dialog, as with other permissions. Site permissions also allows the use of permissions.default.shortcuts to block overriding shortcuts for all sites.
2017-11-09 18:42:39 -05:00 · 2017-11-09 18:42:39 -05:00 · ee8929c13f
--- a/browser/base/content/test/permissions/browser_permissions.js
+++ b/browser/base/content/test/permissions/browser_permissions.js
@ -176,3 +176,43 @@ add_task(async function testPermissionIcons() {
    SitePermissions.remove(gBrowser.currentURI, "camera");
  });
 });
+
+add_task(async function testPermissionShortcuts() {
+  await BrowserTestUtils.withNewTab(PERMISSIONS_PAGE, async function(browser) {
+    browser.focus();
+
+    await new Promise(r => {
+      SpecialPowers.pushPrefEnv({"set": [["permissions.default.shortcuts", 0]]}, r);
+    });
+
+    async function tryKey(desc, expectedValue) {
+      await EventUtils.synthesizeAndWaitKey("c", { accelKey: true });
+      let result = await ContentTask.spawn(browser, null, function() {
+        return content.wrappedJSObject.gKeyPresses;
+      });
+      is(result, expectedValue, desc);
+    }
+
+    await tryKey("pressed with default permissions", 1);
+
+    SitePermissions.set(gBrowser.currentURI, "shortcuts", SitePermissions.BLOCK);
+    await tryKey("pressed when site blocked", 1);
+
+    SitePermissions.set(gBrowser.currentURI, "shortcuts", SitePermissions.ALLOW);
+    await tryKey("pressed when site allowed", 2);
+
+    SitePermissions.remove(gBrowser.currentURI, "shortcuts");
+    await new Promise(r => {
+      SpecialPowers.pushPrefEnv({"set": [["permissions.default.shortcuts", 2]]}, r);
+    });
+
+    await tryKey("pressed when globally blocked", 2);
+    SitePermissions.set(gBrowser.currentURI, "shortcuts", SitePermissions.ALLOW);
+    await tryKey("pressed when globally blocked but site allowed", 3);
+
+    SitePermissions.set(gBrowser.currentURI, "shortcuts", SitePermissions.BLOCK);
+    await tryKey("pressed when globally blocked and site blocked", 3);
+
+    SitePermissions.remove(gBrowser.currentURI, "shortcuts");
+  });
+});
--- a/browser/base/content/test/permissions/browser_reservedkey.js
+++ b/browser/base/content/test/permissions/browser_reservedkey.js
@ -26,8 +26,8 @@ add_task(async function test_reserved_shortcuts() {
  is(document.getElementById("kt_reserveddefault").getAttribute("count"), "0", "default reserved with preference off");

  // Now try with reserved shortcut key handling enabled.
-  await new Promise(r => {
-    SpecialPowers.pushPrefEnv({"set": [["permissions.default.shortcuts", 2]]}, r);
+  await new Promise(resolve => {
+    SpecialPowers.pushPrefEnv({"set": [["permissions.default.shortcuts", 2]]}, resolve);
  });

  EventUtils.synthesizeKey("O", { shiftKey: true });
--- a/browser/base/content/test/permissions/permissions.html
+++ b/browser/base/content/test/permissions/permissions.html
@ -6,7 +6,10 @@
  <head>
    <meta charset="utf8">
  </head>
-  <body>
+<script>
+var gKeyPresses = 0;
+</script>
+  <body onkeypress="gKeyPresses++;">
 	<!-- This page could eventually request permissions from content
 	     and make sure that chrome responds appropriately -->
  <button id="geo" onclick="navigator.geolocation.getCurrentPosition(() => {})">Geolocation</button>
--- a/browser/locales/en-US/chrome/browser/sitePermissions.properties
+++ b/browser/locales/en-US/chrome/browser/sitePermissions.properties
@ -35,5 +35,6 @@ permission.screen.label = Share the Screen
 permission.install.label = Install Add-ons
 permission.popup.label = Open Pop-up Windows
 permission.geo.label = Access Your Location
+permission.shortcuts.label = Override Keyboard Shortcuts
 permission.focus-tab-by-prompt.label = Switch to this Tab
 permission.persistent-storage.label = Store Data in Persistent Storage
--- a/browser/modules/SitePermissions.jsm
+++ b/browser/modules/SitePermissions.jsm
@ -626,7 +626,11 @@ var gPermissionObject = {
  },
  "persistent-storage": {
    exactHostMatch: true
-  }
+  },
+
+  "shortcuts": {
+    states: [ SitePermissions.ALLOW, SitePermissions.BLOCK ],
+  },
 };

 // Delete this entry while being pre-off
--- a/browser/modules/test/browser/browser_SitePermissions.js
+++ b/browser/modules/test/browser/browser_SitePermissions.js
@ -25,10 +25,13 @@ add_task(async function testGetAllPermissionDetailsForBrowser() {

  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, uri.spec);

+  Services.prefs.setIntPref("permissions.default.shortcuts", 2);
+
  SitePermissions.set(uri, "camera", SitePermissions.ALLOW);
  SitePermissions.set(uri, "cookie", SitePermissions.ALLOW_COOKIES_FOR_SESSION);
  SitePermissions.set(uri, "popup", SitePermissions.BLOCK);
  SitePermissions.set(uri, "geo", SitePermissions.ALLOW, SitePermissions.SCOPE_SESSION);
+  SitePermissions.set(uri, "shortcuts", SitePermissions.ALLOW);

  let permissions = SitePermissions.getAllPermissionDetailsForBrowser(tab.linkedBrowser);

@ -71,9 +74,20 @@ add_task(async function testGetAllPermissionDetailsForBrowser() {
    scope: SitePermissions.SCOPE_SESSION,
  });

+  let shortcuts = permissions.find(({id}) => id === "shortcuts");
+  Assert.deepEqual(shortcuts, {
+    id: "shortcuts",
+    label: "Override Keyboard Shortcuts",
+    state: SitePermissions.ALLOW,
+    scope: SitePermissions.SCOPE_PERSISTENT,
+  });
+
  SitePermissions.remove(uri, "cookie");
  SitePermissions.remove(uri, "popup");
  SitePermissions.remove(uri, "geo");
+  SitePermissions.remove(uri, "shortcuts");
+
+  Services.prefs.clearUserPref("permissions.default.shortcuts");

  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
 });
--- a/browser/modules/test/unit/test_SitePermissions.js
+++ b/browser/modules/test/unit/test_SitePermissions.js
@ -10,7 +10,7 @@ const STORAGE_MANAGER_ENABLED = Services.prefs.getBoolPref("browser.storageManag

 add_task(async function testPermissionsListing() {
  let expectedPermissions = ["camera", "cookie", "desktop-notification", "focus-tab-by-prompt",
-     "geo", "image", "install", "microphone", "popup", "screen"];
+     "geo", "image", "install", "microphone", "popup", "screen", "shortcuts"];
  if (STORAGE_MANAGER_ENABLED) {
    // The persistent-storage permission is still only pref-on on Nightly
    // so we add it only when it's pref-on.
@ -58,6 +58,18 @@ add_task(async function testGetAllByURI() {
  SitePermissions.set(uri, "addon", SitePermissions.BLOCK);
  Assert.deepEqual(SitePermissions.getAllByURI(uri), []);
  SitePermissions.remove(uri, "addon");
+
+  Assert.equal(Services.prefs.getIntPref("permissions.default.shortcuts"), 0);
+  SitePermissions.set(uri, "shortcuts", SitePermissions.BLOCK);
+
+  // Customized preference should have been enabled, but the default should not.
+  Assert.equal(Services.prefs.getIntPref("permissions.default.shortcuts"), 0);
+  Assert.deepEqual(SitePermissions.getAllByURI(uri), [
+      { id: "shortcuts", state: SitePermissions.BLOCK, scope: SitePermissions.SCOPE_PERSISTENT },
+  ]);
+
+  SitePermissions.remove(uri, "shortcuts");
+  Services.prefs.clearUserPref("permissions.default.shortcuts");
 });

 add_task(async function testGetAvailableStates() {
@ -96,7 +108,7 @@ add_task(async function testExactHostMatch() {
    // Should remove this checking and add it as default after it is fully pref-on.
    exactHostMatched.push("persistent-storage");
  }
-  let nonExactHostMatched = ["image", "cookie", "popup", "install"];
+  let nonExactHostMatched = ["image", "cookie", "popup", "install", "shortcuts"];

  let permissions = SitePermissions.listPermissions();
  for (let permission of permissions) {
--- a/dom/interfaces/base/nsIRemoteBrowser.idl
+++ b/dom/interfaces/base/nsIRemoteBrowser.idl
@ -4,6 +4,8 @@

 #include "nsISupports.idl"

+interface nsIPrincipal;
+
 [scriptable, uuid(C8379366-F79F-4D25-89A6-22BEC0A93D16)]
 interface nsIRemoteBrowser : nsISupports
 {
@ -22,4 +24,6 @@ interface nsIRemoteBrowser : nsISupports
                             [array, size_is(enabledLength)] in string enabledCommands,
                             in unsigned long disabledLength,
                             [array, size_is(disabledLength)] in string disabledCommands);
+
+  readonly attribute nsIPrincipal contentPrincipal;
 };
--- a/dom/xbl/nsXBLWindowKeyHandler.cpp
+++ b/dom/xbl/nsXBLWindowKeyHandler.cpp
@ -23,6 +23,7 @@
 #include "nsPIDOMWindow.h"
 #include "nsIDocShell.h"
 #include "nsIDOMDocument.h"
+#include "nsIRemoteBrowser.h"
 #include "nsISelectionController.h"
 #include "nsIPresShell.h"
 #include "mozilla/EventListenerManager.h"
@ -733,16 +734,6 @@ nsXBLWindowKeyHandler::WalkHandlersAndExecute(
      continue;
    }

-    bool isReserved = handler->GetIsReserved() == XBLReservedKey_True;
-    if (handler->GetIsReserved() == XBLReservedKey_Unset &&
-        Preferences::GetInt("permissions.default.shortcuts") == 2) {
-      isReserved = true;
-    }
-
-    if (aOutReservedForChrome) {
-      *aOutReservedForChrome = isReserved;
-    }
-
    if (commandElement) {
      if (aExecute && !IsExecutableElement(commandElement)) {
        continue;
@ -751,23 +742,37 @@ nsXBLWindowKeyHandler::WalkHandlersAndExecute(

    if (!aExecute) {
      if (handler->EventTypeEquals(aEventType)) {
+        if (aOutReservedForChrome) {
+          *aOutReservedForChrome = IsReservedKey(widgetKeyboardEvent, handler);
+        }
+
        return true;
      }
+
      // If the command is reserved and the event is keydown, check also if
      // the handler is for keypress because if following keypress event is
      // reserved, we shouldn't dispatch the event into web contents.
-      if (isReserved &&
-          aEventType == nsGkAtoms::keydown &&
+      if (aEventType == nsGkAtoms::keydown &&
          handler->EventTypeEquals(nsGkAtoms::keypress)) {
-        return true;
+        if (IsReservedKey(widgetKeyboardEvent, handler)) {
+          if (aOutReservedForChrome) {
+            *aOutReservedForChrome = true;
+          }
+
+          return true;
+        }
      }
      // Otherwise, we've not found a handler for the event yet.
      continue;
    }

+    // This should only be assigned when aExecute is false.
+    MOZ_ASSERT(!aOutReservedForChrome);
+
    // If it's not reserved and the event is a key event on a plugin,
    // the handler shouldn't be executed.
-    if (!isReserved && widgetKeyboardEvent->IsKeyEventOnPlugin()) {
+    if (widgetKeyboardEvent->IsKeyEventOnPlugin() &&
+        !IsReservedKey(widgetKeyboardEvent, handler)) {
      return false;
    }

@ -804,6 +809,50 @@ nsXBLWindowKeyHandler::WalkHandlersAndExecute(
  return false;
 }

+bool
+nsXBLWindowKeyHandler::IsReservedKey(WidgetKeyboardEvent* aKeyEvent,
+                                     nsXBLPrototypeHandler* aHandler)
+{
+  XBLReservedKey reserved = aHandler->GetIsReserved();
+  // reserved="true" means that the key is always reserved. reserved="false"
+  // means that the key is never reserved. Otherwise, we check site-specific
+  // permissions.
+  if (reserved == XBLReservedKey_True) {
+    return true;
+  }
+
+  if (reserved == XBLReservedKey_Unset) {
+    nsCOMPtr<nsIPrincipal> principal;
+    nsCOMPtr<nsIRemoteBrowser> targetBrowser = do_QueryInterface(aKeyEvent->mOriginalTarget);
+    if (targetBrowser) {
+      targetBrowser->GetContentPrincipal(getter_AddRefs(principal));
+    }
+    else {
+      // Get the top-level document.
+      nsCOMPtr<nsIContent> content = do_QueryInterface(aKeyEvent->mOriginalTarget);
+      if (content) {
+        nsIDocument* doc = content->GetUncomposedDoc();
+        if (doc) {
+          nsCOMPtr<nsIDocShellTreeItem> docShell = doc->GetDocShell();
+          if (docShell && docShell->ItemType() == nsIDocShellTreeItem::typeContent) {
+            nsCOMPtr<nsIDocShellTreeItem> rootItem;
+            docShell->GetSameTypeRootTreeItem(getter_AddRefs(rootItem));
+            if (rootItem && rootItem->GetDocument()) {
+              principal = rootItem->GetDocument()->NodePrincipal();
+            }
+          }
+        }
+      }
+    }
+
+    if (principal) {
+      return nsContentUtils::IsSitePermDeny(principal, "shortcuts");
+    }
+  }
+
+  return false;
+}
+
 bool
 nsXBLWindowKeyHandler::HasHandlerForEvent(nsIDOMKeyEvent* aEvent,
                                          bool* aOutReservedForChrome)
--- a/dom/xbl/nsXBLWindowKeyHandler.h
+++ b/dom/xbl/nsXBLWindowKeyHandler.h
@ -19,6 +19,7 @@ class nsXBLPrototypeHandler;

 namespace mozilla {
 class EventListenerManager;
+class WidgetKeyboardEvent;
 struct IgnoreModifierState;
 namespace dom {
 class Element;
@ -77,6 +78,11 @@ protected:
  bool HasHandlerForEvent(nsIDOMKeyEvent* aEvent,
                          bool* aOutReservedForChrome = nullptr);

+  // Returns true if the key would be reserved for the given handler. A reserved
+  // key is not sent to a content process or single-process equivalent.
+  bool IsReservedKey(mozilla::WidgetKeyboardEvent* aKeyEvent,
+                     nsXBLPrototypeHandler* aHandler);
+
  // Returns event type for matching between aWidgetKeyboardEvent and
  // shortcut key handlers.  This is used for calling WalkHandlers(),
  // WalkHandlersInternal() and WalkHandlersAndExecute().