Bug 1511752 - Make IMContextWrapper::OnKeyEvent() treat GDK_KEY_PRESS event without any key information in a dead key sequence as synthesized by current keyboard layout for asynchronous handling r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 03 Dec 2018 15:15:13 +0000
changeset 449262 f7fafed0e4a9c1d8e1895ddaf0b21f6a6a160c0e
parent 449261 215d850888f9c5f04c98cf05cb4ea70444058db4
child 449263 b118f8ec47f530ab3cf08d0bc6c9af7e1ffe4af8
push id35154
push userdvarga@mozilla.com
push dateTue, 04 Dec 2018 09:35:40 +0000
treeherdermozilla-central@5e260e3fd46f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1511752
milestone65.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 1511752 - Make IMContextWrapper::OnKeyEvent() treat GDK_KEY_PRESS event without any key information in a dead key sequence as synthesized by current keyboard layout for asynchronous handling r=m_kato Some keyboard layouts which have dead keys may handle dead key composition asynchronously. In this case, they don't rely on IME like iBus/Fcitx. As far as I've tested, German (QWERTY) keyboard layout is one of such keyboard layout. This returns true when gtk_im_context_filter_keypress() is called for a base character input (like "a"). Then, it synthesizes GDK_KEY_PRESS event without any key information such as: > { type=GDK_KEY_PRESS, keyval=(null), unicode=0x0, state=, time=0, > hardware_keycode=0, group=0 } So, this is not marked as IBUS_IGNORED_MASK nor FcitxKeyState_IgnoredMask by IME, but we should ignore this event since we should've already dispatched "keydown" event for the preceding "a" key event, and anyway "keydown" event for the synthesized event does not make sense for any web apps. This patch makes IMContextWrapper::OnKeyEvent() ignore such key event, i.e., when it's in a dead key sequence, and GDK_KEY_PRESS does not have enough information, e.g., hardware_keycode shouldn't be 0 especially for printable keys. Therefore, this patch make it check only hardware_keycode value and gdk_keyval_to_unicode(aEvent->keyval) value. If some keyboard layouts would send the original key event as is, we would need to do more, but currently, this is enough and safe to land this timing. Differential Revision: https://phabricator.services.mozilla.com/D13656
widget/gtk/IMContextWrapper.cpp
--- a/widget/gtk/IMContextWrapper.cpp
+++ b/widget/gtk/IMContextWrapper.cpp
@@ -837,18 +837,29 @@ KeyHandlingState IMContextWrapper::OnKey
         // If IBUS_IGNORED_MASK was set to aEvent->state, the event
         // has already been handled by another process and it wasn't
         // used by IME.
         bool isHandlingAsyncEvent = !!(aEvent->state & IBUS_IGNORED_MASK);
 
         // ibus won't send back key press events in a dead key sequcne.
         if (mMaybeInDeadKeySequence && aEvent->type == GDK_KEY_PRESS) {
           maybeHandledAsynchronously = false;
-          isUnexpectedAsyncEvent = isHandlingAsyncEvent;
-          break;
+          if (isHandlingAsyncEvent) {
+            isUnexpectedAsyncEvent = true;
+            break;
+          }
+          // Some keyboard layouts which have dead keys may send
+          // "empty" key event to make us call
+          // gtk_im_context_filter_keypress() to commit composed
+          // character during a GDK_KEY_PRESS event dispatching.
+          if (!gdk_keyval_to_unicode(aEvent->keyval) &&
+              !aEvent->hardware_keycode) {
+            isUnexpectedAsyncEvent = true;
+            break;
+          }
         }
         // ibus handles key events synchronously if focused editor is
         // <input type="password"> or |ime-mode: disabled;|.
         if (mInputContext.mIMEState.mEnabled == IMEState::PASSWORD) {
           maybeHandledAsynchronously = false;
           isUnexpectedAsyncEvent = isHandlingAsyncEvent;
           break;
         }
@@ -873,18 +884,29 @@ KeyHandlingState IMContextWrapper::OnKey
         // the event has already been handled by another process and
         // it wasn't used by IME.
         bool isHandlingAsyncEvent =
             !!(aEvent->state & FcitxKeyState_IgnoredMask);
 
         // fcitx won't send back key press events in a dead key sequcne.
         if (mMaybeInDeadKeySequence && aEvent->type == GDK_KEY_PRESS) {
           maybeHandledAsynchronously = false;
-          isUnexpectedAsyncEvent = isHandlingAsyncEvent;
-          break;
+          if (isHandlingAsyncEvent) {
+            isUnexpectedAsyncEvent = true;
+            break;
+          }
+          // Some keyboard layouts which have dead keys may send
+          // "empty" key event to make us call
+          // gtk_im_context_filter_keypress() to commit composed
+          // character during a GDK_KEY_PRESS event dispatching.
+          if (!gdk_keyval_to_unicode(aEvent->keyval) &&
+              !aEvent->hardware_keycode) {
+            isUnexpectedAsyncEvent = true;
+            break;
+          }
         }
 
         // fcitx handles key events asynchronously even if focused
         // editor cannot use IME actually.
 
         if (isHandlingAsyncEvent) {
           MOZ_LOG(gGtkIMLog, LogLevel::Info,
                   ("0x%p   OnKeyEvent(), aEvent->state has "