Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of WM_KEYDOWN of VK_PACKET with following char message r=m_kato a=ritu
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 25 Apr 2016 16:42:59 +0900
changeset 332664 43fd203f3c5ff3b73971eb08c2fda54141e350a0
parent 332663 705d7c9e1021043be77aa1c8a15b3db291298882
child 332665 8ba304b4b4c6460871a0fa7a703023c1b1618dba
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, ritu
bugs1263389
milestone48.0a2
Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of WM_KEYDOWN of VK_PACKET with following char message r=m_kato a=ritu 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.
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -720,42 +720,40 @@ NativeKey::NativeKey(nsWindowBase* aWidg
   mIsExtended = WinUtils::IsExtendedScanCode(mMsg.lParam);
   switch (mMsg.message) {
     case WM_KEYDOWN:
     case WM_SYSKEYDOWN:
     case MOZ_WM_KEYDOWN:
     case WM_KEYUP:
     case WM_SYSKEYUP:
     case MOZ_WM_KEYUP: {
-      // 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.
-      // NOTE: Cannot compute scancode from keycode if the key message comes
-      //       from plugin process since active keyboard layout may be
-      //       different.
-      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:
@@ -889,22 +887,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());
+      }
+    }
   }
 }
 
 NativeKey::~NativeKey()
 {
   if (mIsOverridingKeyboardLayout) {
     KeyboardLayout* keyboardLayout = KeyboardLayout::GetInstance();
     keyboardLayout->RestoreLayout();
@@ -1598,16 +1619,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;
   }
 
@@ -1784,16 +1811,22 @@ NativeKey::NeedsToHandleWithoutFollowing
 
   // We cannot know following char messages of key messages in a plugin
   // process.  So, let's compute the character to be inputted with every
   // printable key should be computed with the keyboard layout.
   if (IsKeyMessageOnPlugin()) {
     return true;
   }
 
+  // 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.
@@ -1884,31 +1917,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) const
+NativeKey::GetFollowingCharMessage(MSG& aCharMsg, bool aRemove) 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 = true;
+      fakeCharMsg.mConsumed = aRemove;
       if (!IsCharMessage(charMsg)) {
         return false;
       }
       aCharMsg = charMsg;
       return true;
     }
     return false;
   }
@@ -1920,16 +1953,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,
@@ -2504,16 +2542,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)) {
@@ -3173,17 +3213,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
@@ -415,18 +415,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.