Bug 1347835 - NativeKey should dispatch keypress events even if WM_KEYDOWN is processed by IME but followed by printable WM_(SYS)CHAR messages. r=m_kato, a=gchang
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 10 Apr 2017 15:32:02 +0900
changeset 375943 bdce123aa2c2aeb4ccd3848a961fbfd97b6d0ec7
parent 375942 e984689093ed389415affc819776a6eede5d44d2
child 375944 b22fbd26927e239eaa7b1395d905bb5be9fd4e43
push id11063
push userryanvm@gmail.com
push dateMon, 17 Apr 2017 14:39:33 +0000
treeherdermozilla-aurora@d68c89ad6e51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, gchang
bugs1347835
milestone54.0a2
Bug 1347835 - NativeKey should dispatch keypress events even if WM_KEYDOWN is processed by IME but followed by printable WM_(SYS)CHAR messages. r=m_kato, a=gchang Some IME may handle WM_KEYDOWN message before application and may set the keycode value to VK_PROCSSKEY but not do actually. Similarly, IME may handle WM_KEYDOWN message and replace following WM_CHAR messages with different characters. Therefore, even if WM_KEYDOWN message comes with VK_PROCESSKEY, NativeKey shouldn't stop dispatching keypress events if it detects following printable char messages. MozReview-Commit-ID: DcC2qgcLDrQ
widget/tests/test_keycodes.xul
widget/windows/KeyboardLayout.cpp
--- a/widget/tests/test_keycodes.xul
+++ b/widget/tests/test_keycodes.xul
@@ -3015,16 +3015,26 @@ function* runKeyEventTests()
 
     // Even non-printable key could be mapped as a printable key.
     // Only "keyup" event cannot know if it *did* cause inputting text.
     // Therefore, only "keydown" and "keypress" event's key value should be the character inputted by the key.
     yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_F4,
                    modifiers:{}, chars:"a"},
                   ["a", "a", "F4"], "F4", nsIDOMKeyEvent.DOM_VK_F4, "a", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
 
+    // Even if key message is processed by IME, when the key causes inputting text,
+    // keypress event(s) should be fired.
+    const WIN_VK_PROCESSKEY_WITH_SC_A = WIN_VK_PROCESSKEY | 0x001E0000;
+    yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_PROCESSKEY_WITH_SC_A,
+                   modifiers:{}, chars:"a"},
+                  ["a", "a", "Unidentified" /* TODO: Process */], "KeyA", 0 /* TODO: 0xE5 */, "a", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
+    yield testKey({layout:KEYBOARD_LAYOUT_EN_US, keyCode:WIN_VK_PROCESSKEY_WITH_SC_A,
+                   modifiers:{altKey:1}, chars:"a"},
+                  ["a", "a", "Unidentified" /* TODO: Process */], "KeyA", 0 /* TODO: 0xE5 */, "a", 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", "KeyA", nsIDOMKeyEvent.DOM_VK_A, "A", SHOULD_DELIVER_ALL, KeyboardEvent.DOM_KEY_LOCATION_STANDARD);
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -2414,18 +2414,20 @@ NativeKey::HandleKeyDownMessage(bool* aE
       *aEventDispatched = true;
     }
   }
 
   RedirectedKeyDownMessageManager::Forget();
 
   MOZ_ASSERT(!mWidget->Destroyed());
 
-  // If the key was processed by IME, we shouldn't dispatch keypress event.
-  if (mOriginalVirtualKeyCode == VK_PROCESSKEY) {
+  // If the key was processed by IME and didn't cause WM_(SYS)CHAR messages, we
+  // shouldn't dispatch keypress event.
+  if (mOriginalVirtualKeyCode == VK_PROCESSKEY &&
+      !IsFollowedByPrintableCharOrSysCharMessage()) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
       ("%p   NativeKey::HandleKeyDownMessage(), not dispatching keypress "
        "event because the key was already handled by IME, defaultPrevented=%s",
        this, GetBoolName(defaultPrevented)));
     return defaultPrevented;
   }
 
   if (defaultPrevented) {