Bug 1187566 - nsTextStore::Content should compute mMinTextModified Offset only with the latest composition string and original composition string, and also the hack should be enabled on Win10. r=emk, a=ritu
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 03 Aug 2015 15:15:30 +0900
changeset 269175 905fc8e8c2bc7bddf114e44813c7ffd2cbb32742
parent 269174 ba9b666f6429fd945fcaa1ea33c92795a58fd62e
child 269176 0bbd19cd784a51359c03200acb21dc2d508ced9a
push id4932
push userjlund@mozilla.com
push dateMon, 10 Aug 2015 18:23:06 +0000
treeherdermozilla-esr52@6dd5a4f5f745 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemk, ritu
bugs1187566
milestone41.0a2
Bug 1187566 - nsTextStore::Content should compute mMinTextModified Offset only with the latest composition string and original composition string, and also the hack should be enabled on Win10. r=emk, a=ritu
widget/windows/nsTextStore.cpp
widget/windows/nsTextStore.h
--- a/widget/windows/nsTextStore.cpp
+++ b/widget/windows/nsTextStore.cpp
@@ -1924,18 +1924,21 @@ nsTextStore::LockedContent()
       return mLockedContent;
     }
 
     mLockedContent.Init(text);
   }
 
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
          ("TSF: 0x%p   nsTextStore::LockedContent(): "
-          "mLockedContent={ mText.Length()=%d }",
-          this, mLockedContent.Text().Length()));
+          "mLockedContent={ mText.Length()=%d, mLastCompositionString=\"%s\", "
+          "mMinTextModifiedOffset=%u }",
+          this, mLockedContent.Text().Length(),
+          NS_ConvertUTF16toUTF8(mLockedContent.LastCompositionString()).get(),
+          mLockedContent.MinTextModifiedOffset()));
 
   return mLockedContent;
 }
 
 bool
 nsTextStore::GetCurrentText(nsAString& aTextContent)
 {
   if (mLockedContent.IsInitialized()) {
@@ -3353,44 +3356,44 @@ nsTextStore::GetTextExt(TsViewCookie vcV
   // NOTE: TSF (at least on Win 8.1) doesn't return TS_E_NOLAYOUT to the
   // caller even if we return it.  It's converted to just E_FAIL.
   // However, this is fixed on Win 10.
 
   const TSFStaticSink* kSink = TSFStaticSink::GetInstance();
   if (mComposition.IsComposing() && mComposition.mStart < acpEnd &&
       mLockedContent.IsLayoutChangedAfter(acpEnd)) {
     const Selection& currentSel = CurrentSelection();
-    if (!IsWin10OrLater()) {
+    if ((sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar ||
+         sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret) &&
+        kSink->IsGoogleJapaneseInputActive()) {
       // Google Japanese Input doesn't handle ITfContextView::GetTextExt()
       // properly due to the same bug of TSF mentioned above.  Google Japanese
       // Input calls this twice for the first character of changing range of
       // composition string and the caret which is typically at the end of
       // composition string.  The formar is used for showing candidate window.
       // This is typically shown at wrong position.  We should avoid only this
       // case. This is not necessary on Windows 10.
-      if (!mLockedContent.IsLayoutChangedAfter(acpStart) &&
-          acpStart < acpEnd &&
-          sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar &&
-          kSink->IsGoogleJapaneseInputActive()) {
+      if (sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar &&
+          !mLockedContent.IsLayoutChangedAfter(acpStart) &&
+          acpStart < acpEnd) {
         acpEnd = acpStart;
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
                ("TSF: 0x%p   nsTextStore::GetTextExt() hacked the offsets of "
                 "the first character of changing range of the composition "
                 "string for TIP acpStart=%d, acpEnd=%d",
                 this, acpStart, acpEnd));
       }
       // Google Japanese Input sometimes uses caret position for deciding its
       // candidate window position. In such case, we should return the previous
       // offset of selected clause. However, it's difficult to get where is
       // selected clause for now.  Instead, we should use the first character
       // which is modified. This is useful in most cases.
-      else if (acpStart == acpEnd &&
-               currentSel.IsCollapsed() && currentSel.EndOffset() == acpEnd &&
-               sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret &&
-               kSink->IsGoogleJapaneseInputActive()) {
+      else if (sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret &&
+               acpStart == acpEnd &&
+               currentSel.IsCollapsed() && currentSel.EndOffset() == acpEnd) {
         acpEnd = acpStart = mLockedContent.MinOffsetOfLayoutChanged();
         MOZ_LOG(sTextStoreLog, LogLevel::Debug,
                ("TSF: 0x%p   nsTextStore::GetTextExt() hacked the offsets of "
                 "the caret of the composition string for TIP acpStart=%d, "
                 "acpEnd=%d", this, acpStart, acpEnd));
       }
     }
     // Free ChangJie 2010 and Easy Changjei 1.0.12.0 doesn't handle
@@ -5219,17 +5222,39 @@ nsTextStore::Content::ReplaceTextWith(LO
       // TIP may set composition string twice or more times during a document
       // lock.  Therefore, we should compute the first difference offset with
       // mLastCompositionString.
       if (mComposition.mString != mLastCompositionString) {
         firstDifferentOffset =
           mComposition.mStart +
             FirstDifferentCharOffset(mComposition.mString,
                                      mLastCompositionString);
+        // The previous change to the composition string is canceled.
+        if (mMinTextModifiedOffset >=
+              static_cast<uint32_t>(mComposition.mStart) &&
+            mMinTextModifiedOffset < firstDifferentOffset) {
+          mMinTextModifiedOffset = firstDifferentOffset;
+        }
+      } else if (mMinTextModifiedOffset >=
+                   static_cast<uint32_t>(mComposition.mStart) &&
+                 mMinTextModifiedOffset <
+                   static_cast<uint32_t>(mComposition.EndOffset())) {
+        // The previous change to the composition string is canceled.
+        mMinTextModifiedOffset = firstDifferentOffset =
+          mComposition.EndOffset();
       }
+      MOZ_LOG(sTextStoreLog, LogLevel::Debug,
+        ("TSF: 0x%p   nsTextStore::Content::ReplaceTextWith(aStart=%d, "
+         "aLength=%d, aReplaceString=\"%s\"), mComposition={ mStart=%d, "
+         "mString=\"%s\" }, mLastCompositionString=\"%s\", "
+         "mMinTextModifiedOffset=%u, firstDifferentOffset=%u",
+         this, aStart, aLength, NS_ConvertUTF16toUTF8(aReplaceString).get(),
+         mComposition.mStart, NS_ConvertUTF16toUTF8(mComposition.mString).get(),
+         NS_ConvertUTF16toUTF8(mLastCompositionString).get(),
+         mMinTextModifiedOffset, firstDifferentOffset));
     } else {
       firstDifferentOffset =
         static_cast<uint32_t>(aStart) +
           FirstDifferentCharOffset(aReplaceString, replacedString);
     }
     mMinTextModifiedOffset =
       std::min(mMinTextModifiedOffset, firstDifferentOffset);
     mText.Replace(static_cast<uint32_t>(aStart),
--- a/widget/windows/nsTextStore.h
+++ b/widget/windows/nsTextStore.h
@@ -619,16 +619,18 @@ protected:
 
     bool IsInitialized() const { return mInitialized; }
 
     void Init(const nsAString& aText)
     {
       mText = aText;
       if (mComposition.IsComposing()) {
         mLastCompositionString = mComposition.mString;
+      } else {
+        mLastCompositionString.Truncate();
       }
       mMinTextModifiedOffset = NOT_MODIFIED;
       mInitialized = true;
     }
 
     const nsDependentSubstring GetSelectedText() const;
     const nsDependentSubstring GetSubstring(uint32_t aStart,
                                             uint32_t aLength) const;
@@ -640,16 +642,26 @@ protected:
                           bool aPreserveSelection);
     void EndComposition(const PendingAction& aCompEnd);
 
     const nsString& Text() const
     {
       MOZ_ASSERT(mInitialized);
       return mText;
     }
+    const nsString& LastCompositionString() const
+    {
+      MOZ_ASSERT(mInitialized);
+      return mLastCompositionString;
+    }
+    uint32_t MinTextModifiedOffset() const
+    {
+      MOZ_ASSERT(mInitialized);
+      return mMinTextModifiedOffset;
+    }
 
     // Returns true if layout of the character at the aOffset has not been
     // calculated.
     bool IsLayoutChangedAfter(uint32_t aOffset) const
     {
       return mInitialized && (mMinTextModifiedOffset < aOffset);
     }
     // Returns true if layout of the content has been changed, i.e., the new