From 06267cb849a7d8673a1674fef078bb2ca3133e54 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 9 Oct 2018 04:43:37 +0000 Subject: [PATCH] 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_spoofing_keyboard_event.js | 26 ++- dom/bindings/Bindings.conf | 4 + dom/events/KeyboardEvent.cpp | 168 ++++++++++++------ dom/events/KeyboardEvent.h | 46 +++-- dom/events/test/test_dom_keyboard_event.html | 4 +- dom/webidl/KeyEvent.webidl | 1 + dom/webidl/KeyboardEvent.webidl | 3 +- modules/libpref/init/StaticPrefList.h | 14 ++ 8 files changed, 191 insertions(+), 75 deletions(-) diff --git a/browser/components/resistfingerprinting/test/browser/browser_spoofing_keyboard_event.js b/browser/components/resistfingerprinting/test/browser/browser_spoofing_keyboard_event.js index 9713a101f96a..1db696055037 100644 --- a/browser/components/resistfingerprinting/test/browser/browser_spoofing_keyboard_event.js +++ b/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) => { - function verifyKeyboardEvent(aEvent, aResult) { + let keyEventPromise = ContentTask.spawn(aTab.linkedBrowser, {testEvent, result: aTestCase.result, kSameKeyCodeAndCharCodeValue}, async (aInput) => { + 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) { - is(aEvent.keyCode, 0, "KeyboardEvent.keyCode should be 0 for this case."); + if (!aSameKeyCodeAndCharCodeValue) { + 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(); } diff --git a/dom/bindings/Bindings.conf b/dom/bindings/Bindings.conf index 2c859c4d2ae3..10037b37979d 100644 --- a/dom/bindings/Bindings.conf +++ b/dom/bindings/Bindings.conf @@ -494,6 +494,10 @@ DOMInterfaces = { 'headerFile': 'DOMIntersectionObserver.h', }, +'KeyboardEvent': { + 'binaryNames': { 'constructor': 'ConstructorJS' }, +}, + 'KeyEvent': { 'concrete': False }, diff --git a/dom/events/KeyboardEvent.cpp b/dom/events/KeyboardEvent.cpp index c26a894439d9..030d5c45be9f 100644 --- a/dom/events/KeyboardEvent.cpp +++ b/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 -KeyboardEvent::CharCode() +bool +KeyboardEvent::ShouldUseSameValueForCharCodeAndKeyCode( + CallerType aCallerType) const { - // If this event is initialized with ctor, we shouldn't check event type. - if (mInitializedByCtor) { - return mEvent->AsKeyboardEvent()->mCharCode; + // - If this event is initialized by JS, we don't need to return same value + // for keyCode and charCode since they can be initialized separately. + // - 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) { - case eKeyDown: - case eKeyDownOnPlugin: - case eKeyUp: - case eKeyUpOnPlugin: - return 0; - case eKeyPress: - case eAccessKeyNotFound: - return mEvent->AsKeyboardEvent()->mCharCode; - default: - break; + MOZ_ASSERT(aCallerType == CallerType::NonSystem); + + return StaticPrefs:: + dom_keyboardevent_keypress_set_keycode_and_charcode_to_same_value(); +} + +uint32_t +KeyboardEvent::CharCode(CallerType aCallerType) +{ + WidgetKeyboardEvent* widgetKeyboardEvent = mEvent->AsKeyboardEvent(); + 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. - if (mInitializedByCtor) { - return mEvent->AsKeyboardEvent()->mKeyCode; + WidgetKeyboardEvent* widgetKeyboardEvent = mEvent->AsKeyboardEvent(); + if (mInitializedByJS) { + // 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()) { - return 0; + // If the key is not a function key, i.e., the key may be a printable key + // 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. - if (CharCode()) { + // In Netscape style (i.e., traditional behavior of Gecko), the keyCode + // 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 doc = GetDocument(); uint32_t spoofedKeyCode; - if (nsRFPService::GetSpoofedKeyCode(doc, mEvent->AsKeyboardEvent(), - spoofedKeyCode)) { + if (nsRFPService::GetSpoofedKeyCode(doc, &aKeyboardEvent, spoofedKeyCode)) { return spoofedKeyCode; } @@ -241,10 +300,10 @@ KeyboardEvent::Location() // static already_AddRefed -KeyboardEvent::Constructor(const GlobalObject& aGlobal, - const nsAString& aType, - const KeyboardEventInit& aParam, - ErrorResult& aRv) +KeyboardEvent::ConstructorJS(const GlobalObject& aGlobal, + const nsAString& aType, + const KeyboardEventInit& aParam, + ErrorResult& aRv) { nsCOMPtr target = do_QueryInterface(aGlobal.GetAsSupports()); RefPtr newEvent = @@ -261,12 +320,13 @@ KeyboardEvent::InitWithKeyboardEventInit(EventTarget* aOwner, ErrorResult& aRv) { bool trusted = Init(aOwner); - InitKeyEvent(aType, aParam.mBubbles, aParam.mCancelable, - aParam.mView, false, false, false, false, - aParam.mKeyCode, aParam.mCharCode); + InitKeyEventJS(aType, aParam.mBubbles, aParam.mCancelable, + aParam.mView, false, false, false, false, + 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, - bool aCancelable, nsGlobalWindowInner* aView, - bool aCtrlKey, bool aAltKey, - bool aShiftKey, bool aMetaKey, - uint32_t aKeyCode, uint32_t aCharCode) +KeyboardEvent::InitKeyEventJS(const nsAString& aType, bool aCanBubble, + bool aCancelable, nsGlobalWindowInner* aView, + bool aCtrlKey, bool aAltKey, + bool aShiftKey, bool aMetaKey, + 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, - bool aCanBubble, - bool aCancelable, - nsGlobalWindowInner* aView, - const nsAString& aKey, - uint32_t aLocation, - bool aCtrlKey, - bool aAltKey, - bool aShiftKey, - bool aMetaKey, - ErrorResult& aRv) +KeyboardEvent::InitKeyboardEventJS(const nsAString& aType, + bool aCanBubble, + bool aCancelable, + nsGlobalWindowInner* aView, + const nsAString& aKey, + uint32_t aLocation, + bool aCtrlKey, + bool aAltKey, + bool aShiftKey, + bool aMetaKey, + 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() || diff --git a/dom/events/KeyboardEvent.h b/dom/events/KeyboardEvent.h index f1e400c2dd8d..91eb67515965 100644 --- a/dom/events/KeyboardEvent.h +++ b/dom/events/KeyboardEvent.h @@ -29,7 +29,7 @@ public: return this; } - static already_AddRefed Constructor( + static already_AddRefed 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, - nsGlobalWindowInner* aView, bool aCtrlKey, bool aAltKey, - bool aShiftKey, bool aMetaKey, - uint32_t aKeyCode, uint32_t aCharCode); + void InitKeyEventJS(const nsAString& aType, bool aCanBubble, bool aCancelable, + nsGlobalWindowInner* aView, bool aCtrlKey, bool aAltKey, + bool aShiftKey, bool aMetaKey, + uint32_t aKeyCode, uint32_t aCharCode); - void InitKeyboardEvent(const nsAString& aType, - bool aCanBubble, bool aCancelable, - nsGlobalWindowInner* aView, const nsAString& aKey, - uint32_t aLocation, bool aCtrlKey, bool aAltKey, - bool aShiftKey, bool aMetaKey, ErrorResult& aRv); + void InitKeyboardEventJS(const nsAString& aType, + bool aCanBubble, bool aCancelable, + nsGlobalWindowInner* aView, const nsAString& aKey, + uint32_t aLocation, bool aCtrlKey, bool aAltKey, + 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 diff --git a/dom/events/test/test_dom_keyboard_event.html b/dom/events/test/test_dom_keyboard_event.html index c6133c033d1b..d2163073aebe 100644 --- a/dom/events/test/test_dom_keyboard_event.html +++ b/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 diff --git a/dom/webidl/KeyEvent.webidl b/dom/webidl/KeyEvent.webidl index 6fd66afc21d9..883187abb13d 100644 --- a/dom/webidl/KeyEvent.webidl +++ b/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, diff --git a/dom/webidl/KeyboardEvent.webidl b/dom/webidl/KeyboardEvent.webidl index 28e35c74eccf..3f6226622810 100644 --- a/dom/webidl/KeyboardEvent.webidl +++ b/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, diff --git a/modules/libpref/init/StaticPrefList.h b/modules/libpref/init/StaticPrefList.h index 68467baadd91..3256d41c4d3c 100644 --- a/modules/libpref/init/StaticPrefList.h +++ b/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(