From d23a5323b49614332a0fd01b2d7c1d602fd3516b Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 12 Mar 2018 15:41:39 +0900 Subject: [PATCH] Bug 1444572 - IMContextWrapper should dispatch fake eKeyDown event during composition if active IM is uim r=m_kato uim is an old IM which uses key snooper to listen to key events rather than via filter key event API which should be called by applications. It's still used by Debian 9.x, so, we still need to support this. Unfortunately, we cannot detect if uim actually uses key snooper because it's switch by build option of uim. Currently, Debian builds uim as using key snooper. So, we should assume uim uses key snooper always. On the other hand, somebody *might* use uim built as not using key snooper, so, let's decide if uim uses key snooper with new pref, "intl.ime.hack.uim.using_key_snooper", but its default should be true. Note that ibus and Fcitx also have the mode to use key snooper (perhaps for backward compatibility with uim). However, it's not enabled in default settings and even if it's enabled, Firefox is in whitelist in the default settings of them for stop using key snooper. Therefore, we don't need to support key snooper mode for them unless we'll get some requests to support their key snooping mode. MozReview-Commit-ID: 6fTsfKrHzvo --HG-- extra : rebase_source : 8ddf4541db635246e6bb0ddc73b012c9be001c6d --- modules/libpref/init/all.js | 7 ++ widget/gtk/IMContextWrapper.cpp | 204 +++++++++++++++++++++++--------- widget/gtk/IMContextWrapper.h | 6 +- widget/gtk/nsGtkKeyUtils.cpp | 8 ++ widget/gtk/nsWindow.cpp | 25 ++-- widget/gtk/nsWindow.h | 13 ++ 6 files changed, 200 insertions(+), 63 deletions(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 8c1b0e7510b5..b9c35546739a 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4558,6 +4558,13 @@ pref("mousewheel.system_scroll_override_on_root_content.enabled", false); pref("intl.ime.use_simple_context_on_password_field", false); +// uim may use key snooper to listen to key events. Unfortunately, we cannot +// know whether it uses or not since it's a build option. So, we need to make +// distribution switchable whether we think uim uses key snooper or not with +// this pref. Debian 9.x still uses uim as their default IM and it uses key +// snooper. So, let's use true for its default value. +pref("intl.ime.hack.uim.using_key_snooper", true); + #ifdef MOZ_WIDGET_GTK // maximum number of fonts to substitute for a generic pref("gfx.font_rendering.fontconfig.max_generic_substitutions", 3); diff --git a/widget/gtk/IMContextWrapper.cpp b/widget/gtk/IMContextWrapper.cpp index 20f189b4fa15..d8689e0000d7 100644 --- a/widget/gtk/IMContextWrapper.cpp +++ b/widget/gtk/IMContextWrapper.cpp @@ -344,21 +344,42 @@ IMContextWrapper::Init() if (contextID.EqualsLiteral("ibus")) { mIMContextID = IMContextID::eIBus; mIsIMInAsyncKeyHandlingMode = !IsIBusInSyncMode(); + // Although ibus has key snooper mode, it's forcibly disabled on Firefox + // in default settings by its whitelist since we always send key events + // to IME before handling shortcut keys. The whitelist can be + // customized with env, IBUS_NO_SNOOPER_APPS, but we don't need to + // support such rare cases for reducing maintenance cost. + mIsKeySnooped = false; } else if (contextID.EqualsLiteral("fcitx")) { mIMContextID = IMContextID::eFcitx; mIsIMInAsyncKeyHandlingMode = !IsFcitxInSyncMode(); + // Although Fcitx has key snooper mode similar to ibus, it's also + // disabled on Firefox in default settings by its whitelist. The + // whitelist can be customized with env, IBUS_NO_SNOOPER_APPS or + // FCITX_NO_SNOOPER_APPS, but we don't need to support such rare cases + // for reducing maintenance cost. + mIsKeySnooped = false; } else if (contextID.EqualsLiteral("uim")) { mIMContextID = IMContextID::eUim; mIsIMInAsyncKeyHandlingMode = false; + // We cannot know if uim uses key snooper since it's build option of + // uim. Therefore, we need to retrieve the consideration from the + // pref for making users and distributions allowed to choose their + // preferred value. + mIsKeySnooped = + Preferences::GetBool("intl.ime.hack.uim.using_key_snooper", true); } else if (contextID.EqualsLiteral("scim")) { mIMContextID = IMContextID::eScim; mIsIMInAsyncKeyHandlingMode = false; + mIsKeySnooped = false; } else if (contextID.EqualsLiteral("iiim")) { mIMContextID = IMContextID::eIIIMF; mIsIMInAsyncKeyHandlingMode = false; + mIsKeySnooped = false; } else { mIMContextID = IMContextID::eUnknown; mIsIMInAsyncKeyHandlingMode = false; + mIsKeySnooped = false; } // Simple context @@ -391,10 +412,11 @@ IMContextWrapper::Init() MOZ_LOG(gGtkIMLog, LogLevel::Info, ("0x%p Init(), mOwnerWindow=%p, mContext=%p (%s), " - "mIsIMInAsyncKeyHandlingMode=%s, mSimpleContext=%p, " - "mDummyContext=%p", + "mIsIMInAsyncKeyHandlingMode=%s, mIsKeySnooped=%s, " + "mSimpleContext=%p, mDummyContext=%p", this, mOwnerWindow, mContext, contextID.get(), - ToChar(mIsIMInAsyncKeyHandlingMode), mSimpleContext, mDummyContext)); + ToChar(mIsIMInAsyncKeyHandlingMode), ToChar(mIsKeySnooped), + mSimpleContext, mDummyContext)); } IMContextWrapper::~IMContextWrapper() @@ -800,7 +822,7 @@ IMContextWrapper::OnKeyEvent(nsWindow* aCaller, // eKeyUp event yet, we need to dispatch here unless the key event is // now being handled by other IME process. if (!maybeHandledAsynchronously) { - MaybeDispatchKeyEventAsProcessedByIME(); + MaybeDispatchKeyEventAsProcessedByIME(eVoidEvent); // Be aware, the widget might have been gone here. } // If we need to wait reply from IM, IM may send some signals to us @@ -1699,11 +1721,16 @@ IMContextWrapper::GetCompositionString(GtkIMContext* aContext, } bool -IMContextWrapper::MaybeDispatchKeyEventAsProcessedByIME() +IMContextWrapper::MaybeDispatchKeyEventAsProcessedByIME( + EventMessage aFollowingEvent) { - if ((!mProcessingKeyEvent && mPostingKeyEvents.IsEmpty()) || - (mProcessingKeyEvent && mKeyboardEventWasDispatched) || - !mLastFocusedWindow) { + if (!mLastFocusedWindow) { + return false; + } + + if (!mIsKeySnooped && + ((!mProcessingKeyEvent && mPostingKeyEvents.IsEmpty()) || + (mProcessingKeyEvent && mKeyboardEventWasDispatched))) { return true; } @@ -1717,53 +1744,119 @@ IMContextWrapper::MaybeDispatchKeyEventAsProcessedByIME() GtkIMContext* oldComposingContext = mComposingContext; RefPtr lastFocusedWindow(mLastFocusedWindow); - if (mProcessingKeyEvent) { - mKeyboardEventWasDispatched = true; - } - // If we're not handling a key event synchronously, the signal may be - // sent by IME without sending key event to us. In such case, we should - // dispatch keyboard event for the last key event which was posted to - // other IME process. - GdkEventKey* sourceEvent = - mProcessingKeyEvent ? mProcessingKeyEvent : - mPostingKeyEvents.GetFirstEvent(); + if (mProcessingKeyEvent || !mPostingKeyEvents.IsEmpty()) { + if (mProcessingKeyEvent) { + mKeyboardEventWasDispatched = true; + } + // If we're not handling a key event synchronously, the signal may be + // sent by IME without sending key event to us. In such case, we + // should dispatch keyboard event for the last key event which was + // posted to other IME process. + GdkEventKey* sourceEvent = + mProcessingKeyEvent ? mProcessingKeyEvent : + mPostingKeyEvents.GetFirstEvent(); - MOZ_LOG(gGtkIMLog, LogLevel::Info, - ("0x%p MaybeDispatchKeyEventAsProcessedByIME(), dispatch %s %s " - "event: { type=%s, keyval=%s, unicode=0x%X, state=%s, " - "time=%u, hardware_keycode=%u, group=%u }", - this, ToChar(sourceEvent->type == GDK_KEY_PRESS ? eKeyDown : eKeyUp), - mProcessingKeyEvent ? "processing" : "posted", - GetEventType(sourceEvent), gdk_keyval_name(sourceEvent->keyval), - gdk_keyval_to_unicode(sourceEvent->keyval), - GetEventStateName(sourceEvent->state, mIMContextID).get(), - sourceEvent->time, sourceEvent->hardware_keycode, sourceEvent->group)); - - // Let's dispatch eKeyDown event or eKeyUp event now. Note that only when - // we're not in a dead key composition, we should mark the eKeyDown and - // eKeyUp event as "processed by IME" since we should expose raw keyCode - // and key value to web apps the key event is a part of a dead key - // sequence. - // FYI: We should ignore if default of preceding keydown or keyup event is - // prevented since even on the other browsers, web applications - // cannot cancel the following composition event. - // Spec bug: https://github.com/w3c/uievents/issues/180 - bool isCancelled; - lastFocusedWindow->DispatchKeyDownOrKeyUpEvent(sourceEvent, - !mMaybeInDeadKeySequence, - &isCancelled); - MOZ_LOG(gGtkIMLog, LogLevel::Info, - ("0x%p MaybeDispatchKeyEventAsProcessedByIME(), keydown or keyup " - "event is dispatched", - this)); - - if (!mProcessingKeyEvent) { MOZ_LOG(gGtkIMLog, LogLevel::Info, - ("0x%p MaybeDispatchKeyEventAsProcessedByIME(), removing first " - "event from the queue", + ("0x%p MaybeDispatchKeyEventAsProcessedByIME(" + "aFollowingEvent=%s), dispatch %s %s " + "event: { type=%s, keyval=%s, unicode=0x%X, state=%s, " + "time=%u, hardware_keycode=%u, group=%u }", + this, ToChar(aFollowingEvent), + ToChar(sourceEvent->type == GDK_KEY_PRESS ? eKeyDown : eKeyUp), + mProcessingKeyEvent ? "processing" : "posted", + GetEventType(sourceEvent), gdk_keyval_name(sourceEvent->keyval), + gdk_keyval_to_unicode(sourceEvent->keyval), + GetEventStateName(sourceEvent->state, mIMContextID).get(), + sourceEvent->time, sourceEvent->hardware_keycode, + sourceEvent->group)); + + // Let's dispatch eKeyDown event or eKeyUp event now. Note that only + // when we're not in a dead key composition, we should mark the + // eKeyDown and eKeyUp event as "processed by IME" since we should + // expose raw keyCode and key value to web apps the key event is a + // part of a dead key sequence. + // FYI: We should ignore if default of preceding keydown or keyup + // event is prevented since even on the other browsers, web + // applications cannot cancel the following composition event. + // Spec bug: https://github.com/w3c/uievents/issues/180 + bool isCancelled; + lastFocusedWindow->DispatchKeyDownOrKeyUpEvent(sourceEvent, + !mMaybeInDeadKeySequence, + &isCancelled); + MOZ_LOG(gGtkIMLog, LogLevel::Info, + ("0x%p MaybeDispatchKeyEventAsProcessedByIME(), keydown or keyup " + "event is dispatched", this)); - mPostingKeyEvents.RemoveEvent(sourceEvent); + + if (!mProcessingKeyEvent) { + MOZ_LOG(gGtkIMLog, LogLevel::Info, + ("0x%p MaybeDispatchKeyEventAsProcessedByIME(), removing first " + "event from the queue", + this)); + mPostingKeyEvents.RemoveEvent(sourceEvent); + } + } else { + MOZ_ASSERT(mIsKeySnooped); + // Currently, we support key snooper mode of uim only. + MOZ_ASSERT(mIMContextID == IMContextID::eUim); + // uim sends "preedit_start" signal and "preedit_changed" separately + // at starting composition, "commit" and "preedit_end" separately at + // committing composition. + + // Currently, we should dispatch only fake eKeyDown event because + // we cannot decide which is the last signal of each key operation + // and Chromium also dispatches only "keydown" event in this case. + bool dispatchFakeKeyDown = false; + switch (aFollowingEvent) { + case eCompositionStart: + case eCompositionCommit: + case eCompositionCommitAsIs: + dispatchFakeKeyDown = true; + break; + // XXX Unfortunately, I don't have a good idea to prevent to + // dispatch redundant eKeyDown event for eCompositionStart + // immediately after "delete_surrounding" signal. However, + // not dispatching eKeyDown event is worse than dispatching + // redundant eKeyDown events. + case eContentCommandDelete: + dispatchFakeKeyDown = true; + break; + // We need to prevent to dispatch redundant eKeyDown event for + // eCompositionChange immediately after eCompositionStart. So, + // We should not dispatch eKeyDown event if dispatched composition + // string is still empty string. + case eCompositionChange: + dispatchFakeKeyDown = !mDispatchedCompositionString.IsEmpty(); + break; + default: + MOZ_ASSERT_UNREACHABLE("Do you forget to handle the case?"); + break; + } + + if (dispatchFakeKeyDown) { + WidgetKeyboardEvent fakeKeyDownEvent(true, eKeyDown, + lastFocusedWindow); + fakeKeyDownEvent.mKeyCode = NS_VK_PROCESSKEY; + fakeKeyDownEvent.mKeyNameIndex = KEY_NAME_INDEX_Process; + // It's impossible to get physical key information in this case but + // this should be okay since web apps shouldn't do anything with + // physical key information during composition. + fakeKeyDownEvent.mCodeNameIndex = CODE_NAME_INDEX_UNKNOWN; + + MOZ_LOG(gGtkIMLog, LogLevel::Info, + ("0x%p MaybeDispatchKeyEventAsProcessedByIME(" + "aFollowingEvent=%s), dispatch fake eKeyDown event", + this, ToChar(aFollowingEvent))); + + bool isCancelled; + lastFocusedWindow->DispatchKeyDownOrKeyUpEvent(fakeKeyDownEvent, + &isCancelled); + MOZ_LOG(gGtkIMLog, LogLevel::Info, + ("0x%p MaybeDispatchKeyEventAsProcessedByIME(), " + "fake keydown event is dispatched", + this)); + } } if (lastFocusedWindow->IsDestroyed() || @@ -1842,7 +1935,7 @@ IMContextWrapper::DispatchCompositionStart(GtkIMContext* aContext) // eKeyDown or eKeyUp event before dispatching eCompositionStart event. // Note that dispatching a keyboard event which is marked as "processed // by IME" is okay since Chromium also dispatches keyboard event as so. - if (!MaybeDispatchKeyEventAsProcessedByIME()) { + if (!MaybeDispatchKeyEventAsProcessedByIME(eCompositionStart)) { MOZ_LOG(gGtkIMLog, LogLevel::Warning, ("0x%p DispatchCompositionStart(), Warning, " "MaybeDispatchKeyEventAsProcessedByIME() returned false", @@ -1907,7 +2000,7 @@ IMContextWrapper::DispatchCompositionChangeEvent( } // If this composition string change caused by a key press, we need to // dispatch eKeyDown or eKeyUp before dispatching eCompositionChange event. - else if (!MaybeDispatchKeyEventAsProcessedByIME()) { + else if (!MaybeDispatchKeyEventAsProcessedByIME(eCompositionChange)) { MOZ_LOG(gGtkIMLog, LogLevel::Warning, ("0x%p DispatchCompositionChangeEvent(), Warning, " "MaybeDispatchKeyEventAsProcessedByIME() returned false", @@ -2029,7 +2122,8 @@ IMContextWrapper::DispatchCompositionCommitEvent( } // If this commit caused by a key press, we need to dispatch eKeyDown or // eKeyUp before dispatching composition events. - else if (!MaybeDispatchKeyEventAsProcessedByIME()) { + else if (!MaybeDispatchKeyEventAsProcessedByIME( + aCommitString ? eCompositionCommit : eCompositionCommitAsIs)) { MOZ_LOG(gGtkIMLog, LogLevel::Warning, ("0x%p DispatchCompositionCommitEvent(), Warning, " "MaybeDispatchKeyEventAsProcessedByIME() returned false", @@ -2738,7 +2832,7 @@ IMContextWrapper::DeleteText(GtkIMContext* aContext, // If this deleting text caused by a key press, we need to dispatch // eKeyDown or eKeyUp before dispatching eContentCommandDelete event. - if (!MaybeDispatchKeyEventAsProcessedByIME()) { + if (!MaybeDispatchKeyEventAsProcessedByIME(eContentCommandDelete)) { MOZ_LOG(gGtkIMLog, LogLevel::Warning, ("0x%p DeleteText(), Warning, " "MaybeDispatchKeyEventAsProcessedByIME() returned false", diff --git a/widget/gtk/IMContextWrapper.h b/widget/gtk/IMContextWrapper.h index b7f95b1ee9d8..ed8b1a936257 100644 --- a/widget/gtk/IMContextWrapper.h +++ b/widget/gtk/IMContextWrapper.h @@ -455,6 +455,9 @@ protected: // key events asynchronously. I.e., filtered key event may come again // later. bool mIsIMInAsyncKeyHandlingMode; + // mIsKeySnooped is set to true if IM uses key snooper to listen key events. + // In such case, we won't receive key events if IME consumes the event. + bool mIsKeySnooped; // sLastFocusedContext is a pointer to the last focused instance of this // class. When a instance is destroyed and sLastFocusedContext refers it, @@ -620,13 +623,14 @@ protected: * dispatches a keyboard event, this sets mKeyboardEventWasDispatched * to true. * + * @param aFollowingEvent The following event message. * @return If the caller can continue to handle * composition, returns true. Otherwise, * false. For example, if focus is moved * by dispatched keyboard event, returns * false. */ - bool MaybeDispatchKeyEventAsProcessedByIME(); + bool MaybeDispatchKeyEventAsProcessedByIME(EventMessage aFollowingEvent); /** * Dispatches a composition start event. diff --git a/widget/gtk/nsGtkKeyUtils.cpp b/widget/gtk/nsGtkKeyUtils.cpp index 6a02b9a1bc67..133dc42a442c 100644 --- a/widget/gtk/nsGtkKeyUtils.cpp +++ b/widget/gtk/nsGtkKeyUtils.cpp @@ -1411,6 +1411,14 @@ void KeymapWrapper::WillDispatchKeyboardEventInternal(WidgetKeyboardEvent& aKeyEvent, GdkEventKey* aGdkKeyEvent) { + if (!aGdkKeyEvent) { + // If aGdkKeyEvent is nullptr, we're trying to dispatch a fake keyboard + // event in such case, we don't need to set alternative char codes. + // So, we don't need to do nothing here. This case is typically we're + // dispatching eKeyDown or eKeyUp event during composition. + return; + } + uint32_t charCode = GetCharCodeFor(aGdkKeyEvent); if (!charCode) { MOZ_LOG(gKeymapWrapperLog, LogLevel::Info, diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp index 139bf0eb17d0..e10b95e90472 100644 --- a/widget/gtk/nsWindow.cpp +++ b/widget/gtk/nsWindow.cpp @@ -2941,7 +2941,7 @@ nsWindow::DispatchKeyDownOrKeyUpEvent(GdkEventKey* aEvent, bool aIsProcessedByIME, bool* aIsCancelled) { - MOZ_ASSERT(aIsCancelled, "aCancelled must not be null"); + MOZ_ASSERT(aIsCancelled, "aIsCancelled must not be nullptr"); *aIsCancelled = false; @@ -2949,21 +2949,32 @@ nsWindow::DispatchKeyDownOrKeyUpEvent(GdkEventKey* aEvent, return false; } + EventMessage message = + aEvent->type == GDK_KEY_PRESS ? eKeyDown : eKeyUp; + WidgetKeyboardEvent keyEvent(true, message, this); + KeymapWrapper::InitKeyEvent(keyEvent, aEvent, aIsProcessedByIME); + return DispatchKeyDownOrKeyUpEvent(keyEvent, aIsCancelled); +} +bool +nsWindow::DispatchKeyDownOrKeyUpEvent(WidgetKeyboardEvent& aKeyboardEvent, + bool* aIsCancelled) +{ + MOZ_ASSERT(aIsCancelled, "aIsCancelled must not be nullptr"); + + *aIsCancelled = false; + RefPtr dispatcher = GetTextEventDispatcher(); nsresult rv = dispatcher->BeginNativeInputTransaction(); if (NS_WARN_IF(NS_FAILED(rv))) { return FALSE; } - EventMessage message = - aEvent->type == GDK_KEY_PRESS ? eKeyDown : eKeyUp; - WidgetKeyboardEvent keyEvent(true, message, this); - KeymapWrapper::InitKeyEvent(keyEvent, aEvent, aIsProcessedByIME); nsEventStatus status = nsEventStatus_eIgnore; bool dispatched = - dispatcher->DispatchKeyboardEvent(message, keyEvent, status, aEvent); + dispatcher->DispatchKeyboardEvent(aKeyboardEvent.mMessage, + aKeyboardEvent, status, nullptr); *aIsCancelled = (status == nsEventStatus_eConsumeNoDefault); - return dispatched ? TRUE : FALSE; + return dispatched; } WidgetEventTime diff --git a/widget/gtk/nsWindow.h b/widget/gtk/nsWindow.h index 831ae85281f4..505aa018fe86 100644 --- a/widget/gtk/nsWindow.h +++ b/widget/gtk/nsWindow.h @@ -78,6 +78,7 @@ class nsWindow final : public nsBaseWidget public: typedef mozilla::gfx::DrawTarget DrawTarget; typedef mozilla::WidgetEventTime WidgetEventTime; + typedef mozilla::WidgetKeyboardEvent WidgetKeyboardEvent; typedef mozilla::widget::PlatformCompositorWidgetDelegate PlatformCompositorWidgetDelegate; nsWindow(); @@ -294,6 +295,18 @@ public: bool aProcessedByIME, bool* aIsCancelled); + /** + * DispatchKeyDownOrKeyUpEvent() dispatches eKeyDown or eKeyUp event. + * + * @param aEvent An eKeyDown or eKeyUp event. This will be + * dispatched as is. + * @param aIsCancelled [Out] true if the default is prevented. + * @return true if eKeyDown event is actually dispatched. + * Otherwise, false. + */ + bool DispatchKeyDownOrKeyUpEvent(WidgetKeyboardEvent& aEvent, + bool* aIsCancelled); + WidgetEventTime GetWidgetEventTime(guint32 aEventTime); mozilla::TimeStamp GetEventTimeStamp(guint32 aEventTime); mozilla::CurrentX11TimeGetter* GetCurrentTimeGetter();