Bug 1154183 part.2 eKeyDown event should have charCode value of following keypress event r=smaug

MozReview-Commit-ID: 9duzKfCFPro

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

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<uint32_t>(ch);
-    if (ch) {
-      keyEvent.mKeyValue.Assign(ch);
-    } else {
-      keyEvent.mKeyValue.Truncate();
+    keyEvent.SetCharCode(static_cast<uint32_t>(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<TextEventDispatcherListener> listener =

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;

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<uint32_t>(' ')) {
+      pseudoCharCode != static_cast<uint32_t>(' ')) {
     ShortcutKeyCandidate spaceKey(static_cast<uint32_t>(' '), false);
     aCandidates.AppendElement(spaceKey);
   }

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, "

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,

widget/nsGUIEventIPC.h

@@ -378,6 +378,7 @@ struct ParamTraits<mozilla::WidgetKeyboardEvent>
     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<mozilla::WidgetKeyboardEvent>
         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) &&

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) {