Bug 1300003 part.1 NativeKey should remove following char messages before dispatching a keydown event r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 15 Sep 2016 00:02:15 +0900
changeset 413987 584208e811a63590ff6fe890fc6c9cc27ec3021b
parent 413986 320d28e93aab1cfca2218c7b4d3c3aed3917d403
child 413988 ee33f37eec3dfa7acdf6f63b52916c18e1eef7ab
push id29569
push usermasayuki@d-toybox.com
push dateThu, 15 Sep 2016 12:25:58 +0000
reviewersm_kato
bugs1300003
milestone51.0a1
Bug 1300003 part.1 NativeKey should remove following char messages before dispatching a keydown event r?m_kato In some cases, we may receive following char message of WM_*KEYDOWN *during* dispatching an eKeyDown event. In such case, NativeKey shouldn't dispatch eKeyPress event twice (one is caused by receiving WM_*CHAR message, the other is caused by post dispatching eKeyDown event code). This patch makes NativeKey consume following WM_*CHAR messages before dispatching eKeyDown. Therefore, the former case will never occur. For doing that, NativeKey stores following WM_*CHAR messages in mFollowingCharMsgs and uses it when it needs to dispatch eKeyPress events for each WM_*CHAR message. MozReview-Commit-ID: 6QNvlwVVlTz
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -881,20 +881,33 @@ NativeKey::NativeKey(nsWindowBase* aWidg
     InitWithAppCommand();
     return;
   }
 
   mScanCode = WinUtils::GetScanCode(mMsg.lParam);
   mIsExtended = WinUtils::IsExtendedScanCode(mMsg.lParam);
   switch (mMsg.message) {
     case WM_KEYDOWN:
-    case WM_SYSKEYDOWN:
-    case MOZ_WM_KEYDOWN:
+    case WM_SYSKEYDOWN: {
+      // If next message is WM_*CHAR message, let's consume it now.
+      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;
+        }
+        NS_WARN_IF(charMsg.hwnd != mMsg.hwnd);
+        mFollowingCharMsgs.AppendElement(charMsg);
+      }
+      MOZ_FALLTHROUGH;
+    }
     case WM_KEYUP:
     case WM_SYSKEYUP:
+    case MOZ_WM_KEYDOWN:
     case MOZ_WM_KEYUP: {
       // First, resolve the IME converted virtual keycode to its original
       // keycode.
       if (mMsg.wParam == VK_PROCESSKEY) {
         mOriginalVirtualKeyCode =
           static_cast<uint8_t>(::ImmGetVirtualKey(mMsg.hwnd));
       } else {
         mOriginalVirtualKeyCode = static_cast<uint8_t>(mMsg.wParam);
@@ -1069,35 +1082,36 @@ NativeKey::NativeKey(nsWindowBase* aWidg
     // 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 char will be inputted
-      // with following WM_CHAR message.
+      // 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.
-      MSG followingCharMsg;
-      if (GetFollowingCharMessage(followingCharMsg, false) &&
-          !IsControlChar(static_cast<char16_t>(followingCharMsg.wParam))) {
-        mCommittedCharsAndModifiers.Clear();
-        mCommittedCharsAndModifiers.Append(
-          static_cast<char16_t>(followingCharMsg.wParam),
-          mModKeyState.GetModifiers());
+      mCommittedCharsAndModifiers.Clear();
+      for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
+        char16_t ch = static_cast<char16_t>(mFollowingCharMsgs[i].wParam);
+        // Skip control characters.
+        if (IsControlChar(ch)) {
+          continue;
+        }
+        mCommittedCharsAndModifiers.Append(ch, mModKeyState.GetModifiers());
       }
     }
   }
 }
 
 NativeKey::~NativeKey()
 {
   if (mIsOverridingKeyboardLayout) {
@@ -1209,46 +1223,30 @@ NativeKey::IsControlChar(char16_t aChar)
   static const char16_t U_SPACE = 0x20;
   static const char16_t U_DELETE = 0x7F;
   return aChar < U_SPACE || aChar == U_DELETE;
 }
 
 bool
 NativeKey::IsFollowedByDeadCharMessage() const
 {
-  MSG nextMsg;
-  if (mFakeCharMsgs) {
-    nextMsg = mFakeCharMsgs->ElementAt(0).GetCharMsg(mMsg.hwnd);
-  } else if (IsKeyMessageOnPlugin()) {
+  if (mFollowingCharMsgs.IsEmpty()) {
     return false;
-  } else {
-    if (!WinUtils::PeekMessage(&nextMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
-                               PM_NOREMOVE | PM_NOYIELD)) {
-      return false;
-    }
   }
-  return IsDeadCharMessage(nextMsg);
+  return IsDeadCharMessage(mFollowingCharMsgs[0]);
 }
 
 bool
 NativeKey::IsFollowedByNonControlCharMessage() const
 {
-  MSG nextMsg;
-  if (mFakeCharMsgs) {
-    nextMsg = mFakeCharMsgs->ElementAt(0).GetCharMsg(mMsg.hwnd);
-  } else if (IsKeyMessageOnPlugin()) {
+  if (mFollowingCharMsgs.IsEmpty()) {
     return false;
-  } else {
-    if (!WinUtils::PeekMessage(&nextMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
-                               PM_NOREMOVE | PM_NOYIELD)) {
-      return false;
-    }
   }
-  return nextMsg.message == WM_CHAR &&
-         !IsControlChar(static_cast<char16_t>(nextMsg.wParam));
+  return mFollowingCharMsgs[0].message == WM_CHAR &&
+         !IsControlChar(static_cast<char16_t>(mFollowingCharMsgs[0].wParam));
 }
 
 bool
 NativeKey::IsIMEDoingKakuteiUndo() const
 {
   // Following message pattern is caused by "Kakutei-Undo" of ATOK or WXG:
   // ---------------------------------------------------------------------------
   // WM_KEYDOWN              * n (wParam = VK_BACK, lParam = 0x1)
@@ -1808,26 +1806,27 @@ NativeKey::HandleKeyDownMessage(bool* aE
 
   // If we won't be getting a WM_CHAR, WM_SYSCHAR or WM_DEADCHAR, synthesize a
   // keypress for almost all keys
   if (NeedsToHandleWithoutFollowingCharMessages()) {
     return (DispatchPluginEventsAndDiscardsCharMessages() ||
             DispatchKeyPressEventsWithoutCharMessage());
   }
 
-  MSG followingCharMsg;
-  if (GetFollowingCharMessage(followingCharMsg)) {
-    // Even if there was char message, it might be redirected by different
-    // window (perhaps, focus move?).  Then, we shouldn't continue to handle
-    // the message since no input should occur on the window.
-    if (followingCharMsg.message == WM_NULL ||
-        followingCharMsg.hwnd != mMsg.hwnd) {
-      return false;
+  if (!mFollowingCharMsgs.IsEmpty()) {
+    bool consumed = false;
+    for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
+      consumed =
+        DispatchKeyPressEventForFollowingCharMessage(mFollowingCharMsgs[i]) ||
+        consumed;
+      if (mWidget->Destroyed()) {
+        return true;
+      }
     }
-    return DispatchKeyPressEventForFollowingCharMessage(followingCharMsg);
+    return consumed;
   }
 
   // If WM_KEYDOWN of VK_PACKET isn't followed by WM_CHAR, we don't need to
   // dispatch keypress events.
   if (mVirtualKeyCode == VK_PACKET) {
     return false;
   }
 
@@ -2340,49 +2339,38 @@ NativeKey::GetFollowingCharMessage(MSG& 
                        nextKeyMsg.message, nextKeyMsg.wParam,
                        nextKeyMsg.lParam);
   CrashReporter::AppendAppNotesToCrashReport(info);
 #endif // #ifdef MOZ_CRASHREPORTER
   MOZ_CRASH("We lost the following char message");
   return false;
 }
 
+// TODO: Rename this method later.
 bool
 NativeKey::DispatchPluginEventsAndDiscardsCharMessages() const
 {
   MOZ_ASSERT(IsKeyDownMessage());
   MOZ_ASSERT(!IsKeyMessageOnPlugin());
 
-  // Remove a possible WM_CHAR or WM_SYSCHAR messages from the message queue.
-  // They can be more than one because of:
-  //  * Dead-keys not pairing with base character
-  //  * Some keyboard layouts may map up to 4 characters to the single key
   bool anyCharMessagesRemoved = false;
-  MSG msg;
-  while (GetFollowingCharMessage(msg)) {
-    if (msg.message == WM_NULL) {
-      continue;
-    }
+  for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
     anyCharMessagesRemoved = true;
-    // If the window handle is changed, focused window must be changed.
-    // So, plugin shouldn't handle it anymore.
-    if (msg.hwnd != mMsg.hwnd) {
-      break;
-    }
     MOZ_RELEASE_ASSERT(!mWidget->Destroyed(),
       "NativeKey tries to dispatch a plugin event on destroyed widget");
-    mWidget->DispatchPluginEvent(msg);
+    mWidget->DispatchPluginEvent(mFollowingCharMsgs[i]);
     if (mWidget->Destroyed()) {
       return true;
     }
   }
 
   if (!mFakeCharMsgs && !anyCharMessagesRemoved &&
       mDOMKeyCode == NS_VK_BACK && IsIMEDoingKakuteiUndo()) {
     // This is for a hack for ATOK and WXG.  So, PeekMessage() must scceed!
+    MSG msg;
     while (WinUtils::PeekMessage(&msg, mMsg.hwnd, WM_CHAR, WM_CHAR,
                                  PM_REMOVE | PM_NOYIELD)) {
       if (msg.message != WM_CHAR) {
         MOZ_RELEASE_ASSERT(msg.message == WM_NULL,
                            "Unexpected message was removed");
         continue;
       }
       MOZ_RELEASE_ASSERT(!mWidget->Destroyed(),
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -258,16 +258,20 @@ public:
    */
   static bool IsControlChar(char16_t aChar);
 
 private:
   RefPtr<nsWindowBase> mWidget;
   RefPtr<TextEventDispatcher> mDispatcher;
   HKL mKeyboardLayout;
   MSG mMsg;
+  // mFollowingCharMsgs stores WM_CHAR, WM_SYSCHAR, WM_DEADCHAR or
+  // WM_SYSDEADCHAR message which follows WM_KEYDOWN.
+  // Note that the stored messaged are already removed from the queue.
+  nsTArray<MSG> mFollowingCharMsgs;
 
   uint32_t mDOMKeyCode;
   KeyNameIndex mKeyNameIndex;
   CodeNameIndex mCodeNameIndex;
 
   ModifierKeyState mModKeyState;
 
   // mVirtualKeyCode distinguishes left key or right key of modifier key.