From 7bcaf602d0307e29a1d755ff9f358e9403639cdb Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Sat, 19 Mar 2016 20:57:11 +0900 Subject: [PATCH] Bug 1154183 part.2 eKeyDown event should have charCode value of following keypress event r=smaug MozReview-Commit-ID: 9duzKfCFPro --- dom/base/TextInputProcessor.cpp | 3 +- widget/TextEventDispatcher.cpp | 47 ++++++++++++++++++------------- widget/TextEvents.h | 25 ++++++++++++++++ widget/WidgetEventImpl.cpp | 19 ++++++------- widget/cocoa/TextInputHandler.mm | 30 +++++++++++--------- widget/gtk/nsGtkKeyUtils.cpp | 38 ++++--------------------- widget/nsGUIEventIPC.h | 2 ++ widget/windows/KeyboardLayout.cpp | 24 ++++------------ 8 files changed, 92 insertions(+), 96 deletions(-) diff --git a/dom/base/TextInputProcessor.cpp b/dom/base/TextInputProcessor.cpp index e62e1f7081d7..e6506839b7ce 100644 --- a/dom/base/TextInputProcessor.cpp +++ b/dom/base/TextInputProcessor.cpp @@ -712,7 +712,8 @@ TextInputProcessor::WillDispatchKeyboardEvent( uint32_t aIndexOfKeypress, void* aData) { - // TextInputProcessor doesn't set alternative char code. + // TextInputProcessor doesn't set alternative char code nor modify charCode + // even when Ctrl key is pressed. } nsresult diff --git a/widget/TextEventDispatcher.cpp b/widget/TextEventDispatcher.cpp index c6f0ddf99d2d..f358b6076b83 100644 --- a/widget/TextEventDispatcher.cpp +++ b/widget/TextEventDispatcher.cpp @@ -436,29 +436,34 @@ TextEventDispatcher::DispatchKeyboardEventInternal( } // Corrects each member for the specific key event type. - if (aMessage == eKeyDown || aMessage == eKeyUp) { + if (keyEvent.mKeyNameIndex != KEY_NAME_INDEX_USE_STRING) { MOZ_ASSERT(!aIndexOfKeypress, - "aIndexOfKeypress must be 0 for either eKeyDown or eKeyUp"); - // charCode of keydown and keyup should be 0. - keyEvent.charCode = 0; - } else if (keyEvent.mKeyNameIndex != KEY_NAME_INDEX_USE_STRING) { - MOZ_ASSERT(!aIndexOfKeypress, - "aIndexOfKeypress must be 0 for eKeyPress of non-printable key"); - // If keypress event isn't caused by printable key, its charCode should + "aIndexOfKeypress must be 0 for non-printable key"); + // If the keyboard event isn't caused by printable key, its charCode should // be 0. - keyEvent.charCode = 0; + keyEvent.SetCharCode(0); } else { - MOZ_RELEASE_ASSERT( - !aIndexOfKeypress || aIndexOfKeypress < keyEvent.mKeyValue.Length(), - "aIndexOfKeypress must be 0 - mKeyValue.Length() - 1"); - keyEvent.keyCode = 0; + if (aMessage == eKeyDown || aMessage == eKeyUp) { + MOZ_RELEASE_ASSERT(!aIndexOfKeypress, + "aIndexOfKeypress must be 0 for either eKeyDown or eKeyUp"); + } else { + MOZ_RELEASE_ASSERT( + !aIndexOfKeypress || aIndexOfKeypress < keyEvent.mKeyValue.Length(), + "aIndexOfKeypress must be 0 - mKeyValue.Length() - 1"); + } wchar_t ch = keyEvent.mKeyValue.IsEmpty() ? 0 : keyEvent.mKeyValue[aIndexOfKeypress]; - keyEvent.charCode = static_cast(ch); - if (ch) { - keyEvent.mKeyValue.Assign(ch); - } else { - keyEvent.mKeyValue.Truncate(); + keyEvent.SetCharCode(static_cast(ch)); + if (aMessage == eKeyPress) { + // keyCode of eKeyPress events of printable keys should be always 0. + keyEvent.keyCode = 0; + // eKeyPress events are dispatched for every character. + // So, each key value of eKeyPress events should be a character. + if (ch) { + keyEvent.mKeyValue.Assign(ch); + } else { + keyEvent.mKeyValue.Truncate(); + } } } if (aMessage == eKeyUp) { @@ -480,9 +485,11 @@ TextEventDispatcher::DispatchKeyboardEventInternal( // TODO: Manage mUniqueId here. // Request the alternative char codes for the key event. - // XXX Currently, they are necessary only when the event is eKeyPress. + // eKeyDown also needs alternative char codes because nsXBLWindowKeyHandler + // needs to check if a following keypress event is reserved by chrome for + // stopping propagation of its preceding keydown event. keyEvent.alternativeCharCodes.Clear(); - if (aMessage == eKeyPress && + if ((aMessage == eKeyDown || aMessage == eKeyPress) && (keyEvent.IsControl() || keyEvent.IsAlt() || keyEvent.IsMeta() || keyEvent.IsOS())) { nsCOMPtr listener = diff --git a/widget/TextEvents.h b/widget/TextEvents.h index 64465d7be416..69f47772bafd 100644 --- a/widget/TextEvents.h +++ b/widget/TextEvents.h @@ -104,6 +104,7 @@ protected: WidgetKeyboardEvent() : keyCode(0) , charCode(0) + , mPseudoCharCode(0) , location(nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD) , isChar(false) , mIsRepeat(false) @@ -128,6 +129,7 @@ public: : WidgetInputEvent(aIsTrusted, aMessage, aWidget, aEventClassID) , keyCode(0) , charCode(0) + , mPseudoCharCode(0) , location(nsIDOMKeyEvent::DOM_KEY_LOCATION_STANDARD) , isChar(false) , mIsRepeat(false) @@ -163,6 +165,10 @@ public: // character when some modifiers are active. Then, this value should be an // unmodified value except Shift and AltGr. uint32_t charCode; + // mPseudoCharCode is valid only when mMessage is an eKeyDown event. + // This stores charCode value of keypress event which is fired with same + // key value and same modifier state. + uint32_t mPseudoCharCode; // One of nsIDOMKeyEvent::DOM_KEY_LOCATION_* uint32_t location; // OS translated Unicode chars which are used for accesskey and accelkey @@ -209,6 +215,24 @@ public: // Otherwise, false. bool ShouldCauseKeypressEvents() const; + // charCode value of non-eKeyPress events is always 0. However, if + // non-eKeyPress event has one or more alternative char code values, + // its first item should be the charCode value of following eKeyPress event. + // PseudoCharCode() returns charCode value for eKeyPress event, + // the first alternative char code value of non-eKeyPress event or 0. + uint32_t PseudoCharCode() const + { + return mMessage == eKeyPress ? charCode : mPseudoCharCode; + } + void SetCharCode(uint32_t aCharCode) + { + if (mMessage == eKeyPress) { + charCode = aCharCode; + } else { + mPseudoCharCode = aCharCode; + } + } + void GetDOMKeyName(nsAString& aKeyName) { if (mKeyNameIndex == KEY_NAME_INDEX_USE_STRING) { @@ -292,6 +316,7 @@ public: keyCode = aEvent.keyCode; charCode = aEvent.charCode; + mPseudoCharCode = aEvent.mPseudoCharCode; location = aEvent.location; alternativeCharCodes = aEvent.alternativeCharCodes; isChar = aEvent.isChar; diff --git a/widget/WidgetEventImpl.cpp b/widget/WidgetEventImpl.cpp index e3ced7d8c0a1..e132e2e754ad 100644 --- a/widget/WidgetEventImpl.cpp +++ b/widget/WidgetEventImpl.cpp @@ -483,23 +483,20 @@ WidgetKeyboardEvent::GetShortcutKeyCandidates( { MOZ_ASSERT(aCandidates.IsEmpty(), "aCandidates must be empty"); - if (mMessage != eKeyPress) { - return; - } - // ShortcutKeyCandidate::mCharCode is a candidate charCode. // ShortcutKeyCandidate::mIgnoreShift means the mCharCode should be tried to // execute a command with/without shift key state. If this is TRUE, the // shifted key state should be ignored. Otherwise, don't ignore the state. // the priority of the charCodes are (shift key is not pressed): - // 0: charCode/false, + // 0: PseudoCharCode()/false, // 1: unshiftedCharCodes[0]/false, 2: unshiftedCharCodes[1]/false... // the priority of the charCodes are (shift key is pressed): - // 0: charCode/false, + // 0: PseudoCharCode()/false, // 1: shiftedCharCodes[0]/false, 2: shiftedCharCodes[0]/true, // 3: shiftedCharCodes[1]/false, 4: shiftedCharCodes[1]/true... - if (charCode) { - ShortcutKeyCandidate key(charCode, false); + uint32_t pseudoCharCode = PseudoCharCode(); + if (pseudoCharCode) { + ShortcutKeyCandidate key(pseudoCharCode, false); aCandidates.AppendElement(key); } @@ -507,7 +504,7 @@ WidgetKeyboardEvent::GetShortcutKeyCandidates( if (!IsShift()) { for (uint32_t i = 0; i < len; ++i) { uint32_t ch = alternativeCharCodes[i].mUnshiftedCharCode; - if (!ch || ch == charCode) { + if (!ch || ch == pseudoCharCode) { continue; } ShortcutKeyCandidate key(ch, false); @@ -534,7 +531,7 @@ WidgetKeyboardEvent::GetShortcutKeyCandidates( continue; } - if (ch != charCode) { + if (ch != pseudoCharCode) { ShortcutKeyCandidate key(ch, false); aCandidates.AppendElement(key); } @@ -568,7 +565,7 @@ WidgetKeyboardEvent::GetShortcutKeyCandidates( // we should guarantee that the key press works as an ASCII white space key // press. if (mCodeNameIndex == CODE_NAME_INDEX_Space && - charCode != static_cast(' ')) { + pseudoCharCode != static_cast(' ')) { ShortcutKeyCandidate spaceKey(static_cast(' '), false); aCandidates.AppendElement(spaceKey); } diff --git a/widget/cocoa/TextInputHandler.mm b/widget/cocoa/TextInputHandler.mm index 5f01bfc98ce0..5ad6fff9bec9 100644 --- a/widget/cocoa/TextInputHandler.mm +++ b/widget/cocoa/TextInputHandler.mm @@ -1058,6 +1058,14 @@ TISInputSourceWrapper::WillDispatchKeyboardEvent( { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + // Nothing to do here if the native key event is neither NSKeyDown nor + // NSKeyUp because accessing [aNativeKeyEvent characters] causes throwing + // an exception. + if ([aNativeKeyEvent type] != NSKeyDown && + [aNativeKeyEvent type] != NSKeyUp) { + return; + } + UInt32 kbType = GetKbdType(); if (MOZ_LOG_TEST(gLog, LogLevel::Info)) { @@ -1079,22 +1087,16 @@ TISInputSourceWrapper::WillDispatchKeyboardEvent( nsAutoString insertStringForCharCode; ComputeInsertStringForCharCode(aNativeKeyEvent, aKeyEvent, aInsertString, insertStringForCharCode); + + // The charCode was set from mKeyValue. However, for example, when Ctrl key + // is pressed, its value should indicate an ASCII character for backward + // compatibility rather than inputting character without the modifiers. + // Therefore, we need to modify charCode value here. uint32_t charCode = insertStringForCharCode.IsEmpty() ? 0 : insertStringForCharCode[0]; - if (aKeyEvent.mMessage == eKeyPress) { - aKeyEvent.charCode = charCode; - aKeyEvent.isChar = true; // this is not a special key XXX not used in XP - } else if (charCode) { - // If it's not a keypress event, we need to set alternative char code - // to charCode value for shortcut key event handlers. - AlternativeCharCode altCharCodes(0, 0); - if (!aKeyEvent.IsShift()) { - altCharCodes.mUnshiftedCharCode = charCode; - } else { - altCharCodes.mShiftedCharCode = charCode; - } - aKeyEvent.alternativeCharCodes.AppendElement(altCharCodes); - } + aKeyEvent.SetCharCode(charCode); + // this is not a special key XXX not used in XP + aKeyEvent.isChar = (aKeyEvent.mMessage == eKeyPress); MOZ_LOG(gLog, LogLevel::Info, ("%p TISInputSourceWrapper::WillDispatchKeyboardEvent, " diff --git a/widget/gtk/nsGtkKeyUtils.cpp b/widget/gtk/nsGtkKeyUtils.cpp index dfad4d71d93f..4bd0042e7d97 100644 --- a/widget/gtk/nsGtkKeyUtils.cpp +++ b/widget/gtk/nsGtkKeyUtils.cpp @@ -1341,28 +1341,11 @@ KeymapWrapper::WillDispatchKeyboardEventInternal(WidgetKeyboardEvent& aKeyEvent, return; } - AlternativeCharCode* firstAltCharCodes = nullptr; - if (aKeyEvent.mMessage == eKeyPress) { - // charCode of aKeyEvent is set from mKeyValue. However, for backward - // compatibility, we may need to set it to other value, e.g., when - // Ctrl key is pressed. Therefore, we need to overwrite the charCode - // here. - aKeyEvent.charCode = charCode; - MOZ_ASSERT(!aKeyEvent.keyCode); - } else { - MOZ_ASSERT(charCode); - // If it's not a keypress event, we need to set alternative char code - // to charCode value for shortcut key event handlers. - AlternativeCharCode altCharCodes(0, 0); - if (!aKeyEvent.IsShift()) { - altCharCodes.mUnshiftedCharCode = charCode; - } else { - altCharCodes.mShiftedCharCode = charCode; - } - MOZ_ASSERT(aKeyEvent.alternativeCharCodes.IsEmpty()); - firstAltCharCodes = - aKeyEvent.alternativeCharCodes.AppendElement(altCharCodes); - } + // The charCode was set from mKeyValue. However, for example, when Ctrl key + // is pressed, its value should indicate an ASCII character for backward + // compatibility rather than inputting character without the modifiers. + // Therefore, we need to modify charCode value here. + aKeyEvent.SetCharCode(charCode); gint level = GetKeyLevel(aGdkKeyEvent); if (level != 0 && level != 1) { @@ -1456,16 +1439,7 @@ KeymapWrapper::WillDispatchKeyboardEventInternal(WidgetKeyboardEvent& aKeyEvent, altLatinCharCodes.mUnshiftedCharCode; if (ch && !(aKeyEvent.IsAlt() || aKeyEvent.IsMeta()) && charCode == unmodifiedCh) { - if (aKeyEvent.mMessage == eKeyPress) { - aKeyEvent.charCode = ch; - } else { - MOZ_RELEASE_ASSERT(firstAltCharCodes); - if (!aKeyEvent.IsShift()) { - firstAltCharCodes->mUnshiftedCharCode = ch; - } else { - firstAltCharCodes->mShiftedCharCode = ch; - } - } + aKeyEvent.SetCharCode(ch); } MOZ_LOG(gKeymapWrapperLog, LogLevel::Info, diff --git a/widget/nsGUIEventIPC.h b/widget/nsGUIEventIPC.h index 964625feabd9..8fd62c4070ef 100644 --- a/widget/nsGUIEventIPC.h +++ b/widget/nsGUIEventIPC.h @@ -378,6 +378,7 @@ struct ParamTraits WriteParam(aMsg, aParam.mCodeValue); WriteParam(aMsg, aParam.keyCode); WriteParam(aMsg, aParam.charCode); + WriteParam(aMsg, aParam.mPseudoCharCode); WriteParam(aMsg, aParam.alternativeCharCodes); WriteParam(aMsg, aParam.isChar); WriteParam(aMsg, aParam.mIsRepeat); @@ -405,6 +406,7 @@ struct ParamTraits ReadParam(aMsg, aIter, &aResult->mCodeValue) && ReadParam(aMsg, aIter, &aResult->keyCode) && ReadParam(aMsg, aIter, &aResult->charCode) && + ReadParam(aMsg, aIter, &aResult->mPseudoCharCode) && ReadParam(aMsg, aIter, &aResult->alternativeCharCodes) && ReadParam(aMsg, aIter, &aResult->isChar) && ReadParam(aMsg, aIter, &aResult->mIsRepeat) && diff --git a/widget/windows/KeyboardLayout.cpp b/widget/windows/KeyboardLayout.cpp index f6a81fea1ad7..a675ca92f974 100644 --- a/widget/windows/KeyboardLayout.cpp +++ b/widget/windows/KeyboardLayout.cpp @@ -2227,24 +2227,12 @@ NativeKey::WillDispatchKeyboardEvent(WidgetKeyboardEvent& aKeyboardEvent, } uint16_t uniChar = mInputtingStringAndModifiers.mChars[aIndex - skipUniChars]; - if (aKeyboardEvent.mMessage == eKeyPress) { - // charCode is set from mKeyValue but e.g., when Ctrl key is pressed, - // the value should indicate an ASCII character for backward - // compatibility instead of inputting character without the modifiers. - aKeyboardEvent.charCode = uniChar; - MOZ_ASSERT(!aKeyboardEvent.keyCode); - } else if (uniChar) { - // If the event is not keypress event, we should set charCode as - // first alternative char code since the char code value is necessary - // for shortcut key handlers at looking for a proper handler. - AlternativeCharCode chars(0, 0); - if (!aKeyboardEvent.IsShift()) { - chars.mUnshiftedCharCode = uniChar; - } else { - chars.mShiftedCharCode = uniChar; - } - altArray.AppendElement(chars); - } + + // The charCode was set from mKeyValue. However, for example, when Ctrl key + // is pressed, its value should indicate an ASCII character for backward + // compatibility rather than inputting character without the modifiers. + // Therefore, we need to modify charCode value here. + aKeyboardEvent.SetCharCode(uniChar); } if (skipShiftedChars <= aIndex) {