Bug 1303273 part.2 KeyboardLayout::InitNativeKey() should initialize aNativeKey.mCommittedCharsAndModifiers with following WM_CHAR or WM_SYSCHAR messages which are not providing a control character r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 07 Oct 2016 11:36:34 +0900
changeset 423626 3d6499049912f3213863ca3abb349b8e7676ff24
parent 423625 cc1008939023532a32ab8e9f87e8b3ee10701f34
child 423627 41edfc38e7fd91262f57bc0ff89149f38ce01567
push id31948
push usermasayuki@d-toybox.com
push dateTue, 11 Oct 2016 12:46:29 +0000
reviewersm_kato
bugs1303273
milestone52.0a1
Bug 1303273 part.2 KeyboardLayout::InitNativeKey() should initialize aNativeKey.mCommittedCharsAndModifiers with following WM_CHAR or WM_SYSCHAR messages which are not providing a control character r?m_kato First, mCommittedCharsAndModifiers should be initialized with following char messages because the messages could be different from the information of current keyboard layout. So, this patch guarantees that mCommittedCharsAndModifiers are same as the user expected when there is one or more WM_CHAR or WM_SYSCHAR messages. MozReview-Commit-ID: I5Ack0xccoL
widget/tests/test_keycodes.xul
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/tests/test_keycodes.xul
+++ b/widget/tests/test_keycodes.xul
@@ -3570,16 +3570,74 @@ function* runKeyEventTests()
                   ["\u00E4", "\u00E4", "a"], "KeyA", nsIDOMKeyEvent.DOM_VK_A, "\u00E4", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
 
     yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_7,
                    modifiers:{shiftKey:1}, chars:""},
                   "Dead", "Quote", 0, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
     yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_Q,
                    modifiers:{shiftKey:1}, chars:"\u00A8Q"},
                   ["\u00A8Q", "\u00A8", "Q", "Q"], "KeyQ", nsIDOMKeyEvent.DOM_VK_Q, "\u00A8Q", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    // When Alt key is pressed, dead key sequence is generated with WM_SYSKEYDOWN, WM_SYSDEADCHAR, WM_SYSCHAR and WM_SYSKEYUP.
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1}, chars:"``"},
+                  ["``", "`", "`", "`"], "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "``", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_A,
+                   modifiers:{altKey:1}, chars:"\u00E0"},
+                  ["\u00E0", "\u00E0", "a"], "KeyA", nsIDOMKeyEvent.DOM_VK_A, "\u00E0", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_A,
+                   modifiers:{altKey:1, shiftKey:1}, chars:"\u00C0"},
+                  ["\u00C0", "\u00C0", "A"], "KeyA", nsIDOMKeyEvent.DOM_VK_A, "\u00C0", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_Q,
+                   modifiers:{altKey:1}, chars:"`q"},
+                  ["`q", "`", "q", "q"], "KeyQ", nsIDOMKeyEvent.DOM_VK_Q, "`q", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1, shiftKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1, shiftKey:1}, chars:"^^"},
+                  ["^^", "^", "^", "^"], "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "^^", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1, shiftKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_A,
+                   modifiers:{altKey:1, shiftKey:1}, chars:"\u00C2"},
+                  ["\u00C2", "\u00C2", "A"], "KeyA", nsIDOMKeyEvent.DOM_VK_A, "\u00C2", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1, shiftKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_A,
+                   modifiers:{altKey:1}, chars:"\u00E2"},
+                  ["\u00E2", "\u00E2", "a"], "KeyA", nsIDOMKeyEvent.DOM_VK_A, "\u00E2", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_OEM_1,
+                   modifiers:{altKey:1, shiftKey:1}, chars:""},
+                  "Dead", "BracketLeft", nsIDOMKeyEvent.DOM_VK_BACK_QUOTE, "", SHOULD_DELIVER_KEYDOWN_KEYUP, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_SPANISH, keyCode:WIN_VK_Q,
+                   modifiers:{altKey:1, shiftKey:1}, chars:"^Q"},
+                  ["^Q", "^", "Q", "Q"], "KeyQ", nsIDOMKeyEvent.DOM_VK_Q, "^Q", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+
     cleanup();
   }
 
 
   if (IS_WIN) {
     yield* testKeysOnWindows();
   } else if (IS_MAC) {
     yield* testKeysOnMac();
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -946,16 +946,25 @@ UniCharsAndModifiers::UniCharsCaseInsens
   if (mLength != aOther.mLength) {
     return false;
   }
 
   nsCaseInsensitiveStringComparator comp;
   return !comp(mChars, aOther.mChars, mLength, aOther.mLength);
 }
 
+bool
+UniCharsAndModifiers::BeginsWith(const UniCharsAndModifiers& aOther) const
+{
+  if (mLength < aOther.mLength) {
+    return false;
+  }
+  return !memcmp(mChars, aOther.mChars, aOther.mLength * sizeof(char16_t));
+}
+
 UniCharsAndModifiers&
 UniCharsAndModifiers::operator+=(const UniCharsAndModifiers& aOther)
 {
   uint32_t copyCount = std::min(aOther.mLength, 5 - mLength);
   NS_ENSURE_TRUE(copyCount > 0, *this);
   memcpy(&mChars[mLength], aOther.mChars, copyCount * sizeof(char16_t));
   memcpy(&mModifiers[mLength], aOther.mModifiers,
          copyCount * sizeof(Modifiers));
@@ -1465,16 +1474,25 @@ NativeKey::InitWithKeyChar()
     KEY_NAME_INDEX_USE_STRING :
     keyboardLayout->ConvertNativeKeyCodeToKeyNameIndex(mOriginalVirtualKeyCode);
   mCodeNameIndex =
     KeyboardLayout::ConvertScanCodeToCodeNameIndex(
       GetScanCodeWithExtendedFlag());
 
   // If next message of WM_(SYS)KEYDOWN is WM_*CHAR message and the key
   // combination is not reserved by the system, let's consume it now.
+  // TODO: We cannot initialize mCommittedCharsAndModifiers for VK_PACKET
+  //       if the message is WM_KEYUP because we don't have preceding
+  //       WM_CHAR message.
+  // TODO: Like Edge, we shouldn't dispatch two sets of keyboard events
+  //       for a Unicode character in non-BMP because its key value looks
+  //       broken and not good thing for our editor if only one keydown or
+  //       keypress event's default is prevented.  I guess, we should store
+  //       key message information globally and we should wait following
+  //       WM_KEYDOWN if following WM_CHAR is a part of a Unicode character.
   if ((mMsg.message == WM_KEYDOWN || mMsg.message == WM_SYSKEYDOWN) &&
       !IsReservedBySystem()) {
     MSG charMsg;
     while (GetFollowingCharMessage(charMsg)) {
       // Although, got message shouldn't be WM_NULL in desktop apps,
       // we should keep checking this.  FYI: This was added for Metrofox.
       if (charMsg.message == WM_NULL) {
         continue;
@@ -1482,64 +1500,62 @@ NativeKey::InitWithKeyChar()
       MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
         ("%p   NativeKey::InitWithKeyChar(), removed char message, %s",
          this, ToString(charMsg).get()));
       NS_WARN_IF(charMsg.hwnd != mMsg.hwnd);
       mFollowingCharMsgs.AppendElement(charMsg);
     }
   }
 
-
   keyboardLayout->InitNativeKey(*this, mModKeyState);
 
   mIsDeadKey =
     (IsFollowedByDeadCharMessage() ||
      keyboardLayout->IsDeadKey(mOriginalVirtualKeyCode, mModKeyState));
   mIsPrintableKey =
     mKeyNameIndex == KEY_NAME_INDEX_USE_STRING ||
     KeyboardLayout::IsPrintableCharKey(mOriginalVirtualKeyCode);
 
   if (IsKeyDownMessage()) {
     // Compute some strings which may be inputted by the key with various
     // modifier state if this key event won't cause text input actually.
     // They will be used for setting mAlternativeCharCodes in the callback
     // method which will be called by TextEventDispatcher.
     if (NeedsToHandleWithoutFollowingCharMessages()) {
       ComputeInputtingStringWithKeyboardLayout();
-    } else {
-      // This message might be sent by SendInput() API to input a Unicode
-      // character, in such case, we can only know what chars will be inputted
-      // with following WM_CHAR messages.
-      // TODO: We cannot initialize mCommittedCharsAndModifiers for VK_PACKET
-      //       if the message is WM_KEYUP because we don't have preceding
-      //       WM_CHAR message.  Therefore, we should dispatch eKeyUp event at
-      //       handling WM_KEYDOWN.
-      // TODO: Like Edge, we shouldn't dispatch two sets of keyboard events
-      //       for a Unicode character in non-BMP because its key value looks
-      //       broken and not good thing for our editor if only one keydown or
-      //       keypress event's default is prevented.  I guess, we should store
-      //       key message information globally and we should wait following
-      //       WM_KEYDOWN if following WM_CHAR is a part of a Unicode character.
-      mCommittedCharsAndModifiers.Clear();
-      Modifiers modifiers =
-        mModKeyState.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
-      for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
-        // Ignore non-printable char messages.
-        if (!IsPrintableCharMessage(mFollowingCharMsgs[i])) {
-          continue;
-        }
-        char16_t ch = static_cast<char16_t>(mFollowingCharMsgs[i].wParam);
-        mCommittedCharsAndModifiers.Append(ch, modifiers);
-      }
     }
     // Remove odd char messages if there are.
     RemoveFollowingOddCharMessages();
   }
 }
 
+void
+NativeKey::InitCommittedCharsAndModifiersWithFollowingCharMessages(
+             const ModifierKeyState& aModKeyState)
+{
+  mCommittedCharsAndModifiers.Clear();
+  // This should cause inputting text in focused editor.  However, it
+  // ignores keypress events whose altKey or ctrlKey is true.
+  // Therefore, we need to remove these modifier state here.
+  Modifiers modifiers = aModKeyState.GetModifiers();
+  if (IsFollowedByPrintableCharMessage()) {
+    modifiers &= ~(MODIFIER_ALT | MODIFIER_CONTROL);
+  }
+  // NOTE: This method assumes that WM_CHAR and WM_SYSCHAR are never retrieved
+  //       at same time.
+  for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
+    // Ignore non-printable char messages.
+    if (!IsPrintableCharOrSysCharMessage(mFollowingCharMsgs[i])) {
+      continue;
+    }
+    char16_t ch = static_cast<char16_t>(mFollowingCharMsgs[i].wParam);
+    mCommittedCharsAndModifiers.Append(ch, modifiers);
+  }
+}
+
 NativeKey::~NativeKey()
 {
   MOZ_LOG(sNativeKeyLogger, LogLevel::Debug,
     ("%p   NativeKey::~NativeKey(), destroyed", this));
   if (mIsOverridingKeyboardLayout) {
     KeyboardLayout* keyboardLayout = KeyboardLayout::GetInstance();
     keyboardLayout->RestoreLayout();
   }
@@ -1667,16 +1683,27 @@ NativeKey::IsFollowedByPrintableCharMess
     if (IsPrintableCharMessage(mFollowingCharMsgs[i])) {
       return true;
     }
   }
   return false;
 }
 
 bool
+NativeKey::IsFollowedByPrintableCharOrSysCharMessage() const
+{
+  for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
+    if (IsPrintableCharOrSysCharMessage(mFollowingCharMsgs[i])) {
+      return true;
+    }
+  }
+  return false;
+}
+
+bool
 NativeKey::IsReservedBySystem() const
 {
   // Alt+Space key is handled by OS, we shouldn't touch it.
   if (mModKeyState.IsAlt() && !mModKeyState.IsControl() &&
       mVirtualKeyCode == VK_SPACE) {
     return true;
   }
 
@@ -3561,16 +3588,50 @@ KeyboardLayout::InitNativeKey(NativeKey&
       aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
       Modifiers modifiers =
         aModKeyState.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
       aNativeKey.mCommittedCharsAndModifiers.Append(ch, modifiers);
       return;
     }
   }
 
+  // When it's followed by non-dead char message(s) for printable character(s),
+  // aNativeKey should dispatch eKeyPress events for them rather than
+  // information from keyboard layout because respecting WM_(SYS)CHAR messages
+  // guarantees that we can always input characters which is expected by
+  // the user even if the user uses odd keyboard layout.
+  if (aNativeKey.IsFollowedByPrintableCharOrSysCharMessage()) {
+    MOZ_ASSERT(!aNativeKey.IsCharMessage(aNativeKey.mMsg));
+    // Initialize mCommittedCharsAndModifiers with following char messages.
+    aNativeKey.
+      InitCommittedCharsAndModifiersWithFollowingCharMessages(aModKeyState);
+    MOZ_ASSERT(!aNativeKey.mCommittedCharsAndModifiers.IsEmpty());
+    aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
+
+    // If it's not in dead key sequence, we don't need to do anymore here.
+    if (!IsInDeadKeySequence()) {
+      return;
+    }
+
+    // If it's in dead key sequence and dead char is inputted as is, we need to
+    // set the previous modifier state which is stored when preceding dead key
+    // is pressed.
+    UniCharsAndModifiers deadChars =
+      GetUniCharsAndModifiers(mActiveDeadKey, mDeadKeyShiftState);
+    if (aNativeKey.mCommittedCharsAndModifiers.BeginsWith(deadChars)) {
+      for (uint32_t i = 0; i < deadChars.mLength; ++i) {
+        aNativeKey.mCommittedCharsAndModifiers.mModifiers[i] =
+          deadChars.mModifiers[i];
+      }
+    }
+    // Finish the dead key sequence.
+    DeactivateDeadKeyState();
+    return;
+  }
+
   // If the key is not a usual printable key, KeyboardLayout class assume that
   // it's not cause dead char nor printable char.  Therefore, there are nothing
   // to do here fore such keys (e.g., function keys).
   // However, this should keep dead key state even if non-printable key is
   // pressed during a dead key sequence.
   if (!IsPrintableCharKey(aNativeKey.mOriginalVirtualKeyCode)) {
     return;
   }
@@ -3659,16 +3720,21 @@ KeyboardLayout::MaybeInitNativeKeyAsDead
   // another dead key before releasing current key.  Therefore, we can
   // set only a character for current key for keyup event.
   if (!IsInDeadKeySequence()) {
     aNativeKey.mCommittedCharsAndModifiers =
       GetUniCharsAndModifiers(aNativeKey.mOriginalVirtualKeyCode, aModKeyState);
     return true;
   }
 
+  // FYI: Following code may run when the user doesn't input text actually
+  //      but the key sequence is a dead key sequence.  For example,
+  //      ` -> Ctrl+` with Spanish keyboard layout.  Let's keep using this
+  //      complicated code for now because this runs really rarely.
+
   if (NS_WARN_IF(!IsPrintableCharKey(mActiveDeadKey))) {
 #if defined(DEBUG) || defined(MOZ_CRASHREPORTER)
     nsPrintfCString warning("The virtual key index (%d) of mActiveDeadKey "
                             "(0x%02X) is not a printable key "
                             "(aNativeKey.mOriginalVirtualKeyCode=0x%02X)",
                             GetKeyIndex(mActiveDeadKey), mActiveDeadKey,
                             aNativeKey.mOriginalVirtualKeyCode);
     NS_WARNING(warning.get());
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -74,16 +74,17 @@ struct UniCharsAndModifiers
   void Append(char16_t aUniChar, Modifiers aModifiers);
   void Clear() { mLength = 0; }
   bool IsEmpty() const { return !mLength; }
 
   void FillModifiers(Modifiers aModifiers);
 
   bool UniCharsEqual(const UniCharsAndModifiers& aOther) const;
   bool UniCharsCaseInsensitiveEqual(const UniCharsAndModifiers& aOther) const;
+  bool BeginsWith(const UniCharsAndModifiers& aOther) const;
 
   nsString ToString() const { return nsString(mChars, mLength); }
 };
 
 struct DeadKeyEntry;
 class DeadKeyTable;
 
 
@@ -364,16 +365,25 @@ private:
   {
     MOZ_CRASH("The default constructor of NativeKey isn't available");
   }
 
   void InitWithAppCommand();
   void InitWithKeyChar();
 
   /**
+   * InitCommittedCharsAndModifiersWithFollowingCharMessages() initializes
+   * mCommittedCharsAndModifiers with mFollowingCharMsgs and aModKeyState.
+   * If mFollowingCharMsgs includes non-printable char messages, they are
+   * ignored (skipped).
+   */
+  void InitCommittedCharsAndModifiersWithFollowingCharMessages(
+         const ModifierKeyState& aModKeyState);
+
+  /**
    * Returns true if the key event is caused by auto repeat.
    */
   bool IsRepeat() const
   {
     switch (mMsg.message) {
       case WM_KEYDOWN:
       case WM_SYSKEYDOWN:
       case WM_CHAR:
@@ -461,27 +471,33 @@ private:
     return IsSysCharMessage(aMSG.message);
   }
   bool IsSysCharMessage(UINT aMessage) const
   {
     return (aMessage == WM_SYSCHAR || aMessage == WM_SYSDEADCHAR);
   }
   bool MayBeSameCharMessage(const MSG& aCharMsg1, const MSG& aCharMsg2) const;
   bool IsFollowedByPrintableCharMessage() const;
+  bool IsFollowedByPrintableCharOrSysCharMessage() const;
   bool IsFollowedByDeadCharMessage() const;
   bool IsKeyMessageOnPlugin() const
   {
     return (mMsg.message == MOZ_WM_KEYDOWN ||
             mMsg.message == MOZ_WM_KEYUP);
   }
   bool IsPrintableCharMessage(const MSG& aMSG) const
   {
     return aMSG.message == WM_CHAR &&
            !IsControlChar(static_cast<char16_t>(aMSG.wParam));
   }
+  bool IsPrintableCharOrSysCharMessage(const MSG& aMSG) const
+  {
+    return IsCharOrSysCharMessage(aMSG) &&
+           !IsControlChar(static_cast<char16_t>(aMSG.wParam));
+  }
   bool IsControlCharMessage(const MSG& aMSG) const
   {
     return IsCharMessage(aMSG.message) &&
            IsControlChar(static_cast<char16_t>(aMSG.wParam));
   }
 
   /**
    * IsReservedBySystem() returns true if the key combination is reserved by