From 3a0b903d0ab4a3c0f6aa6f7e23ab4f87e46e88b4 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 5 Feb 2019 11:59:38 +0000 Subject: [PATCH] Bug 1523635 - part 2: Make IMContextWrapper::OnKeyEvent() trust the result of gtk_im_context_filter_keypress() to decide whether handling event causes async event later r=m_kato Unfortunately, we're not sure whether ibus always handles non-dead key events asynchronously or synchoronously. Therefore, for safer fix, this patch makes IMContextWrapper::OnKeyEvent() decide that with the result of gtk_im_context_filter_keypress(). If active IME is ibus and it consumes non- synthesized events on password fields, it adjusts probablyHandledAsynchronously value. So, this patch changes the behavior of only when: - active IME is ibus. - only when a password field or ime-mode:disabled field has focus. - not in dead key sequence. - and the key event is consumed by ibus but nothing happend. This must be enough safe to uplift. Differential Revision: https://phabricator.services.mozilla.com/D18635 --HG-- extra : moz-landing-system : lando --- widget/gtk/IMContextWrapper.cpp | 84 +++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/widget/gtk/IMContextWrapper.cpp b/widget/gtk/IMContextWrapper.cpp index eac5ab0ba715..a90f48fd4229 100644 --- a/widget/gtk/IMContextWrapper.cpp +++ b/widget/gtk/IMContextWrapper.cpp @@ -827,6 +827,14 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( bool probablyHandledAsynchronously = mIsIMInAsyncKeyHandlingMode && currentContext == mContext; + // If we're not sure whether the event is handled asynchronously, this is + // set to true. + bool maybeHandledAsynchronously = false; + + // If aEvent is a synthesized event for async handling, this will be set to + // true. + bool isHandlingAsyncEvent = false; + // If we've decided that the event won't be synthesized asyncrhonously // by IME, but actually IME did it, this is set to true. bool isUnexpectedAsyncEvent = false; @@ -844,7 +852,7 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( // 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); + isHandlingAsyncEvent = !!(aEvent->state & IBUS_IGNORED_MASK); if (!isHandlingAsyncEvent) { // On some environments, IBUS_IGNORED_MASK flag is not set as // expected. In such case, we keep pusing all events into the queue. @@ -864,6 +872,21 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( } } + // If it's a synthesized event, let's remove it from the posting + // event queue first. Otherwise the following blocks cannot use + // `break`. + if (isHandlingAsyncEvent) { + MOZ_LOG(gGtkIMLog, LogLevel::Info, + ("0x%p OnKeyEvent(), aEvent->state has IBUS_IGNORED_MASK " + "or aEvent is in the " + "posting event queue, so, it won't be handled " + "asynchronously anymore. Removing " + "the posted events from the queue", + this)); + probablyHandledAsynchronously = false; + mPostingKeyEvents.RemoveEvent(aEvent); + } + // ibus won't send back key press events in a dead key sequcne. if (mMaybeInDeadKeySequence && aEvent->type == GDK_KEY_PRESS) { probablyHandledAsynchronously = false; @@ -880,25 +903,15 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( isUnexpectedAsyncEvent = true; break; } - } - // ibus handles key events synchronously if focused editor is - // or |ime-mode: disabled;|. - if (mInputContext.mIMEState.mEnabled == IMEState::PASSWORD) { - probablyHandledAsynchronously = false; - isUnexpectedAsyncEvent = isHandlingAsyncEvent; break; } - - if (isHandlingAsyncEvent) { - MOZ_LOG(gGtkIMLog, LogLevel::Info, - ("0x%p OnKeyEvent(), aEvent->state has IBUS_IGNORED_MASK " - "or aEvent is in the " - "posting event queue, so, it won't be handled " - "asynchronously anymore. Removing " - "the posted events from the queue", - this)); + // ibus may handle key events synchronously if focused editor is + // or |ime-mode: disabled;|. However, in + // some environments, not so actually. Therefore, we need to check + // the result of gtk_im_context_filter_keypress() later. + if (mInputContext.mIMEState.mEnabled == IMEState::PASSWORD) { probablyHandledAsynchronously = false; - mPostingKeyEvents.RemoveEvent(aEvent); + maybeHandledAsynchronously = !isHandlingAsyncEvent; break; } break; @@ -909,8 +922,7 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( // If FcitxKeyState_IgnoredMask 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 & FcitxKeyState_IgnoredMask); + isHandlingAsyncEvent = !!(aEvent->state & FcitxKeyState_IgnoredMask); if (!isHandlingAsyncEvent) { // On some environments, FcitxKeyState_IgnoredMask flag *might* be not // set as expected. If there were such cases, we'd keep pusing all @@ -988,6 +1000,16 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( mFallbackToKeyEvent = false; mProcessingKeyEvent = aEvent; gboolean isFiltered = gtk_im_context_filter_keypress(currentContext, aEvent); + + // If we're not sure whether the event is handled by IME asynchronously or + // synchronously, we need to trust the result of + // gtk_im_context_filter_keypress(). If it consumed and but did nothing, + // we can assume that another event will be synthesized. + if (!isHandlingAsyncEvent && maybeHandledAsynchronously) { + probablyHandledAsynchronously |= + isFiltered && !mFallbackToKeyEvent && !mKeyboardEventWasDispatched; + } + if (aEvent->type == GDK_KEY_PRESS) { if (isFiltered && probablyHandledAsynchronously) { sWaitingSynthesizedKeyPressHardwareKeyCode = aEvent->hardware_keycode; @@ -1050,17 +1072,19 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( mMaybeInDeadKeySequence = false; } - MOZ_LOG(gGtkIMLog, LogLevel::Debug, - ("0x%p OnKeyEvent(), succeeded, filterThisEvent=%s " - "(isFiltered=%s, mFallbackToKeyEvent=%s, " - "probablyHandledAsynchronously=%s), mPostingKeyEvents.Length()=%zu, " - "mCompositionState=%s, mMaybeInDeadKeySequence=%s, " - "mKeyboardEventWasDispatched=%s, mKeyboardEventWasConsumed=%s", - this, ToChar(filterThisEvent), ToChar(isFiltered), - ToChar(mFallbackToKeyEvent), ToChar(probablyHandledAsynchronously), - mPostingKeyEvents.Length(), GetCompositionStateName(), - ToChar(mMaybeInDeadKeySequence), ToChar(mKeyboardEventWasDispatched), - ToChar(mKeyboardEventWasConsumed))); + MOZ_LOG( + gGtkIMLog, LogLevel::Debug, + ("0x%p OnKeyEvent(), succeeded, filterThisEvent=%s " + "(isFiltered=%s, mFallbackToKeyEvent=%s, " + "probablyHandledAsynchronously=%s, maybeHandledAsynchronously=%s), " + "mPostingKeyEvents.Length()=%zu, mCompositionState=%s, " + "mMaybeInDeadKeySequence=%s, mKeyboardEventWasDispatched=%s, " + "mKeyboardEventWasConsumed=%s", + this, ToChar(filterThisEvent), ToChar(isFiltered), + ToChar(mFallbackToKeyEvent), ToChar(probablyHandledAsynchronously), + ToChar(maybeHandledAsynchronously), mPostingKeyEvents.Length(), + GetCompositionStateName(), ToChar(mMaybeInDeadKeySequence), + ToChar(mKeyboardEventWasDispatched), ToChar(mKeyboardEventWasConsumed))); if (filterThisEvent) { return KeyHandlingState::eHandled;