Bug 380637, add a general preference to prevent pages from overriding keyboard shortcuts. If a key doesn't specify the reserved attribute, this preference will be used, r=felipe
authorNeil Deakin <neil@mozilla.com>
Thu, 09 Nov 2017 18:42:39 -0500
changeset 444383 814806beaaf8d82d9ff8d57543263d8ca2720d51
parent 444317 a9930291f639145b9a612de69fca1804cfb4d4f3
child 444384 b65525d75c01cd73197b457f930c6415f3bfafdf
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfelipe
bugs380637
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 380637, add a general preference to prevent pages from overriding keyboard shortcuts. If a key doesn't specify the reserved attribute, this preference will be used, r=felipe
browser/app/profile/firefox.js
browser/base/content/test/permissions/browser.ini
browser/base/content/test/permissions/browser_reservedkey.js
dom/xbl/nsXBLPrototypeHandler.cpp
dom/xbl/nsXBLPrototypeHandler.h
dom/xbl/nsXBLWindowKeyHandler.cpp
extensions/cookie/nsPermissionManager.cpp
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -424,16 +424,17 @@ pref("browser.sessionhistory.max_entries
 pref("permissions.manager.defaultsUrl", "resource://app/defaults/permissions");
 
 // Set default fallback values for site permissions we want
 // the user to be able to globally change.
 pref("permissions.default.camera", 0);
 pref("permissions.default.microphone", 0);
 pref("permissions.default.geo", 0);
 pref("permissions.default.desktop-notification", 0);
+pref("permissions.default.shortcuts", 0);
 
 // handle links targeting new windows
 // 1=current window/tab, 2=new window, 3=new tab in most recent window
 pref("browser.link.open_newwindow", 3);
 
 // handle external links (i.e. links opened from a different application)
 // default: use browser.link.open_newwindow
 // 1-3: see browser.link.open_newwindow for interpretation
--- a/browser/base/content/test/permissions/browser.ini
+++ b/browser/base/content/test/permissions/browser.ini
@@ -1,14 +1,15 @@
 [DEFAULT]
 support-files=
   head.js
   permissions.html
 
 [browser_canvas_fingerprinting_resistance.js]
 [browser_permissions.js]
+[browser_reservedkey.js]
 [browser_temporary_permissions.js]
 support-files =
   temporary_permissions_subframe.html
   ../webrtc/get_user_media.html
 [browser_temporary_permissions_expiry.js]
 [browser_temporary_permissions_navigation.js]
 [browser_temporary_permissions_tabs.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/permissions/browser_reservedkey.js
@@ -0,0 +1,44 @@
+add_task(async function test_reserved_shortcuts() {
+  /* eslint-disable no-unsanitized/property */
+  let keyset = `<keyset>
+                  <key id='kt_reserved' modifiers='shift' key='O' reserved='true' count='0'
+                       oncommand='this.setAttribute("count", Number(this.getAttribute("count")) + 1)'/>
+                  <key id='kt_notreserved' modifiers='shift' key='P' reserved='false' count='0'
+                       oncommand='this.setAttribute("count", Number(this.getAttribute("count")) + 1)'/>
+                  <key id='kt_reserveddefault' modifiers='shift' key='Q' count='0'
+                       oncommand='this.setAttribute("count", Number(this.getAttribute("count")) + 1)'/>
+                </keyset>`;
+
+  let container = document.createElement("box");
+  container.innerHTML = keyset;
+  document.documentElement.appendChild(container);
+  /* eslint-enable no-unsanitized/property */
+
+  const pageUrl = "data:text/html,<body onload='document.body.firstChild.focus();'><div onkeydown='event.preventDefault();' tabindex=0>Test</div></body>";
+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, pageUrl);
+
+  EventUtils.synthesizeKey("O", { shiftKey: true });
+  EventUtils.synthesizeKey("P", { shiftKey: true });
+  EventUtils.synthesizeKey("Q", { shiftKey: true });
+
+  is(document.getElementById("kt_reserved").getAttribute("count"), "1", "reserved='true' with preference off");
+  is(document.getElementById("kt_notreserved").getAttribute("count"), "0", "reserved='false' with preference off");
+  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);
+  });
+
+  EventUtils.synthesizeKey("O", { shiftKey: true });
+  EventUtils.synthesizeKey("P", { shiftKey: true });
+  EventUtils.synthesizeKey("Q", { shiftKey: true });
+
+  is(document.getElementById("kt_reserved").getAttribute("count"), "2", "reserved='true' with preference on");
+  is(document.getElementById("kt_notreserved").getAttribute("count"), "0", "reserved='false' with preference on");
+  is(document.getElementById("kt_reserveddefault").getAttribute("count"), "1", "default reserved with preference on");
+
+  document.documentElement.removeChild(container);
+
+  await BrowserTestUtils.removeTab(tab);
+});
--- a/dom/xbl/nsXBLPrototypeHandler.cpp
+++ b/dom/xbl/nsXBLPrototypeHandler.cpp
@@ -88,44 +88,44 @@ nsXBLPrototypeHandler::nsXBLPrototypeHan
                                              const char16_t* aClickCount,
                                              const char16_t* aGroup,
                                              const char16_t* aPreventDefault,
                                              const char16_t* aAllowUntrusted,
                                              nsXBLPrototypeBinding* aBinding,
                                              uint32_t aLineNumber)
   : mHandlerText(nullptr),
     mLineNumber(aLineNumber),
-    mReserved(false),
+    mReserved(XBLReservedKey_False),
     mNextHandler(nullptr),
     mPrototypeBinding(aBinding)
 {
   Init();
 
   ConstructPrototype(nullptr, aEvent, aPhase, aAction, aCommand, aKeyCode,
                      aCharCode, aModifiers, aButton, aClickCount,
                      aGroup, aPreventDefault, aAllowUntrusted);
 }
 
-nsXBLPrototypeHandler::nsXBLPrototypeHandler(nsIContent* aHandlerElement, bool aReserved)
+nsXBLPrototypeHandler::nsXBLPrototypeHandler(nsIContent* aHandlerElement, XBLReservedKey aReserved)
   : mHandlerElement(nullptr),
     mLineNumber(0),
     mReserved(aReserved),
     mNextHandler(nullptr),
     mPrototypeBinding(nullptr)
 {
   Init();
 
   // Make sure our prototype is initialized.
   ConstructPrototype(aHandlerElement);
 }
 
 nsXBLPrototypeHandler::nsXBLPrototypeHandler(nsXBLPrototypeBinding* aBinding)
   : mHandlerText(nullptr),
     mLineNumber(0),
-    mReserved(false),
+    mReserved(XBLReservedKey_False),
     mNextHandler(nullptr),
     mPrototypeBinding(aBinding)
 {
   Init();
 }
 
 nsXBLPrototypeHandler::~nsXBLPrototypeHandler()
 {
--- a/dom/xbl/nsXBLPrototypeHandler.h
+++ b/dom/xbl/nsXBLPrototypeHandler.h
@@ -50,16 +50,25 @@ class KeyboardShortcut;
 #define NS_HANDLER_TYPE_SYSTEM              (1 << 6)
 #define NS_HANDLER_TYPE_PREVENTDEFAULT      (1 << 7)
 
 // XXX Use nsIDOMEvent:: codes?
 #define NS_PHASE_CAPTURING          1
 #define NS_PHASE_TARGET             2
 #define NS_PHASE_BUBBLING           3
 
+// Values of the reserved attribute. When unset, the default value depends on
+// the permissions.default.shortcuts preference.
+enum XBLReservedKey : uint8_t
+{
+  XBLReservedKey_False = 0,
+  XBLReservedKey_True = 1,
+  XBLReservedKey_Unset = 2,
+};
+
 class nsXBLPrototypeHandler
 {
   typedef mozilla::IgnoreModifierState IgnoreModifierState;
   typedef mozilla::layers::KeyboardShortcut KeyboardShortcut;
   typedef mozilla::Modifiers Modifiers;
 
 public:
   // This constructor is used by XBL handlers (both the JS and command shorthand variety)
@@ -69,17 +78,17 @@ public:
                         const char16_t* aModifiers, const char16_t* aButton,
                         const char16_t* aClickCount, const char16_t* aGroup,
                         const char16_t* aPreventDefault,
                         const char16_t* aAllowUntrusted,
                         nsXBLPrototypeBinding* aBinding,
                         uint32_t aLineNumber);
 
   // This constructor is used only by XUL key handlers (e.g., <key>)
-  explicit nsXBLPrototypeHandler(nsIContent* aKeyElement, bool aReserved);
+  explicit nsXBLPrototypeHandler(nsIContent* aKeyElement, XBLReservedKey aReserved);
 
   // This constructor is used for handlers loaded from the cache
   explicit nsXBLPrototypeHandler(nsXBLPrototypeBinding* aBinding);
 
   ~nsXBLPrototypeHandler();
 
   /**
    * Try and convert this XBL handler into an APZ KeyboardShortcut for handling
@@ -113,17 +122,17 @@ public:
   }
 
   already_AddRefed<nsIContent> GetHandlerElement();
 
   void AppendHandlerText(const nsAString& aText);
 
   uint8_t GetPhase() { return mPhase; }
   uint8_t GetType() { return mType; }
-  bool GetIsReserved() { return mReserved; }
+  XBLReservedKey GetIsReserved() { return mReserved; }
 
   nsXBLPrototypeHandler* GetNextHandler() { return mNextHandler; }
   void SetNextHandler(nsXBLPrototypeHandler* aHandler) { mNextHandler = aHandler; }
 
   nsresult ExecuteHandler(mozilla::dom::EventTarget* aTarget, nsIDOMEvent* aEvent);
 
   already_AddRefed<nsAtom> GetEventName();
   void SetEventName(nsAtom* aName) { mEventName = aName; }
@@ -228,17 +237,17 @@ protected:
   uint8_t mType;             // The type of the handler.  The handler is either a XUL key
                              // handler, an XBL "command" event, or a normal XBL event with
                              // accompanying JavaScript.  The high bit is used to indicate
                              // whether this handler should prevent the default action.
   uint8_t mMisc;             // Miscellaneous extra information.  For key events,
                              // stores whether or not we're a key code or char code.
                              // For mouse events, stores the clickCount.
 
-  bool mReserved;            // <key> is reserved for chrome. Not used by handlers.
+  XBLReservedKey mReserved;  // <key> is reserved for chrome. Not used by handlers.
 
   int32_t mKeyMask;          // Which modifier keys this event handler expects to have down
                              // in order to be matched.
 
   // The primary filter information for mouse/key events.
   int32_t mDetail;           // For key events, contains a charcode or keycode. For
                              // mouse events, stores the button info.
 
--- a/dom/xbl/nsXBLWindowKeyHandler.cpp
+++ b/dom/xbl/nsXBLWindowKeyHandler.cpp
@@ -206,18 +206,26 @@ BuildHandlerChain(nsIContent* aContent, 
       bool attrExists =
         key->GetAttr(kNameSpaceID_None, nsGkAtoms::key, valKey) ||
         key->GetAttr(kNameSpaceID_None, nsGkAtoms::charcode, valCharCode) ||
         key->GetAttr(kNameSpaceID_None, nsGkAtoms::keycode, valKeyCode);
       if (attrExists &&
           valKey.IsEmpty() && valCharCode.IsEmpty() && valKeyCode.IsEmpty())
         continue;
 
-      bool reserved = key->AttrValueIs(kNameSpaceID_None, nsGkAtoms::reserved,
-                                       nsGkAtoms::_true, eCaseMatters);
+      // reserved="pref" is the default for <key> elements.
+      XBLReservedKey reserved = XBLReservedKey_Unset;
+      if (key->AttrValueIs(kNameSpaceID_None, nsGkAtoms::reserved,
+                           nsGkAtoms::_true, eCaseMatters)) {
+        reserved = XBLReservedKey_True;
+      } else if (key->AttrValueIs(kNameSpaceID_None, nsGkAtoms::reserved,
+                                   nsGkAtoms::_false, eCaseMatters)) {
+        reserved = XBLReservedKey_False;
+      }
+
       nsXBLPrototypeHandler* handler = new nsXBLPrototypeHandler(key, reserved);
 
       handler->SetNextHandler(*aResult);
       *aResult = handler;
     }
   }
 }
 
@@ -720,17 +728,22 @@ nsXBLWindowKeyHandler::WalkHandlersAndEx
     // Before executing this handler, check that it's not disabled,
     // and that it has something to do (oncommand of the <key> or its
     // <command> is non-empty).
     nsCOMPtr<Element> commandElement;
     if (!GetElementForHandler(handler, getter_AddRefs(commandElement))) {
       continue;
     }
 
-    bool isReserved = handler->GetIsReserved();
+    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;
       }
--- a/extensions/cookie/nsPermissionManager.cpp
+++ b/extensions/cookie/nsPermissionManager.cpp
@@ -130,17 +130,18 @@ static const char* kPreloadPermissions[]
 };
 
 // A list of permissions that can have a fallback default permission
 // set under the permissions.default.* pref.
 static const char* kPermissionsWithDefaults[] = {
   "camera",
   "microphone",
   "geo",
-  "desktop-notification"
+  "desktop-notification",
+  "shortcuts"
 };
 
 // NOTE: nullptr can be passed as aType - if it is this function will return
 // "false" unconditionally.
 bool
 HasDefaultPref(const char* aType)
 {
   if (aType) {