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 draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 11 May 2016 16:47:38 +0900
changeset 366163 4c31267f1548a995a402f09ad2bfa041706881c4
parent 366162 11194cb04c74fa6667ab4dad83024eca05f888dc
child 366164 1fb42ec6f691760d3f8b04dbb3f8f64716d9855e
push id17910
push usermasayuki@d-toybox.com
push dateThu, 12 May 2016 03:08:45 +0000
reviewersm_kato
bugs1261880
milestone49.0a1
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 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: