Bug 1263389 NativeKey should initialize WidgetKeyboardEvent::mKeyValue of WM_KEYDOWN of VK_PACKET with following char message r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 25 Apr 2016 16:42:59 +0900
changeset 294699 14a02164b038bddd89b96c33b6932f069b8371ff
parent 294698 7aeac9da4265727deef801de5416f1f50c9a74dd
child 294700 6292f27863498e6367b679f37e8c710ab41bfff4
push id75654
push usermasayuki@d-toybox.com
push dateMon, 25 Apr 2016 07:43:10 +0000
treeherdermozilla-inbound@14a02164b038 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1263389
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.
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.