Bug 1143197 part.1 Use current IM context at handling key events rather than active IM context r=m_kato

widget/gtk/nsGtkIMModule.cpp

@@ -21,6 +21,12 @@ using namespace mozilla::widget;
 #ifdef PR_LOGGING
 PRLogModuleInfo* gGtkIMLog = nullptr;
 
+static const char*
+GetBoolName(bool aBool)
+{
+  return aBool ? "true" : "false";
+}
+
 static const char*
 GetRangeTypeName(uint32_t aRangeType)
 {
@@ -56,6 +62,19 @@ GetEnabledStateName(uint32_t aState)
             return "UNKNOWN ENABLED STATUS!!";
     }
 }
+
+static const char*
+GetEventType(GdkEventKey* aKeyEvent)
+{
+    switch (aKeyEvent->type) {
+        case GDK_KEY_PRESS:
+            return "GDK_KEY_PRESS";
+        case GDK_KEY_RELEASE:
+            return "GDK_KEY_RELEASE";
+        default:
+            return "Unknown";
+    }
+}
 #endif
 
 const static bool kUseSimpleContextDefault = MOZ_WIDGET_GTK == 2;
@@ -298,8 +317,9 @@ nsGtkIMModule::OnBlurWindow(nsWindow* aWindow)
     }
 
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
-        ("GtkIMModule(%p): OnBlurWindow, aWindow=%p, mLastFocusedWindow=%p, mIsIMFocused=%s",
+        ("GtkIMModule(%p): OnBlurWindow, aWindow=%p, mLastFocusedWindow=%p, "
-         this, aWindow, mLastFocusedWindow, mIsIMFocused ? "YES" : "NO"));
+         "mIsIMFocused=%s",
+         this, aWindow, mLastFocusedWindow, GetBoolName(mIsIMFocused)));
 
     if (!mIsIMFocused || mLastFocusedWindow != aWindow) {
         return;
@@ -320,13 +340,12 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent,
     }
 
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
-        ("GtkIMModule(%p): OnKeyEvent, aCaller=%p, aKeyDownEventWasSent=%s",
+        ("GtkIMModule(%p): OnKeyEvent, aCaller=%p, aKeyDownEventWasSent=%s, "
-         this, aCaller, aKeyDownEventWasSent ? "TRUE" : "FALSE"));
+         "mCompositionState=%s, current context=%p, active context=%p, "
-    PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
+         "aEvent(%p): { type=%s, keyval=%s, unicode=0x%X }",
-        ("    aEvent: type=%s, keyval=%s, unicode=0x%X",
+         this, aCaller, GetBoolName(aKeyDownEventWasSent),
-         aEvent->type == GDK_KEY_PRESS ? "GDK_KEY_PRESS" :
+         GetCompositionStateName(), GetCurrentContext(), GetActiveContext(),
-         aEvent->type == GDK_KEY_RELEASE ? "GDK_KEY_RELEASE" : "Unknown",
+         aEvent, GetEventType(aEvent), gdk_keyval_name(aEvent->keyval),
-         gdk_keyval_name(aEvent->keyval),
          gdk_keyval_to_unicode(aEvent->keyval)));
 
     if (aCaller != mLastFocusedWindow) {
@@ -336,8 +355,10 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent,
         return false;
     }
 
-    GtkIMContext* activeContext = GetActiveContext();
+    // Even if old IM context has composition, key event should be sent to
-    if (MOZ_UNLIKELY(!activeContext)) {
+    // current context since the user expects so.
+    GtkIMContext* currentContext = GetCurrentContext();
+    if (MOZ_UNLIKELY(!currentContext)) {
         PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
             ("    FAILED, there are no context"));
         return false;
@@ -347,7 +368,7 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent,
     mFilterKeyEvent = true;
     mProcessingKeyEvent = aEvent;
     gboolean isFiltered =
-        gtk_im_context_filter_keypress(activeContext, aEvent);
+        gtk_im_context_filter_keypress(currentContext, aEvent);
     mProcessingKeyEvent = nullptr;
 
     // We filter the key event if the event was not committed (because
@@ -357,7 +378,7 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent,
     // composed characters.
     bool filterThisEvent = isFiltered && mFilterKeyEvent;
 
-    if (IsComposing() && !isFiltered) {
+    if (IsComposingOnCurrentContext() && !isFiltered) {
         if (aEvent->type == GDK_KEY_PRESS) {
             if (!mDispatchedCompositionString.IsEmpty()) {
                 // If there is composition string, we shouldn't dispatch
@@ -371,7 +392,7 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent,
                 // IM.  For compromising this issue, we should dispatch
                 // compositionend event, however, we don't need to reset IM
                 // actually.
-                DispatchCompositionCommitEvent(activeContext, &EmptyString());
+                DispatchCompositionCommitEvent(currentContext, &EmptyString());
                 filterThisEvent = false;
             }
         } else {
@@ -382,9 +403,10 @@ nsGtkIMModule::OnKeyEvent(nsWindow* aCaller, GdkEventKey* aEvent,
     }
 
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
-        ("    filterThisEvent=%s (isFiltered=%s, mFilterKeyEvent=%s)",
+        ("    filterThisEvent=%s (isFiltered=%s, mFilterKeyEvent=%s), "
-         filterThisEvent ? "TRUE" : "FALSE", isFiltered ? "YES" : "NO",
+         "mCompositionState=%s",
-         mFilterKeyEvent ? "YES" : "NO"));
+         GetBoolName(filterThisEvent), GetBoolName(isFiltered),
+         GetBoolName(mFilterKeyEvent), GetCompositionStateName()));
 
     return filterThisEvent;
 }
@@ -395,8 +417,8 @@ nsGtkIMModule::OnFocusChangeInGecko(bool aFocus)
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
         ("GtkIMModule(%p): OnFocusChangeInGecko, aFocus=%s, "
          "mCompositionState=%s, mIsIMFocused=%s",
-         this, aFocus ? "YES" : "NO", GetCompositionStateName(),
+         this, GetBoolName(aFocus), GetCompositionStateName(),
-         mIsIMFocused ? "YES" : "NO"));
+         GetBoolName(mIsIMFocused)));
 
     // We shouldn't carry over the removed string to another editor.
     mSelectedString.Truncate();
@@ -407,7 +429,7 @@ nsGtkIMModule::ResetIME()
 {
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
         ("GtkIMModule(%p): ResetIME, mCompositionState=%s, mIsIMFocused=%s",
-         this, GetCompositionStateName(), mIsIMFocused ? "YES" : "NO"));
+         this, GetCompositionStateName(), GetBoolName(mIsIMFocused)));
 
     GtkIMContext* activeContext = GetActiveContext();
     if (MOZ_UNLIKELY(!activeContext)) {
@@ -646,7 +668,7 @@ nsGtkIMModule::Blur()
 {
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
         ("GtkIMModule(%p): Blur, mIsIMFocused=%s",
-         this, mIsIMFocused ? "YES" : "NO"));
+         this, GetBoolName(mIsIMFocused)));
 
     if (!mIsIMFocused) {
         return;
@@ -889,11 +911,13 @@ nsGtkIMModule::OnCommitCompositionNative(GtkIMContext *aContext,
 
     PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
         ("GtkIMModule(%p): OnCommitCompositionNative, aContext=%p, "
-         "current context=%p, commitString=\"%s\"",
+         "current context=%p, active context=%p, commitString=\"%s\", "
-         this, aContext, GetCurrentContext(), commitString));
+         "mProcessingKeyEvent=%p, IsComposingOn(aContext)=%s",
+         this, aContext, GetCurrentContext(), GetActiveContext(), commitString,
+         mProcessingKeyEvent, GetBoolName(IsComposingOn(aContext))));
 
     // See bug 472635, we should do nothing if IM context doesn't match.
-    if (GetCurrentContext() != aContext) {
+    if (!IsValidContext(aContext)) {
         PR_LOG(gGtkIMLog, PR_LOG_ALWAYS,
             ("    FAILED, given context doesn't match"));
         return;
@@ -904,14 +928,17 @@ nsGtkIMModule::OnCommitCompositionNative(GtkIMContext *aContext,
     // signal, we would dispatch compositionstart, text, compositionend
     // events with empty string.  Of course, they are unnecessary events
     // for Web applications and our editor.
-    if (!IsComposing() && !commitString[0]) {
+    if (!IsComposingOn(aContext) && !commitString[0]) {
         return;
     }
 
     // If IME doesn't change their keyevent that generated this commit,
     // don't send it through XIM - just send it as a normal key press
     // event.
-    if (!IsComposing() && mProcessingKeyEvent) {
+    // NOTE: While a key event is being handled, this might be caused on
+    // current context.  Otherwise, this may be caused on active context.
+    if (!IsComposingOn(aContext) && mProcessingKeyEvent &&
+        aContext == GetCurrentContext()) {
         char keyval_utf8[8]; /* should have at least 6 bytes of space */
         gint keyval_utf8_len;
         guint32 keyval_unicode;

widget/gtk/nsGtkIMModule.h

@@ -132,11 +132,21 @@ protected:
     };
     eCompositionState mCompositionState;
 
-    bool IsComposing()
+    bool IsComposing() const
     {
         return (mCompositionState != eCompositionState_NotComposing);
     }
 
+    bool IsComposingOn(GtkIMContext* aContext) const
+    {
+        return IsComposing() && mComposingContext == aContext;
+    }
+
+    bool IsComposingOnCurrentContext() const
+    {
+        return IsComposingOn(GetCurrentContext());
+    }
+
     bool EditorHasCompositionString()
     {
         return (mCompositionState ==