Bug 1307112 part.7 Get rid of Enter and Backspace key hack in NativeKey class r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 04 Oct 2016 00:21:40 +0900
changeset 316881 c6dca012337bcb0a625335b3c4904ce6ae3bcf23
parent 316880 6d50e95e5494dd263c0e98ca2d04567e47ba86df
child 316882 b4b179a1d43e0d6288fee3ffa62ac8331ce11a76
push id32935
push usermasayuki@d-toybox.com
push dateFri, 07 Oct 2016 05:19:38 +0000
treeherderautoland@1de5e07bed66 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1307112
milestone52.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 1307112 part.7 Get rid of Enter and Backspace key hack in NativeKey class r=m_kato Currently, Enter and Backspace keys are handled without following WM_(SYS)CHAR message. However, older code needs hack for them for avoiding conflict with Ctrl+J vs. Ctrl+Enter, etc. So, we can get rid of them now. And when a keydown causes inputting a control character, NativeKey should handle it with keyboard layout (i.e., without following char message(s)). MozReview-Commit-ID: 6bVuK0BzFbx
widget/tests/test_keycodes.xul
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
--- a/widget/tests/test_keycodes.xul
+++ b/widget/tests/test_keycodes.xul
@@ -2090,28 +2090,35 @@ function* runKeyEventTests()
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_INSERT,
                    modifiers:{}, chars:""},
                   "Insert", "Insert", nsIDOMKeyEvent.DOM_VK_INSERT, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_DELETE,
                    modifiers:{}, chars:""},
                   "Delete", "Delete", nsIDOMKeyEvent.DOM_VK_DELETE, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
 
     // Backspace and Enter are handled with special path in mozilla::widget::NativeKey.  So, let's test them with modifiers too.
+    // Note that when both Ctrl and Alt are pressed, they don't cause WM_(SYS)CHAR message.
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_BACK,
                    modifiers:{ctrlKey:1}, chars:"\u007F"},
                   "Backspace", "Backspace", nsIDOMKeyEvent.DOM_VK_BACK_SPACE, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_BACK,
                    modifiers:{altKey:1}, chars:"\u0008"},
                   "Backspace", "Backspace", nsIDOMKeyEvent.DOM_VK_BACK_SPACE, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_BACK,
+                   modifiers:{ctrl:1, altKey:1}, chars:""},
+                  "Backspace", "Backspace", nsIDOMKeyEvent.DOM_VK_BACK_SPACE, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_RETURN,
                    modifiers:{ctrlKey:1}, chars:"\n"},
                   "Enter", "Enter", nsIDOMKeyEvent.DOM_VK_RETURN, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_RETURN,
                    modifiers:{altKey:1}, chars:"\r"},
                   "Enter", "Enter", nsIDOMKeyEvent.DOM_VK_RETURN, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_RETURN,
+                   modifiers:{ctrl:1, altKey:1}, chars:""},
+                  "Enter", "Enter", nsIDOMKeyEvent.DOM_VK_RETURN, "", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
 
     // US
     // Alphabet
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_A,
                    modifiers:{}, chars:"a"},
                   "a", "KeyA", nsIDOMKeyEvent.DOM_VK_A, "a", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_A,
                    modifiers:{shiftKey:1}, chars:"A"},
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -2520,25 +2520,16 @@ NativeKey::HandleCharMessage(const MSG& 
   // eKeyPress event for it and passes the message to next wndproc.
   if (IsReservedBySystem()) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
       ("%p   NativeKey::HandleCharMessage(), doesn't dispatch keypress "
        "event because the key combination is reserved by the system", this));
     return false;
   }
 
-  // Bug 818235: Ignore Ctrl+Enter.
-  if (!mModKeyState.IsAlt() && mModKeyState.IsControl() &&
-      mVirtualKeyCode == VK_RETURN) {
-    MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
-      ("%p   NativeKey::HandleCharMessage(), doesn't dispatch keypress "
-       "event due to Ctrl+Enter", this));
-    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_EQUAL = 0x3D;
 
   // First, handle normal text input or non-printable key case here.
   if ((!mModKeyState.IsAlt() && !mModKeyState.IsControl()) ||
@@ -2784,19 +2775,20 @@ NativeKey::NeedsToHandleWithoutFollowing
   }
 
   // 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) {
+  // If following char message is for a control character, it should be handled
+  // without WM_CHAR message.  This is typically Ctrl + [a-z].
+  if (mFollowingCharMsgs.Length() == 1 &&
+      IsControlCharMessage(mFollowingCharMsgs[0])) {
     return true;
   }
 
   // If inputting two or more characters, should be dispatched after removing
   // whole following char messages.
   if (mCommittedCharsAndModifiers.mLength > 1) {
     return true;
   }
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -472,16 +472,21 @@ private:
     return (mMsg.message == MOZ_WM_KEYDOWN ||
             mMsg.message == MOZ_WM_KEYUP);
   }
   bool IsPrintableCharMessage(const MSG& aMSG) const
   {
     return aMSG.message == WM_CHAR &&
            !IsControlChar(static_cast<char16_t>(aMSG.wParam));
   }
+  bool IsControlCharMessage(const MSG& aMSG) const
+  {
+    return IsCharMessage(aMSG.message) &&
+           IsControlChar(static_cast<char16_t>(aMSG.wParam));
+  }
 
   /**
    * IsReservedBySystem() returns true if the key combination is reserved by
    * the system.  Even if it's consumed by web apps, the message should be
    * sent to next wndproc.
    */
   bool IsReservedBySystem() const;