Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of WM_KEYDOWN of VK_PACKET with following char message r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 21 Apr 2016 14:42:09 +0900
changeset 354799 e7e7be19b68762d7925998e732ed4017ba484595
parent 354481 4feb4dd910a5a2d3061dbdd376a80975206819c6
child 519078 6868c4406e0722cc5ffc6f9685f20d2e63b81001
push id16161
push usermasayuki@d-toybox.com
push dateThu, 21 Apr 2016 14:57:24 +0000
reviewersm_kato
bugs1263389
milestone48.0a1
Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of WM_KEYDOWN of VK_PACKET with following char message r?m_kato TextEventDispatcher initializes charCode value with mKeyValue (and mKeyNameIndex) of WidgetKeyboardEvent. Therefore, NativeKey needs to initialize mKeyValue properly at handling WM_KEYDOWN message of VK_PACKET. However, nobody initializes mCommittedCharsAndModifiers value of VK_PACKET. Additionally, KeyboardLayout::ConvertNativeKeyCodeToKeyNameIndex() returns KEY_NAME_INDEX_Unidentified for it too. Therefore, this patch creates a path for handling VK_PACKET. First, makes KeyboardLayout::ConvertNativeKeyCodeToKeyNameIndex() returns KEY_NAME_INDEX_USE_STRING. Next, the constructor of NativeKey initializes mCommittedCharsAndModifiers with following char message. Additionally, makes sure that VK_PACKET is always handled with following char message even if there is no char message. MozReview-Commit-ID: G4eoN3jUm9n
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -689,39 +689,40 @@ NativeKey::NativeKey(nsWindowBase* aWidg
 
   mScanCode = WinUtils::GetScanCode(mMsg.lParam);
   mIsExtended = WinUtils::IsExtendedScanCode(mMsg.lParam);
   switch (mMsg.message) {
     case WM_KEYDOWN:
     case WM_SYSKEYDOWN:
     case WM_KEYUP:
     case WM_SYSKEYUP: {
-      // If the key message is sent from other application like a11y tools, the
-      // scancode value might not be set proper value.  Then, probably the value
-      // is 0.
-      // NOTE: If the virtual keycode can be caused by both non-extended key
-      //       and extended key, the API returns the non-extended key's
-      //       scancode.  E.g., VK_LEFT causes "4" key on numpad.
-      if (!mScanCode) {
-        uint16_t scanCodeEx = ComputeScanCodeExFromVirtualKeyCode(mMsg.wParam);
-        if (scanCodeEx) {
-          mScanCode = static_cast<uint8_t>(scanCodeEx & 0xFF);
-          uint8_t extended = static_cast<uint8_t>((scanCodeEx & 0xFF00) >> 8);
-          mIsExtended = (extended == 0xE0) || (extended == 0xE1);
-        }
-      }
       // 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);
       }
 
+      // If the key message is sent from other application like a11y tools, the
+      // scancode value might not be set proper value.  Then, probably the value
+      // is 0.
+      // NOTE: If the virtual keycode can be caused by both non-extended key
+      //       and extended key, the API returns the non-extended key's
+      //       scancode.  E.g., VK_LEFT causes "4" key on numpad.
+      if (!mScanCode && mOriginalVirtualKeyCode != VK_PACKET) {
+        uint16_t scanCodeEx = ComputeScanCodeExFromVirtualKeyCode(mMsg.wParam);
+        if (scanCodeEx) {
+          mScanCode = static_cast<uint8_t>(scanCodeEx & 0xFF);
+          uint8_t extended = static_cast<uint8_t>((scanCodeEx & 0xFF00) >> 8);
+          mIsExtended = (extended == 0xE0) || (extended == 0xE1);
+        }
+      }
+
       // Most keys are not distinguished as left or right keys.
       bool isLeftRightDistinguishedKey = false;
 
       // mOriginalVirtualKeyCode must not distinguish left or right of
       // Shift, Control or Alt.
       switch (mOriginalVirtualKeyCode) {
         case VK_SHIFT:
         case VK_CONTROL:
@@ -855,22 +856,45 @@ NativeKey::NativeKey(nsWindowBase* aWidg
 
   keyboardLayout->InitNativeKey(*this, mModKeyState);
 
   mIsDeadKey =
     (IsFollowedByDeadCharMessage() ||
      keyboardLayout->IsDeadKey(mOriginalVirtualKeyCode, mModKeyState));
   mIsPrintableKey = KeyboardLayout::IsPrintableCharKey(mOriginalVirtualKeyCode);
 
-  // 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 alternativeCharCodes in the callback
-  // method which will be called by TextEventDispatcher.
-  if (IsKeyDownMessage() && NeedsToHandleWithoutFollowingCharMessages()) {
-    ComputeInputtingStringWithKeyboardLayout();
+  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 alternativeCharCodes in the callback
+    // method which will be called by TextEventDispatcher.
+    if (NeedsToHandleWithoutFollowingCharMessages()) {
+      ComputeInputtingStringWithKeyboardLayout();
+    } else if (mVirtualKeyCode == VK_PACKET) {
+      // If this is sent by SendInput() API to input a Unicode character,
+      // we can only know what char will be inputted with following WM_CHAR
+      // message.
+      // 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) &&
+          followingCharMsg.wParam) {
+        mCommittedCharsAndModifiers.Append(
+          static_cast<char16_t>(followingCharMsg.wParam),
+          mModKeyState.GetModifiers());
+      }
+    }
   }
 }
 
 void
 NativeKey::InitWithAppCommand()
 {
   if (GET_DEVICE_LPARAM(mMsg.lParam) != FAPPCOMMAND_KEY) {
     return;
@@ -1544,16 +1568,22 @@ NativeKey::HandleKeyDownMessage(bool* aE
     // the message since no input should occur on the window.
     if (followingCharMsg.message == WM_NULL ||
         followingCharMsg.hwnd != mMsg.hwnd) {
       return false;
     }
     return DispatchKeyPressEventForFollowingCharMessage(followingCharMsg);
   }
 
+  // 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;
+  }
+
   if (!mModKeyState.IsControl() && !mModKeyState.IsAlt() &&
       !mModKeyState.IsWin() && mIsPrintableKey) {
     // If this is simple KeyDown event but next message is not WM_CHAR,
     // this event may not input text, so we should ignore this event.
     // See bug 314130.
     return false;
   }
 
@@ -1722,16 +1752,22 @@ NativeKey::HandleKeyUpMessage(bool* aEve
   return status == nsEventStatus_eConsumeNoDefault;
 }
 
 bool
 NativeKey::NeedsToHandleWithoutFollowingCharMessages() const
 {
   MOZ_ASSERT(IsKeyDownMessage());
 
+  // If the keydown message is generated for inputting some Unicode characters
+  // via SendInput() API, we need to handle it only with WM_*CHAR messages.
+  if (mVirtualKeyCode == VK_PACKET) {
+    return false;
+  }
+
   // Enter and backspace are always handled here to avoid for example the
   // confusion between ctrl-enter and ctrl-J.
   if (mDOMKeyCode == NS_VK_RETURN || mDOMKeyCode == NS_VK_BACK) {
     return true;
   }
 
   // If inputting two or more characters, should be dispatched after removing
   // whole following char messages.
@@ -1822,30 +1858,30 @@ 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) const
+NativeKey::GetFollowingCharMessage(MSG& aCharMsg, bool aRemove) const
 {
   MOZ_ASSERT(IsKeyDownMessage());
 
   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 = true;
+      fakeCharMsg.mConsumed = aRemove;
       if (!IsCharMessage(charMsg)) {
         return false;
       }
       aCharMsg = charMsg;
       return true;
     }
     return false;
   }
@@ -1857,16 +1893,21 @@ 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,
@@ -2440,16 +2481,18 @@ KeyboardLayout::InitNativeKey(NativeKey&
   int32_t virtualKeyIndex = GetKeyIndex(virtualKey);
 
   if (virtualKeyIndex < 0) {
     // Does not produce any printable characters, but still preserves the
     // dead-key state.
     return;
   }
 
+  MOZ_ASSERT(virtualKey != VK_PACKET,
+    "At handling VK_PACKET, we shouldn't refer keyboard layout");
   MOZ_ASSERT(aNativeKey.mKeyNameIndex == KEY_NAME_INDEX_USE_STRING,
     "Printable key's key name index must be KEY_NAME_INDEX_USE_STRING");
 
   bool isKeyDown = aNativeKey.IsKeyDownMessage();
   uint8_t shiftState =
     VirtualKey::ModifiersToShiftState(aModKeyState.GetModifiers());
 
   if (mVirtualKeys[virtualKeyIndex].IsDeadKey(shiftState)) {
@@ -3109,17 +3152,17 @@ KeyboardLayout::ConvertNativeKeyCodeToDO
   NS_WARNING(warning.get());
 #endif
   return 0;
 }
 
 KeyNameIndex
 KeyboardLayout::ConvertNativeKeyCodeToKeyNameIndex(uint8_t aVirtualKey) const
 {
-  if (IsPrintableCharKey(aVirtualKey)) {
+  if (IsPrintableCharKey(aVirtualKey) || aVirtualKey == VK_PACKET) {
     return KEY_NAME_INDEX_USE_STRING;
   }
 
   switch (aVirtualKey) {
 
 #undef NS_NATIVE_KEY_TO_DOM_KEY_NAME_INDEX
 #define NS_NATIVE_KEY_TO_DOM_KEY_NAME_INDEX(aNativeKey, aKeyNameIndex) \
     case aNativeKey: return aKeyNameIndex;
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -434,18 +434,21 @@ 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) const;
+  bool GetFollowingCharMessage(MSG& aCharMsg, bool aRemove = true) const;
 
   /**
    * Whether the key event can compute virtual keycode from the scancode value.
    */
   bool CanComputeVirtualKeyCodeFromScanCode() const;
 
   /**
    * Wraps MapVirtualKeyEx() with MAPVK_VSC_TO_VK.