Bug 1479964 - Set KeyboardEvent.keyCode and KeyboardEvent.charCode to same value if the event is "keypress" event r=smaug

Chrome sets both KeyboardEvent.keyCode and KeyboardEvent.charCode of "keypress"
event to same value.  On the other hand, our traditional behavior is, sets
one of them to 0.

Therefore, we need to set keyCode value to charCode value if the keypress
event is caused by a non-function key, i.e., it may be a printable key with
specific modifier state and/or different keyboard layout for compatibility
with Chrome.  Similarly, we need to set charCode value to keyCode value if
the keypress event is caused by a function key which is not mapped to producing
a character.

Note that this hack is for compatibility with Chrome.  So, for now, it's enough
to change the behavior only for "keypress" event handlers in web content.  If
we completely change the behavior, we need to fix a lot of default handlers
and mochitests too.  However, it's really difficult because default handlers
check whether keypress events are printable or not with following code:

> if (event.charCode &&
>     !event.altKey && !event.ctrlKey && !event.metaKey) {

or

> if (!event.keyCode &&
>     !event.altKey && !event.ctrlKey && !event.metaKey) {

So, until we stop dispatching "keypress" events for non-printable keys,
we need complicated check in each of them.

And also note that this patch changes the behavior of KeyboardEvent::KeyCode()
when spoofing is enabled and the instance is initialized by initKeyEvent() or
initKeyboardEvent().  That was changed by bug 1222285 unexpectedly and keeping
the behavior makes patched code really ugly.  Therefore, this takes back the
old behavior even if spoofing is enabled.

Differential Revision: https://phabricator.services.mozilla.com/D7974

--HG--
extra : moz-landing-system : lando

browser/components/resistfingerprinting/test/browser/browser_spoofing_keyboard_event.js

@@ -7,7 +7,8 @@ const CC = Components.Constructor;
 
 const kStrictKeyPressEvents =
   SpecialPowers.getBoolPref("dom.keyboardevent.keypress.dispatch_non_printable_keys_only_system_group_in_content");
-
+const kSameKeyCodeAndCharCodeValue =
+  SpecialPowers.getBoolPref("dom.keyboardevent.keypress.set_keycode_and_charcode_to_same_value");
 const SHOULD_DELIVER_KEYDOWN          = 0x1;
 const SHOULD_DELIVER_KEYPRESS         = 0x2;
 const SHOULD_DELIVER_KEYUP            = 0x4;
@@ -652,8 +653,8 @@ async function testKeyEvent(aTab, aTestCase) {
   let allKeyEventPromises = [];
 
   for (let testEvent of testEvents) {
-    let keyEventPromise = ContentTask.spawn(aTab.linkedBrowser, {testEvent, result: aTestCase.result}, async (aInput) => {
+    let keyEventPromise = ContentTask.spawn(aTab.linkedBrowser, {testEvent, result: aTestCase.result, kSameKeyCodeAndCharCodeValue}, async (aInput) => {
-      function verifyKeyboardEvent(aEvent, aResult) {
+      function verifyKeyboardEvent(aEvent, aResult, aSameKeyCodeAndCharCodeValue) {
         is(aEvent.key, aResult.key, "KeyboardEvent.key is correctly spoofed.");
         is(aEvent.code, aResult.code, "KeyboardEvent.code is correctly spoofed.");
         is(aEvent.location, aResult.location, "KeyboardEvent.location is correctly spoofed.");
@@ -663,12 +664,20 @@ async function testKeyEvent(aTab, aTestCase) {
 
         // If the charCode is not 0, this is a character. The keyCode will be remained as 0.
         // Otherwise, we should check the keyCode.
-        if (aEvent.charCode != 0) {
+        if (!aSameKeyCodeAndCharCodeValue) {
-          is(aEvent.keyCode, 0, "KeyboardEvent.keyCode should be 0 for this case.");
+          if (aEvent.charCode != 0) {
+            is(aEvent.keyCode, 0, "KeyboardEvent.keyCode should be 0 for this case.");
+            is(aEvent.charCode, aResult.charCode, "KeyboardEvent.charCode is correctly spoofed.");
+          } else {
+            is(aEvent.keyCode, aResult.keyCode, "KeyboardEvent.keyCode is correctly spoofed.");
+            is(aEvent.charCode, 0, "KeyboardEvent.charCode should be 0 for this case.");
+          }
+        } else if (aResult.charCode) {
+          is(aEvent.keyCode, aResult.charCode, "KeyboardEvent.keyCode should be same as expected charCode for this case.");
           is(aEvent.charCode, aResult.charCode, "KeyboardEvent.charCode is correctly spoofed.");
         } else {
           is(aEvent.keyCode, aResult.keyCode, "KeyboardEvent.keyCode is correctly spoofed.");
-          is(aEvent.charCode, 0, "KeyboardEvent.charCode should be 0 for this case.");
+          is(aEvent.charCode, aResult.keyCode, "KeyboardEvent.charCode should be same as expected keyCode for this case.");
         }
 
         // Check getModifierState().
@@ -682,7 +691,7 @@ async function testKeyEvent(aTab, aTestCase) {
             `KeyboardEvent.getModifierState() reports a correctly spoofed value for 'Control'.`);
       }
 
-      let {testEvent: eventType, result} = aInput;
+      let {testEvent: eventType, result, kSameKeyCodeAndCharCodeValue: sameKeyCodeAndCharCodeValue} = aInput;
       let inputBox = content.document.getElementById("test");
 
       // We need to put the real access of event object into the content page instead of
@@ -708,7 +717,8 @@ async function testKeyEvent(aTab, aTestCase) {
       // result.
       await new Promise(resolve => {
         function eventHandler(aEvent) {
-          verifyKeyboardEvent(JSON.parse(resElement.value), result);
+          verifyKeyboardEvent(JSON.parse(resElement.value), result,
+                              eventType == "keypress" && sameKeyCodeAndCharCodeValue);
           resElement.removeEventListener("resultAvailable", eventHandler, true);
           resolve();
         }

dom/bindings/Bindings.conf

@@ -494,6 +494,10 @@ DOMInterfaces = {
     'headerFile': 'DOMIntersectionObserver.h',
 },
 
+'KeyboardEvent': {
+    'binaryNames': { 'constructor': 'ConstructorJS' },
+},
+
 'KeyEvent': {
     'concrete': False
 },

dom/events/KeyboardEvent.cpp

@@ -18,6 +18,7 @@ KeyboardEvent::KeyboardEvent(EventTarget* aOwner,
   : UIEvent(aOwner, aPresContext,
             aEvent ? aEvent :
                      new WidgetKeyboardEvent(false, eVoidEvent, nullptr))
+  , mInitializedByJS(false)
   , mInitializedByCtor(false)
   , mInitializedWhichValue(0)
 {
@@ -145,47 +146,106 @@ void KeyboardEvent::GetInitDict(KeyboardEventInit& aParam)
   aParam.mCancelable = internalEvent->mFlags.mCancelable;
 }
 
-uint32_t
+bool
-KeyboardEvent::CharCode()
+KeyboardEvent::ShouldUseSameValueForCharCodeAndKeyCode(
+                 CallerType aCallerType) const
 {
-  // If this event is initialized with ctor, we shouldn't check event type.
+  // - If this event is initialized by JS, we don't need to return same value
-  if (mInitializedByCtor) {
+  //   for keyCode and charCode since they can be initialized separately.
-    return mEvent->AsKeyboardEvent()->mCharCode;
+  // - If this is not a keypress event, we shouldn't return same value for
+  //   keyCode and charCode.
+  // - If this event is referred by default handler, i.e., the caller is
+  //   system or this event is now in the system group, we don't need to use
+  //   hack for web-compat.
+  if (mInitializedByJS ||
+      mEvent->mMessage != eKeyPress ||
+      aCallerType == CallerType::System ||
+      mEvent->mFlags.mInSystemGroup) {
+    return false;
   }
 
-  switch (mEvent->mMessage) {
+  MOZ_ASSERT(aCallerType == CallerType::NonSystem);
-  case eKeyDown:
+
-  case eKeyDownOnPlugin:
+  return StaticPrefs::
-  case eKeyUp:
+    dom_keyboardevent_keypress_set_keycode_and_charcode_to_same_value();
-  case eKeyUpOnPlugin:
+}
-    return 0;
+
-  case eKeyPress:
+uint32_t
-  case eAccessKeyNotFound:
+KeyboardEvent::CharCode(CallerType aCallerType)
-    return mEvent->AsKeyboardEvent()->mCharCode;
+{
-  default:
+  WidgetKeyboardEvent* widgetKeyboardEvent = mEvent->AsKeyboardEvent();
-    break;
+  if (mInitializedByJS) {
+    // If this is initialized by Ctor, we should return the initialized value.
+    if (mInitializedByCtor) {
+      return widgetKeyboardEvent->mCharCode;
+    }
+    // Otherwise, i.e., initialized by InitKey*Event(), we should return the
+    // initialized value only when eKeyPress or eAccessKeyNotFound event.
+    // Although this is odd, but our traditional behavior.
+    return widgetKeyboardEvent->mMessage == eKeyPress ||
+           widgetKeyboardEvent->mMessage == eAccessKeyNotFound ?
+             widgetKeyboardEvent->mCharCode : 0;
   }
-  return 0;
+
+  // If the key is a function key, we should return the result of KeyCode()
+  // even from CharCode().  Otherwise, i.e., the key may be a printable
+  // key or actually a printable key, we should return the given charCode
+  // value.
+
+  if (widgetKeyboardEvent->mKeyNameIndex != KEY_NAME_INDEX_USE_STRING &&
+      ShouldUseSameValueForCharCodeAndKeyCode(aCallerType)) {
+    return ComputeTraditionalKeyCode(*widgetKeyboardEvent, aCallerType);
+  }
+
+  return widgetKeyboardEvent->mCharCode;
 }
 
 uint32_t
 KeyboardEvent::KeyCode(CallerType aCallerType)
 {
-  // If this event is initialized with ctor, we shouldn't check event type.
+  WidgetKeyboardEvent* widgetKeyboardEvent = mEvent->AsKeyboardEvent();
-  if (mInitializedByCtor) {
+  if (mInitializedByJS) {
-    return mEvent->AsKeyboardEvent()->mKeyCode;
+    // If this is initialized by Ctor, we should return the initialized value.
+    if (mInitializedByCtor) {
+      return widgetKeyboardEvent->mKeyCode;
+    }
+    // Otherwise, i.e., initialized by InitKey*Event(), we should return the
+    // initialized value only when the event message is a valid keyboard event
+    // message.  Although this is odd, but our traditional behavior.
+    // NOTE: The fix of bug 1222285 changed the behavior temporarily if
+    //       spoofing is enabled.  However, the behavior does not make sense
+    //       since if the event is generated by JS, the behavior shouldn't
+    //       be changed by whether spoofing is enabled or not.  Therefore,
+    //       we take back the original behavior.
+    return widgetKeyboardEvent->HasKeyEventMessage() ?
+             widgetKeyboardEvent->mKeyCode : 0;
   }
 
-  if (!mEvent->HasKeyEventMessage()) {
+  // If the key is not a function key, i.e., the key may be a printable key
-    return 0;
+  // or a function key mapped as a printable key, we should use charCode value
+  // for keyCode value if this is a "keypress" event.
+
+  if (widgetKeyboardEvent->mKeyNameIndex == KEY_NAME_INDEX_USE_STRING &&
+      ShouldUseSameValueForCharCodeAndKeyCode(aCallerType)) {
+    return widgetKeyboardEvent->mCharCode;
   }
 
+  return ComputeTraditionalKeyCode(*widgetKeyboardEvent, aCallerType);
+}
+
+uint32_t
+KeyboardEvent::ComputeTraditionalKeyCode(WidgetKeyboardEvent& aKeyboardEvent,
+                                         CallerType aCallerType)
+{
   if (!ShouldResistFingerprinting(aCallerType)) {
-    return mEvent->AsKeyboardEvent()->mKeyCode;
+    return aKeyboardEvent.mKeyCode;
   }
 
-  // The keyCode should be zero if the char code is given.
+  // In Netscape style (i.e., traditional behavior of Gecko), the keyCode
-  if (CharCode()) {
+  // should be zero if the char code is given.
+  if ((mEvent->mMessage == eKeyPress ||
+       mEvent->mMessage == eAccessKeyNotFound) &&
+      aKeyboardEvent.mCharCode) {
     return 0;
   }
 
@@ -194,8 +254,7 @@ KeyboardEvent::KeyCode(CallerType aCallerType)
   nsCOMPtr<nsIDocument> doc = GetDocument();
   uint32_t spoofedKeyCode;
 
-  if (nsRFPService::GetSpoofedKeyCode(doc, mEvent->AsKeyboardEvent(),
+  if (nsRFPService::GetSpoofedKeyCode(doc, &aKeyboardEvent, spoofedKeyCode)) {
-                                      spoofedKeyCode)) {
     return spoofedKeyCode;
   }
 
@@ -241,10 +300,10 @@ KeyboardEvent::Location()
 
 // static
 already_AddRefed<KeyboardEvent>
-KeyboardEvent::Constructor(const GlobalObject& aGlobal,
+KeyboardEvent::ConstructorJS(const GlobalObject& aGlobal,
-                           const nsAString& aType,
+                             const nsAString& aType,
-                           const KeyboardEventInit& aParam,
+                             const KeyboardEventInit& aParam,
-                           ErrorResult& aRv)
+                             ErrorResult& aRv)
 {
   nsCOMPtr<EventTarget> target = do_QueryInterface(aGlobal.GetAsSupports());
   RefPtr<KeyboardEvent> newEvent =
@@ -261,12 +320,13 @@ KeyboardEvent::InitWithKeyboardEventInit(EventTarget* aOwner,
                                          ErrorResult& aRv)
 {
   bool trusted = Init(aOwner);
-  InitKeyEvent(aType, aParam.mBubbles, aParam.mCancelable,
+  InitKeyEventJS(aType, aParam.mBubbles, aParam.mCancelable,
-               aParam.mView, false, false, false, false,
+                 aParam.mView, false, false, false, false,
-               aParam.mKeyCode, aParam.mCharCode);
+                 aParam.mKeyCode, aParam.mCharCode);
   InitModifiers(aParam);
   SetTrusted(trusted);
   mDetail = aParam.mDetail;
+  mInitializedByJS = true;
   mInitializedByCtor = true;
   mInitializedWhichValue = aParam.mWhich;
 
@@ -287,13 +347,15 @@ KeyboardEvent::InitWithKeyboardEventInit(EventTarget* aOwner,
 }
 
 void
-KeyboardEvent::InitKeyEvent(const nsAString& aType, bool aCanBubble,
+KeyboardEvent::InitKeyEventJS(const nsAString& aType, bool aCanBubble,
-                            bool aCancelable, nsGlobalWindowInner* aView,
+                              bool aCancelable, nsGlobalWindowInner* aView,
-                            bool aCtrlKey, bool aAltKey,
+                              bool aCtrlKey, bool aAltKey,
-                            bool aShiftKey, bool aMetaKey,
+                              bool aShiftKey, bool aMetaKey,
-                            uint32_t aKeyCode, uint32_t aCharCode)
+                              uint32_t aKeyCode, uint32_t aCharCode)
 {
   NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);
+  mInitializedByJS = true;
+  mInitializedByCtor = false;
 
   UIEvent::InitUIEvent(aType, aCanBubble, aCancelable, aView, 0);
 
@@ -304,19 +366,21 @@ KeyboardEvent::InitKeyEvent(const nsAString& aType, bool aCanBubble,
 }
 
 void
-KeyboardEvent::InitKeyboardEvent(const nsAString& aType,
+KeyboardEvent::InitKeyboardEventJS(const nsAString& aType,
-                                 bool aCanBubble,
+                                   bool aCanBubble,
-                                 bool aCancelable,
+                                   bool aCancelable,
-                                 nsGlobalWindowInner* aView,
+                                   nsGlobalWindowInner* aView,
-                                 const nsAString& aKey,
+                                   const nsAString& aKey,
-                                 uint32_t aLocation,
+                                   uint32_t aLocation,
-                                 bool aCtrlKey,
+                                   bool aCtrlKey,
-                                 bool aAltKey,
+                                   bool aAltKey,
-                                 bool aShiftKey,
+                                   bool aShiftKey,
-                                 bool aMetaKey,
+                                   bool aMetaKey,
-                                 ErrorResult& aRv)
+                                   ErrorResult& aRv)
 {
   NS_ENSURE_TRUE_VOID(!mEvent->mFlags.mIsBeingDispatched);
+  mInitializedByJS = true;
+  mInitializedByCtor = false;
 
   UIEvent::InitUIEvent(aType, aCanBubble, aCancelable, aView, 0);
 
@@ -349,13 +413,13 @@ bool
 KeyboardEvent::ShouldResistFingerprinting(CallerType aCallerType)
 {
   // There are five situations we don't need to spoof this keyboard event.
-  //   1. This event is generated by scripts.
+  //   1. This event is initialized by scripts.
   //   2. This event is from Numpad.
   //   3. This event is in the system group.
   //   4. The caller type is system.
   //   5. The pref privcy.resistFingerprinting' is false, we fast return here since
   //      we don't need to do any QI of following codes.
-  if (mInitializedByCtor ||
+  if (mInitializedByJS ||
       aCallerType == CallerType::System ||
       mEvent->mFlags.mInSystemGroup ||
       !nsContentUtils::ShouldResistFingerprinting() ||

dom/events/KeyboardEvent.h

@@ -29,7 +29,7 @@ public:
     return this;
   }
 
-  static already_AddRefed<KeyboardEvent> Constructor(
+  static already_AddRefed<KeyboardEvent> ConstructorJS(
                                            const GlobalObject& aGlobal,
                                            const nsAString& aType,
                                            const KeyboardEventInit& aParam,
@@ -63,7 +63,7 @@ public:
   bool Repeat();
   bool IsComposing();
   void GetKey(nsAString& aKey) const;
-  uint32_t CharCode();
+  uint32_t CharCode(CallerType aCallerType = CallerType::System);
   uint32_t KeyCode(CallerType aCallerType = CallerType::System);
   virtual uint32_t Which(CallerType aCallerType = CallerType::System) override;
   uint32_t Location();
@@ -71,16 +71,16 @@ public:
   void GetCode(nsAString& aCode, CallerType aCallerType = CallerType::System);
   void GetInitDict(KeyboardEventInit& aParam);
 
-  void InitKeyEvent(const nsAString& aType, bool aCanBubble, bool aCancelable,
+  void InitKeyEventJS(const nsAString& aType, bool aCanBubble, bool aCancelable,
-                    nsGlobalWindowInner* aView, bool aCtrlKey, bool aAltKey,
+                      nsGlobalWindowInner* aView, bool aCtrlKey, bool aAltKey,
-                    bool aShiftKey, bool aMetaKey,
+                      bool aShiftKey, bool aMetaKey,
-                    uint32_t aKeyCode, uint32_t aCharCode);
+                      uint32_t aKeyCode, uint32_t aCharCode);
 
-  void InitKeyboardEvent(const nsAString& aType,
+  void InitKeyboardEventJS(const nsAString& aType,
-                         bool aCanBubble, bool aCancelable,
+                           bool aCanBubble, bool aCancelable,
-                         nsGlobalWindowInner* aView, const nsAString& aKey,
+                           nsGlobalWindowInner* aView, const nsAString& aKey,
-                         uint32_t aLocation, bool aCtrlKey, bool aAltKey,
+                           uint32_t aLocation, bool aCtrlKey, bool aAltKey,
-                         bool aShiftKey, bool aMetaKey, ErrorResult& aRv);
+                           bool aShiftKey, bool aMetaKey, ErrorResult& aRv);
 
 protected:
   ~KeyboardEvent() {}
@@ -91,7 +91,9 @@ protected:
                                  ErrorResult& aRv);
 
 private:
-  // True, if the instance is created with Constructor().
+  // True, if the instance is initialized by JS.
+  bool mInitializedByJS;
+  // True, if the instance is initialized by Ctor.
   bool mInitializedByCtor;
 
   // If the instance is created with Constructor(), which may have independent
@@ -113,6 +115,26 @@ private:
   // for fingerprinting resistance.
   bool GetSpoofedModifierStates(const Modifiers aModifierKey,
                                 const bool aRawModifierState);
+
+  /**
+   * ComputeTraditionalKeyCode() computes traditional keyCode value.  I.e.,
+   * returns 0 if this event should return non-zero from CharCode().
+   * In spite of the name containing "traditional", this computes spoof
+   * keyCode value if user wants it.
+   *
+   * @param aKeyboardEvent  Should be |*mEvent->AsKeyboardEvent()|.
+   * @param aCallerType     Set caller type of KeyCode() or CharCode().
+   * @return                If traditional charCode value is 0, returns
+   *                        the raw keyCode value or spoof keyCode value.
+   *                        Otherwise, 0.
+   */
+  uint32_t ComputeTraditionalKeyCode(WidgetKeyboardEvent& aKeyboardEvent,
+                                     CallerType aCallerType);
+  /**
+   * ShouldUseSameValueForCharCodeAndKeyCode() returns true if KeyCode() and
+   * CharCode() should return same value.
+   */
+  bool ShouldUseSameValueForCharCodeAndKeyCode(CallerType aCallerType) const;
 };
 
 } // namespace dom

dom/events/test/test_dom_keyboard_event.html

@@ -40,7 +40,7 @@ function testInitializingUntrustedEvent()
     }, // 1
 
     { createEventArg: "Keyboardevent", useInitKeyboardEvent: false,
-      type: "keypess", bubbles: true, cancelable: false, view: null,
+      type: "keypress", bubbles: true, cancelable: false, view: null,
       ctrlKey: false, altKey: true, shiftKey: false, metaKey: false,
       keyCode: 0x11, charCode: 0x30, detail: 0, key: "", location: 0,
     }, // 2
@@ -101,7 +101,7 @@ function testInitializingUntrustedEvent()
     }, // 11
 
     { createEventArg: "Keyboardevent", useInitKeyboardEvent: true,
-      type: "keypess", bubbles: true, cancelable: false, view: null,
+      type: "keypress", bubbles: true, cancelable: false, view: null,
       ctrlKey: false, altKey: true, shiftKey: false, metaKey: false,
       keyCode: 0x00, charCode: 0x00, key: "FooBar", location: 2,
     }, // 12

dom/webidl/KeyEvent.webidl

@@ -230,6 +230,7 @@ interface KeyEvent
   // for compatibility with the other web browsers on Windows.
   const unsigned long DOM_VK_WIN_OEM_CLEAR  = 0xFE;
 
+  [BinaryName="initKeyEventJS"]
   void initKeyEvent(DOMString type,
                     optional boolean canBubble = false,
                     optional boolean cancelable = false,

dom/webidl/KeyboardEvent.webidl

@@ -7,6 +7,7 @@
 [Constructor(DOMString typeArg, optional KeyboardEventInit keyboardEventInitDict)]
 interface KeyboardEvent : UIEvent
 {
+  [NeedsCallerType]
   readonly attribute unsigned long    charCode;
   [NeedsCallerType]
   readonly attribute unsigned long    keyCode;
@@ -35,7 +36,7 @@ interface KeyboardEvent : UIEvent
   [NeedsCallerType]
   readonly attribute DOMString code;
 
-  [Throws]
+  [Throws, BinaryName="initKeyboardEventJS"]
   void initKeyboardEvent(DOMString typeArg,
                          optional boolean bubblesArg = false,
                          optional boolean cancelableArg = false,

modules/libpref/init/StaticPrefList.h

@@ -179,6 +179,20 @@ VARCACHE_PREF(
 )
 #undef PREF_VALUE
 
+// If this is true, "keypress" event's keyCode value and charCode value always
+// become same if the event is not created/initialized by JS.
+#ifdef RELEASE_OR_BETA
+# define PREF_VALUE false
+#else
+# define PREF_VALUE true
+#endif
+VARCACHE_PREF(
+  "dom.keyboardevent.keypress.set_keycode_and_charcode_to_same_value",
+   dom_keyboardevent_keypress_set_keycode_and_charcode_to_same_value,
+  bool, PREF_VALUE
+)
+#undef PREF_VALUE
+
 // NOTE: This preference is used in unit tests. If it is removed or its default
 // value changes, please update test_sharedMap_var_caches.js accordingly.
 VARCACHE_PREF(