Bug 1294536 part.2 NativeKey should remove following char messages before dispatching a keydown event r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 22 Aug 2016 17:14:18 +0900
changeset 405259 917a12bfaaa915db4e8bcb9139c55e9b1bf5ca45
parent 405258 d971f53c919de9ea678e16b0675186b3f45ed941
child 529402 0c45d601fd3049d96d08853d504e75793b9703f0
push id27449
push usermasayuki@d-toybox.com
push dateThu, 25 Aug 2016 06:10:59 +0000
reviewersm_kato
bugs1294536
milestone51.0a1
Bug 1294536 part.2 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. Note that dispatching an eKeyDown or an eKeyPress causes focus window change, NativeKey stops handling stored char messages because firing renaming events in old window must not be expected. MozReview-Commit-ID: 6QNvlwVVlTz
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -679,16 +679,17 @@ uint8_t NativeKey::sDispatchedKeyOfAppCo
 NativeKey::NativeKey(nsWindowBase* aWidget,
                      const MSG& aMessage,
                      const ModifierKeyState& aModKeyState,
                      HKL aOverrideKeyboardLayout,
                      nsTArray<FakeCharMsg>* aFakeCharMsgs)
   : mWidget(aWidget)
   , mDispatcher(aWidget->GetTextEventDispatcher())
   , mMsg(aMessage)
+  , mFocusedWndBeforeDispatch(::GetFocus())
   , mDOMKeyCode(0)
   , mKeyNameIndex(KEY_NAME_INDEX_Unidentified)
   , mCodeNameIndex(CODE_NAME_INDEX_UNKNOWN)
   , mModKeyState(aModKeyState)
   , mVirtualKeyCode(0)
   , mOriginalVirtualKeyCode(0)
   , mShiftedLatinChar(0)
   , mUnshiftedLatinChar(0)
@@ -888,43 +889,49 @@ NativeKey::NativeKey(nsWindowBase* aWidg
   keyboardLayout->InitNativeKey(*this, mModKeyState);
 
   mIsDeadKey =
     (IsFollowedByDeadCharMessage() ||
      keyboardLayout->IsDeadKey(mOriginalVirtualKeyCode, mModKeyState));
   mIsPrintableKey = KeyboardLayout::IsPrintableCharKey(mOriginalVirtualKeyCode);
 
   if (IsKeyDownMessage()) {
+    // If next message is WM_*CHAR message, let's consume it now.
+    MSG charMsg;
+    while (GetFollowingCharMessage(charMsg)) {
+      mFollowingCharMsgs.AppendElement(charMsg);
+    }
     // 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 {
+    } else if (!mFollowingCharMsgs.IsEmpty()) {
       // 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) {
@@ -1616,26 +1623,35 @@ 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) {
+      // Even if there was char messages, 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 (mFollowingCharMsgs[i].message == WM_NULL ||
+          mFollowingCharMsgs[i].hwnd != mMsg.hwnd ||
+          mFocusedWndBeforeDispatch != ::GetFocus()) {
+        return consumed;
+      }
+      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;
   }
 
@@ -1926,31 +1942,31 @@ NativeKey::MayBeSameCharMessage(const MS
   static const LPARAM kScanCodeMask = 0x00FF0000;
   return
     aCharMsg1.message == aCharMsg2.message &&
     aCharMsg1.wParam == aCharMsg2.wParam &&
     (aCharMsg1.lParam & ~kScanCodeMask) == (aCharMsg2.lParam & ~kScanCodeMask);
 }
 
 bool
-NativeKey::GetFollowingCharMessage(MSG& aCharMsg, bool aRemove) const
+NativeKey::GetFollowingCharMessage(MSG& aCharMsg) const
 {
   MOZ_ASSERT(IsKeyDownMessage());
   MOZ_ASSERT(!IsKeyMessageOnPlugin());
 
   aCharMsg.message = WM_NULL;
 
   if (mFakeCharMsgs) {
     for (size_t i = 0; i < mFakeCharMsgs->Length(); i++) {
       FakeCharMsg& fakeCharMsg = mFakeCharMsgs->ElementAt(i);
       if (fakeCharMsg.mConsumed) {
         continue;
       }
       MSG charMsg = fakeCharMsg.GetCharMsg(mMsg.hwnd);
-      fakeCharMsg.mConsumed = aRemove;
+      fakeCharMsg.mConsumed = true;
       if (!IsCharMessage(charMsg)) {
         return false;
       }
       aCharMsg = charMsg;
       return true;
     }
     return false;
   }
@@ -1962,21 +1978,16 @@ NativeKey::GetFollowingCharMessage(MSG& 
   // to get char message for the handling keydown message.
   MSG nextKeyMsg;
   if (!WinUtils::PeekMessage(&nextKeyMsg, mMsg.hwnd, WM_KEYFIRST, WM_KEYLAST,
                              PM_NOREMOVE | PM_NOYIELD) ||
       !IsCharMessage(nextKeyMsg)) {
     return false;
   }
 
-  if (!aRemove) {
-    aCharMsg = nextKeyMsg;
-    return true;
-  }
-
   // On Metrofox, PeekMessage() sometimes returns WM_NULL even if we specify
   // the message range.  So, if it returns WM_NULL, we should retry to get
   // the following char message it was found above.
   for (uint32_t i = 0; i < 5; i++) {
     MSG removedMsg, nextKeyMsgInAllWindows;
     bool doCrash = false;
     if (!WinUtils::PeekMessage(&removedMsg, mMsg.hwnd,
                                nextKeyMsg.message, nextKeyMsg.message,
@@ -2152,38 +2163,39 @@ NativeKey::DispatchPluginEventsAndDiscar
   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) {
+  for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
+    if (mFollowingCharMsgs[i].message == WM_NULL) {
       continue;
     }
     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) {
+    if (mFollowingCharMsgs[i].hwnd != mMsg.hwnd ||
+        mFocusedWndBeforeDispatch != ::GetFocus()) {
       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
@@ -250,16 +250,18 @@ public:
    */
   static bool IsControlChar(char16_t aChar);
 
 private:
   RefPtr<nsWindowBase> mWidget;
   RefPtr<TextEventDispatcher> mDispatcher;
   HKL mKeyboardLayout;
   MSG mMsg;
+  nsTArray<MSG> mFollowingCharMsgs;
+  HWND mFocusedWndBeforeDispatch;
 
   uint32_t mDOMKeyCode;
   KeyNameIndex mKeyNameIndex;
   CodeNameIndex mCodeNameIndex;
 
   ModifierKeyState mModKeyState;
 
   // mVirtualKeyCode distinguishes left key or right key of modifier key.
@@ -421,21 +423,18 @@ private:
 
   /**
    * GetFollowingCharMessage() returns following char message of handling
    * keydown event.  If the message is found, this method returns true.
    * Otherwise, returns false.
    *
    * WARNING: Even if this returns true, aCharMsg may be WM_NULL or its
    *          hwnd may be different window.
-   *
-   * @param aRemove     true if the found message should be removed from the
-   *                    queue.  Otherwise, false.
    */
-  bool GetFollowingCharMessage(MSG& aCharMsg, bool aRemove = true) const;
+  bool GetFollowingCharMessage(MSG& aCharMsg) const;
 
   /**
    * Whether the key event can compute virtual keycode from the scancode value.
    */
   bool CanComputeVirtualKeyCodeFromScanCode() const;
 
   /**
    * Wraps MapVirtualKeyEx() with MAPVK_VSC_TO_VK.