Bug 1307112 part.10 Clean up NativeKey::HandleCharMessage() r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 04 Oct 2016 23:01:14 +0900
changeset 316884 1de5e07bed6601d2647e88ed2690b3f30418d8aa
parent 316883 99042be37c7c777ae1ef75c8e044bf4053bec8bc
child 316885 ff08c7cfb55df8cb1850adcf9c92430713086b2e
child 316950 9c11fedf43675388a547f09cc4a64e78739f5220
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.10 Clean up NativeKey::HandleCharMessage() r=m_kato Now, NativeKey::HandleCharMessage() has almost same code, one is for dispatching eKeyPress event for non-printable keys or printable keys when one of Alt or Ctrl key is pressed, the other is for printable keys when Alt or Ctrl key is pressed. The difference of them is, the former block removes Alt state and Ctrl state for handling AltGr key. When AltGr key is pressed, both Alt and Ctrl state are true. However, EditorBase treas keypress events whose altKey or ctrlKey is true as non-printable key event. Therefore, we need to set these modifier state to false when AltGr key is pressed and the key causes some text. Note that as far as we know, when a key press with AltGr doesn't cause any characters, WM_CHAR isn't generated. Therefore, we don't need to check with complicated logic if the key event is actually inputting a character. MozReview-Commit-ID: BRNWfICvkSm
widget/windows/KeyboardLayout.cpp
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -1514,23 +1514,25 @@ NativeKey::InitWithKeyChar()
       //       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.
       mCommittedCharsAndModifiers.Clear();
+      Modifiers modifiers =
+        mModKeyState.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
       for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
         // Ignore non-printable char messages.
         if (!IsPrintableCharMessage(mFollowingCharMsgs[i])) {
           continue;
         }
         char16_t ch = static_cast<char16_t>(mFollowingCharMsgs[i].wParam);
-        mCommittedCharsAndModifiers.Append(ch, mModKeyState.GetModifiers());
+        mCommittedCharsAndModifiers.Append(ch, modifiers);
       }
     }
     // Remove odd char messages if there are.
     RemoveFollowingOddCharMessages();
   }
 }
 
 NativeKey::~NativeKey()
@@ -2549,100 +2551,58 @@ NativeKey::HandleCharMessage(const MSG& 
   }
 
   // XXXmnakano I think that if mMsg is WM_CHAR, i.e., it comes without
   //            preceding WM_KEYDOWN, we should should dispatch composition
   //            events instead of eKeyPress because they are not caused by
   //            actual keyboard operation.
 
   // 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);
-    keypressEvent.mCharCode = static_cast<uint32_t>(aCharMsg.wParam);
-    nsresult rv = mDispatcher->BeginNativeInputTransaction();
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      MOZ_LOG(sNativeKeyLogger, LogLevel::Error,
-        ("%p   NativeKey::HandleCharMessage(), FAILED due to "
-         "BeginNativeInputTransaction() failure", this));
-      return true;
-    }
-
-    MOZ_LOG(sNativeKeyLogger, LogLevel::Debug,
-      ("%p   NativeKey::HandleCharMessage(), initializing keypress "
-       "event...", this));
-
-    // When AltGr (Alt+Ctrl) is pressed, that causes normal text input.
-    // At this time, if either alt or ctrl flag is set, EditorBase ignores the
-    // keypress event.  For avoiding this issue, we should remove ctrl and alt
-    // flags.
-    ModifierKeyState modKeyState(mModKeyState);
-    modKeyState.Unset(MODIFIER_ALT | MODIFIER_CONTROL);
-    nsEventStatus status = InitKeyEvent(keypressEvent, modKeyState, &aCharMsg);
-    MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
-      ("%p   NativeKey::HandleCharMessage(), dispatching keypress event...",
-       this));
-    bool dispatched =
-      mDispatcher->MaybeDispatchKeypressEvents(keypressEvent, status,
-                                               const_cast<NativeKey*>(this));
-    if (aEventDispatched) {
-      *aEventDispatched = dispatched;
-    }
-    if (mWidget->Destroyed()) {
-      MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
-        ("%p   NativeKey::HandleCharMessage(), keypress event caused "
-         "destroying the widget", this));
-      return true;
-    }
-    bool consumed = status == nsEventStatus_eConsumeNoDefault;
-    MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
-      ("%p   NativeKey::HandleCharMessage(), dispatched keypress event, "
-       "dispatched=%s, consumed=%s",
-       this, GetBoolName(dispatched), GetBoolName(consumed)));
-    return consumed;
-  }
-
+  WidgetKeyboardEvent keypressEvent(true, eKeyPress, mWidget);
+  keypressEvent.mCharCode = static_cast<uint32_t>(aCharMsg.wParam);
   nsresult rv = mDispatcher->BeginNativeInputTransaction();
   if (NS_WARN_IF(NS_FAILED(rv))) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Error,
       ("%p   NativeKey::HandleCharMessage(), FAILED due to "
        "BeginNativeInputTransaction() failure", this));
     return true;
   }
 
   MOZ_LOG(sNativeKeyLogger, LogLevel::Debug,
     ("%p   NativeKey::HandleCharMessage(), initializing keypress "
-     "event after some hacks...", this));
-  WidgetKeyboardEvent keypressEvent(true, eKeyPress, mWidget);
-  keypressEvent.mCharCode = static_cast<char16_t>(aCharMsg.wParam);
-  if (!keypressEvent.mCharCode) {
-    keypressEvent.mKeyCode = mDOMKeyCode;
+     "event...", this));
+
+  ModifierKeyState modKeyState(mModKeyState);
+  // When AltGr is pressed, both Alt and Ctrl are active.  However, when they
+  // are active, EditorBase won't treat the keypress event as inputting a
+  // character.  Therefore, when AltGr is pressed and the key tries to input
+  // a character, let's set them to false.
+  if (modKeyState.IsAltGr() && IsPrintableCharMessage(aCharMsg)) {
+    modKeyState.Unset(MODIFIER_ALT | MODIFIER_CONTROL);
   }
-  nsEventStatus status = InitKeyEvent(keypressEvent, mModKeyState, &aCharMsg);
+  nsEventStatus status = InitKeyEvent(keypressEvent, modKeyState, &aCharMsg);
   MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
-    ("%p   NativeKey::HandleCharMessage(), dispatching keypress event with "
-     "some hacks...", this));
+    ("%p   NativeKey::HandleCharMessage(), dispatching keypress event...",
+     this));
   bool dispatched =
     mDispatcher->MaybeDispatchKeypressEvents(keypressEvent, status,
                                              const_cast<NativeKey*>(this));
   if (aEventDispatched) {
     *aEventDispatched = dispatched;
   }
   if (mWidget->Destroyed()) {
     MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
       ("%p   NativeKey::HandleCharMessage(), keypress event caused "
        "destroying the widget", this));
     return true;
   }
   bool consumed = status == nsEventStatus_eConsumeNoDefault;
   MOZ_LOG(sNativeKeyLogger, LogLevel::Info,
-    ("%p   NativeKey::HandleCharMessage(), dispatched keypress event with "
-     "some hacks, dispatched=%s, consumed=%s",
+    ("%p   NativeKey::HandleCharMessage(), dispatched keypress event, "
+     "dispatched=%s, consumed=%s",
      this, GetBoolName(dispatched), GetBoolName(consumed)));
   return consumed;
 }
 
 bool
 NativeKey::HandleKeyUpMessage(bool* aEventDispatched) const
 {
   MOZ_ASSERT(IsKeyUpMessage());
@@ -3593,18 +3553,19 @@ KeyboardLayout::InitNativeKey(NativeKey&
   // should be discarded because mKeyValue should have the string to be
   // inputted.
   if (aNativeKey.mMsg.message == WM_CHAR) {
     char16_t ch = static_cast<char16_t>(aNativeKey.mMsg.wParam);
     // But don't set key value as printable key if the character is a control
     // character such as 0x0D at pressing Enter key.
     if (!NativeKey::IsControlChar(ch)) {
       aNativeKey.mKeyNameIndex = KEY_NAME_INDEX_USE_STRING;
-      aNativeKey.mCommittedCharsAndModifiers.
-        Append(ch, aModKeyState.GetModifiers());
+      Modifiers modifiers =
+        aModKeyState.GetModifiers() & ~(MODIFIER_ALT | MODIFIER_CONTROL);
+      aNativeKey.mCommittedCharsAndModifiers.Append(ch, modifiers);
       return;
     }
   }
 
   // If the key is not a usual printable key, KeyboardLayout class assume that
   // it's not cause dead char nor printable char.  Therefore, there are nothing
   // to do here fore such keys (e.g., function keys).
   // However, this should keep dead key state even if non-printable key is