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

@@ -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);

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<nsWindow> 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",

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.

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,

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<TextEventDispatcher> 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

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();