Bug 1138159 Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 15 Sep 2016 22:36:23 +0900
changeset 414102 a5fe78db0884d2223f9dae0e5c70105e4855e7e1
parent 413960 29af101880db7ce7f5f87f58e1ff20988c1c5fc3
child 531363 330136ebeec786a7d95c1ba43a8d4273b0fa64b6
push id29583
push usermasayuki@d-toybox.com
push dateThu, 15 Sep 2016 14:52:16 +0000
reviewersm_kato
bugs1138159
milestone51.0a1
Bug 1138159 Don't reset IM context at selection change when there is no composition and hasn't retrieved surrounding text after last selection change r?m_kato ibus-pinyin has a bug. When application calls gtk_im_context_reset(), which means selection is changed in application, ibus-pinyin sents a set of composition signals with empty commit string. Therefore, selecting text causes removing it. For preventing it but not breaking the other IMEs which use surrounding text, we should give up to call gtk_im_context_reset() if IME hasn't retrieved surrounding text after the last selection change. Not having retrieved surrounding text means that the IME doesn't have any cache of contents. Therefore, not calling gtk_im_context_reset() at selection change must be safe for such IMEs. MozReview-Commit-ID: 5cbIZjpd7zN
widget/gtk/IMContextWrapper.cpp
widget/gtk/IMContextWrapper.h
--- a/widget/gtk/IMContextWrapper.cpp
+++ b/widget/gtk/IMContextWrapper.cpp
@@ -174,16 +174,17 @@ IMContextWrapper::IMContextWrapper(nsWin
     , mCompositionStart(UINT32_MAX)
     , mProcessingKeyEvent(nullptr)
     , mCompositionState(eCompositionState_NotComposing)
     , mIsIMFocused(false)
     , mIsDeletingSurrounding(false)
     , mLayoutChanged(false)
     , mSetCursorPositionOnKeyEvent(true)
     , mPendingResettingIMContext(false)
+    , mRetrieveSurroundingSignalReceived(false)
 {
     static bool sFirstInstance = true;
     if (sFirstInstance) {
         sFirstInstance = false;
         sUseSimpleContext =
             Preferences::GetBool(
                 "intl.ime.use_simple_context_on_password_field",
                 kUseSimpleContextDefault);
@@ -925,38 +926,43 @@ IMContextWrapper::Blur()
     mIsIMFocused = false;
 }
 
 void
 IMContextWrapper::OnSelectionChange(nsWindow* aCaller,
                                     const IMENotification& aIMENotification)
 {
     mSelection.Assign(aIMENotification);
+    bool retrievedSurroundingSignalReceived =
+      mRetrieveSurroundingSignalReceived;
+    mRetrieveSurroundingSignalReceived = false;
 
     if (MOZ_UNLIKELY(IsDestroyed())) {
         return;
     }
 
     const IMENotification::SelectionChangeDataBase& selectionChangeData =
         aIMENotification.mSelectionChangeData;
 
     MOZ_LOG(gGtkIMLog, LogLevel::Info,
         ("0x%p OnSelectionChange(aCaller=0x%p, aIMENotification={ "
          "mSelectionChangeData={ mOffset=%u, Length()=%u, mReversed=%s, "
          "mWritingMode=%s, mCausedByComposition=%s, "
          "mCausedBySelectionEvent=%s, mOccurredDuringComposition=%s "
-         "} }), mCompositionState=%s, mIsDeletingSurrounding=%s",
+         "} }), mCompositionState=%s, mIsDeletingSurrounding=%s, "
+         "mRetrieveSurroundingSignalReceived=%s",
          this, aCaller, selectionChangeData.mOffset,
          selectionChangeData.Length(),
          ToChar(selectionChangeData.mReversed),
          GetWritingModeName(selectionChangeData.GetWritingMode()).get(),
          ToChar(selectionChangeData.mCausedByComposition),
          ToChar(selectionChangeData.mCausedBySelectionEvent),
          ToChar(selectionChangeData.mOccurredDuringComposition),
-         GetCompositionStateName(), ToChar(mIsDeletingSurrounding)));
+         GetCompositionStateName(), ToChar(mIsDeletingSurrounding),
+         ToChar(retrievedSurroundingSignalReceived)));
 
     if (aCaller != mLastFocusedWindow) {
         MOZ_LOG(gGtkIMLog, LogLevel::Error,
             ("0x%p   OnSelectionChange(), FAILED, "
              "the caller isn't focused window, mLastFocusedWindow=0x%p",
              this, mLastFocusedWindow));
         return;
     }
@@ -1006,17 +1012,28 @@ IMContextWrapper::OnSelectionChange(nsWi
     }
 
     // When the selection change is caused by dispatching composition event,
     // selection set event and/or occurred before starting current composition,
     // we shouldn't notify IME of that and commit existing composition.
     if (!selectionChangeData.mCausedByComposition &&
         !selectionChangeData.mCausedBySelectionEvent &&
         !occurredBeforeComposition) {
-        ResetIME();
+        // Hack for ibus-pinyin.  ibus-pinyin will synthesize a set of
+        // composition which commits with empty string after calling
+        // gtk_im_context_reset().  Therefore, selecting text causes
+        // unexpectedly removing it.  For preventing it but not breaking the
+        // other IMEs which use surrounding text, we should call it only when
+        // surrounding text has been retrieved after last selection range was
+        // set.  If it's not retrieved, that means that current IME doesn't
+        // have any content cache, so, it must not need the notification of
+        // selection change.
+        if (IsComposing() || retrievedSurroundingSignalReceived) {
+            ResetIME();
+        }
     }
 }
 
 /* static */
 void
 IMContextWrapper::OnStartCompositionCallback(GtkIMContext* aContext,
                                              IMContextWrapper* aModule)
 {
@@ -1159,16 +1176,17 @@ IMContextWrapper::OnRetrieveSurroundingN
         return FALSE;
     }
 
     NS_ConvertUTF16toUTF8 utf8Str(nsDependentSubstring(uniStr, 0, cursorPos));
     uint32_t cursorPosInUTF8 = utf8Str.Length();
     AppendUTF16toUTF8(nsDependentSubstring(uniStr, cursorPos), utf8Str);
     gtk_im_context_set_surrounding(aContext, utf8Str.get(), utf8Str.Length(),
                                    cursorPosInUTF8);
+    mRetrieveSurroundingSignalReceived = true;
     return TRUE;
 }
 
 /* static */
 gboolean
 IMContextWrapper::OnDeleteSurroundingCallback(GtkIMContext* aContext,
                                               gint aOffset,
                                               gint aNChars,
--- a/widget/gtk/IMContextWrapper.h
+++ b/widget/gtk/IMContextWrapper.h
@@ -279,16 +279,19 @@ protected:
     bool mSetCursorPositionOnKeyEvent;
     // mPendingResettingIMContext becomes true if selection change notification
     // is received during composition but the selection change occurred before
     // starting the composition.  In such case, we cannot notify IME of
     // selection change during composition because we don't want to commit
     // the composition in such case.  However, we should notify IME of the
     // selection change after the composition is committed.
     bool mPendingResettingIMContext;
+    // mRetrieveSurroundingSignalReceived is true after "retrieve_surrounding"
+    // signal is received until selection is changed in Gecko.
+    bool mRetrieveSurroundingSignalReceived;
 
     // sLastFocusedContext is a pointer to the last focused instance of this
     // class.  When a instance is destroyed and sLastFocusedContext refers it,
     // this is cleared.  So, this refers valid pointer always.
     static IMContextWrapper* sLastFocusedContext;
 
     // sUseSimpleContext indeicates if password editors and editors with
     // |ime-mode: disabled;| should use GtkIMContextSimple.