From 3664b56c3395fa146dc8aac93452cd98b2fbb0be Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 7 Sep 2026 07:20:40 +0000 Subject: [PATCH] Bug 2061489 - Make `IMContextWrapper::CommitCompositionNative` not dispatch `eKeyDown` event if we've already dispatched r=m_kato `IMContextWrapper::CommitCompositionNative` started to dispatch `eKeyDown` and `eKeyPress` as far as possible for Microsoft Word's bug. However, we should not do that for a special case. That is, IME may send canceling composition first, then, send `commit` signal without another composing state during a call of `gtk_im_context_filter_keypress`. The reason is, some web apps may count `keydown` events and that may be important for them (e.g., typing test apps). Therefore, this patch makes `CommitCompositionNative` treats the commit composition without composing state as an input without a key press after we've already dispatched `eKeyDown`. Note that this is the traditional behavior before bug 2051354 even though the old code didn't check `mKeyboardEventWasDispatched`. https://searchfox.org/firefox-main/source/widget/gtk/IMContextWrapper.cpp#1991,2003,2013,2019,2036,2040 (not matching the `keyval` and `commitString`, `mMaybeInDeadKeySequence` was set to `false` at dispatching the composition commit event for the preceding composition cancel and there is `mProcessingKeyEvent`, so, `DispatchCompositionCommitEvent()` was used.) Differential Revision: https://phabricator.services.mozilla.com/D323805 --- widget/gtk/IMContextWrapper.cpp | 38 +++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/widget/gtk/IMContextWrapper.cpp b/widget/gtk/IMContextWrapper.cpp index 6e5c2802b823..e8a3b8e2ac32 100644 --- a/widget/gtk/IMContextWrapper.cpp +++ b/widget/gtk/IMContextWrapper.cpp @@ -347,6 +347,24 @@ class MOZ_STACK_CLASS IMContextWrapper::AutoHandlingCompositionSignalHelper { return mIMContextWrapper.mHandlingKeyEvent && !mTemporarilySetEvent; } + /** + * Return true if the owner should not dispatch eKeyDown for handling a commit + * event without composing state anymore. + */ + [[nodiscard]] bool ShouldNotDispatchKeyEvents() const { + // If the owner is NOT handling the event during a call of + // gtk_im_context_filter_keypress(), we may need to dispatch another + // eKeyDown event for IME which consumes the keyboard events such as + // Wayland. + return IsCallingGtkIMContextFilterKeypress() && + // Otherwise, i.e., if the commit occurs during a call of + // gtk_im_context_filter_keypress() and we've already dispatched + // eKeyDown for the handling key event, we should not dispatch + // another eKeyDown for web apps which count the `keydown` events, + // e.g., typing apps. + mIMContextWrapper.mKeyboardEventWasDispatched; + } + [[nodiscard]] bool EditorMayHandleKeyPressEventAsTextInput() const { return mIMContextWrapper.mHandlingKeyEvent && mIMContextWrapper.mHandlingKeyEvent->type == GDK_KEY_PRESS && @@ -1211,11 +1229,13 @@ KeyHandlingState IMContextWrapper::OnKeyEvent( // the caller should've stopped handling the event if preceding eKeyDown // event was consumed. if (aKeyboardEventWasDispatched) { + MOZ_ASSERT(mGraphemeClusterFallbackToKeyEvent.IsVoid()); return KeyHandlingState::eNotHandledButEventDispatched; } if (!mKeyboardEventWasDispatched) { return KeyHandlingState::eNotHandled; } + MOZ_ASSERT(mGraphemeClusterFallbackToKeyEvent.IsVoid()); return mKeyboardEventWasConsumed ? KeyHandlingState::eNotHandledButEventConsumed : KeyHandlingState::eNotHandledButEventDispatched; @@ -2018,15 +2038,15 @@ void IMContextWrapper::OnCommitCompositionNative(GtkIMContext* aContext, "{} OnCommitCompositionNative(aContext={}), " "current context={}, active context={}, utf8CommitString=\"{}\", " "mHandlingKeyEvent={}, mPendingKeyEvents.CountOfPendingEvents()={}, " - "IsComposingOn(aContext)={}, editorMayTreatKeyPressAsTypingText={}", + "IsComposingOn(aContext)={}, EditorMayTreatKeyPressAsTypingText={}, " + "ShouldNotDispatchKeyEvents()={}", static_cast(this), static_cast(aContext), static_cast(GetCurrentContext()), static_cast(GetActiveContext()), utf8CommitString, static_cast(mHandlingKeyEvent), - mPendingKeyEvents.CountOfPendingEvents(), - TrueOrFalse(IsComposingOn(aContext)), - TrueOrFalse( - signalHandlerHelper.EditorMayHandleKeyPressEventAsTextInput())); + mPendingKeyEvents.CountOfPendingEvents(), IsComposingOn(aContext), + signalHandlerHelper.EditorMayHandleKeyPressEventAsTextInput(), + signalHandlerHelper.ShouldNotDispatchKeyEvents()); if (!IsComposingOn(aContext)) { // If we are not in composition and committing with empty string, @@ -2043,7 +2063,13 @@ void IMContextWrapper::OnCommitCompositionNative(GtkIMContext* aContext, } if (KeymapWrapper::StringHasOnlyOneGraphemeCluster(utf16CommitString) && - aContext == GetCurrentContext()) { + aContext == GetCurrentContext() && + // Some IME may cancel composition and then commit composition without + // another composing state. In this case, we've already dispatched a + // processed keydown event. So, we should not dispatch another keydown + // event followed by a printable keypress event in such case. Anyway, + // we cannot do that via OnKeyEvent(). + !signalHandlerHelper.ShouldNotDispatchKeyEvents()) { // If IME inserts commit string for the current key press event or for the // immediate preceding key press event without composing state, the IME // must want to work as a keyboard layout. Then, if and only if the commit