Bug 1261880 - NativeKey should decide printable KeyboardEvent.key value of keydown and keypress events with following WM_CHAR message of WM_KEYDOWN. r=m_kato, a=sylvestre
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 11 May 2016 16:47:38 +0900
changeset 333134 7209d10232de4068271ff3230e78814586e9eae6
parent 333133 b168c418ec09549699fa5085b8b054ca40f96a98
child 333135 ed1c3aae460813241ce2c3480df30fae2268ab1f
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, sylvestre
bugs1261880
milestone48.0a2
Bug 1261880 - NativeKey should decide printable KeyboardEvent.key value of keydown and keypress events with following WM_CHAR message of WM_KEYDOWN. r=m_kato, a=sylvestre Some special keyboard layout may use a key as a non-lockable modifier key even if the key isn't a non-lockable modifier key (e.g., CapsLock key of 'Neo' for German). In such case, KeyboardLayout class cannot initialize NativeKey::mCommittedCharsAndModifiers with actual input character properly because KeyboardLayout class doesn't support such eccentric keyboard layouts. For preventing this issue, NativeKey should overwrite mCommittedCharsAndModifiers with following WM_CHAR message when it handles WM_KEYDOWN and should handle with following WM_CHAR message. However, we should ignore following WM_CHAR message if the character is a control character which shouldn't be inputted into focused text editor. MozReview-Commit-ID: Ax01nnaRXek
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -894,33 +894,34 @@ NativeKey::NativeKey(nsWindowBase* aWidg
 
   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.
+    } 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.
       // 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) {
+          !IsControlChar(static_cast<char16_t>(followingCharMsg.wParam))) {
+        mCommittedCharsAndModifiers.Clear();
         mCommittedCharsAndModifiers.Append(
           static_cast<char16_t>(followingCharMsg.wParam),
           mModKeyState.GetModifiers());
       }
     }
   }
 }
 
@@ -1024,16 +1025,23 @@ NativeKey::InitWithAppCommand()
     KeyboardLayout::GetInstance()->
       ConvertNativeKeyCodeToDOMKeyCode(mOriginalVirtualKeyCode);
   mCodeNameIndex =
     KeyboardLayout::ConvertScanCodeToCodeNameIndex(
       GetScanCodeWithExtendedFlag());
 }
 
 bool
+NativeKey::IsControlChar(char16_t aChar) const
+{
+  static const char16_t U_SPACE = 0x20;
+  return aChar < U_SPACE;
+}
+
+bool
 NativeKey::IsFollowedByDeadCharMessage() const
 {
   MSG nextMsg;
   if (mFakeCharMsgs) {
     nextMsg = mFakeCharMsgs->ElementAt(0).GetCharMsg(mMsg.hwnd);
   } else if (IsKeyMessageOnPlugin()) {
     return false;
   } else {
@@ -1667,26 +1675,25 @@ NativeKey::HandleCharMessage(const MSG& 
       mVirtualKeyCode == VK_RETURN) {
     return false;
   }
 
   // XXXmnakao I think that if aNativeKeyDown is null, such lonely WM_CHAR
   //           should cause composition events because they are not caused
   //           by actual keyboard operation.
 
-  static const char16_t U_SPACE = 0x20;
   static const char16_t U_EQUAL = 0x3D;
 
   // First, handle normal text input or non-printable key case here.
   if ((!mModKeyState.IsAlt() && !mModKeyState.IsControl()) ||
       mModKeyState.IsAltGr() ||
       (mOriginalVirtualKeyCode &&
        !KeyboardLayout::IsPrintableCharKey(mOriginalVirtualKeyCode))) {
     WidgetKeyboardEvent keypressEvent(true, eKeyPress, mWidget);
-    if (aCharMsg.wParam >= U_SPACE) {
+    if (!IsControlChar(static_cast<char16_t>(aCharMsg.wParam))) {
       keypressEvent.charCode = static_cast<uint32_t>(aCharMsg.wParam);
     } else {
       keypressEvent.keyCode = mDOMKeyCode;
     }
     nsresult rv = mDispatcher->BeginNativeInputTransaction();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return true;
     }
@@ -1710,26 +1717,27 @@ NativeKey::HandleCharMessage(const MSG& 
   // XXX It seems that following code was implemented for shortcut key
   //     handling.  However, it's now handled in WM_KEYDOWN message handler.
   //     So, this actually runs only when WM_CHAR is sent/posted without
   //     WM_KEYDOWN.  I think that we don't need to keypress event in such
   //     case especially for shortcut keys.
 
   char16_t uniChar;
   // Ctrl+A Ctrl+Z, see Programming Windows 3.1 page 110 for details
-  if (mModKeyState.IsControl() && aCharMsg.wParam <= 0x1A) {
+  if (mModKeyState.IsControl() &&
+      IsControlChar(static_cast<char16_t>(aCharMsg.wParam))) {
     // Bug 16486: Need to account for shift here.
     uniChar = aCharMsg.wParam - 1 + (mModKeyState.IsShift() ? 'A' : 'a');
   } else if (mModKeyState.IsControl() && aCharMsg.wParam <= 0x1F) {
     // Bug 50255: <ctrl><[> and <ctrl><]> are not being processed.
     // also fixes ctrl+\ (x1c), ctrl+^ (x1e) and ctrl+_ (x1f)
     // for some reason the keypress handler need to have the uniChar code set
     // with the addition of a upper case A not the lower case.
     uniChar = aCharMsg.wParam - 1 + 'A';
-  } else if (aCharMsg.wParam < U_SPACE ||
+  } else if (IsControlChar(static_cast<char16_t>(aCharMsg.wParam)) ||
              (aCharMsg.wParam == U_EQUAL && mModKeyState.IsControl())) {
     uniChar = 0;
   } else {
     uniChar = aCharMsg.wParam;
   }
 
   // Bug 50255 and Bug 351310: Keep the characters unshifted for shortcuts and
   // accesskeys and make sure that numbers are always passed as such.
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -311,16 +311,22 @@ private:
   NativeKey()
   {
     MOZ_CRASH("The default constructor of NativeKey isn't available");
   }
 
   void InitWithAppCommand();
 
   /**
+   * Returns true if aChar is a control character which shouldn't be inputted
+   * into focused text editor.
+   */
+  bool IsControlChar(char16_t aChar) const;
+
+  /**
    * Returns true if the key event is caused by auto repeat.
    */
   bool IsRepeat() const
   {
     switch (mMsg.message) {
       case WM_KEYDOWN:
       case WM_SYSKEYDOWN:
       case WM_CHAR: