Bug 1187566 TSFTextStore::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
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 03 Aug 2015 15:15:30 +0900
changeset 287472 d1aaf77d3988d82623b4b040c001502a807533fb
parent 287471 d4d2b60cd41218a8256a930703b82a9b0b37876d
child 287473 b40379bbe7f35429072cce42ffdbce4ad9f21d5a
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemk
bugs1187566
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1187566 TSFTextStore::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
widget/windows/TSFTextStore.cpp
widget/windows/TSFTextStore.h
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -1964,18 +1964,21 @@ TSFTextStore::LockedContent()
       return mLockedContent;
     }
 
     mLockedContent.Init(text);
   }
 
   MOZ_LOG(sTextStoreLog, LogLevel::Debug,
          ("TSF: 0x%p   TSFTextStore::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
 TSFTextStore::GetCurrentText(nsAString& aTextContent)
 {
   if (mLockedContent.IsInitialized()) {
@@ -3394,44 +3397,44 @@ TSFTextStore::GetTextExt(TsViewCookie vc
   // 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   TSFTextStore::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   TSFTextStore::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
@@ -5306,17 +5309,39 @@ TSFTextStore::Content::ReplaceTextWith(L
       // 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   TSFTextStore::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/TSFTextStore.h
+++ b/widget/windows/TSFTextStore.h
@@ -616,16 +616,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;
     }
 
     void OnLayoutChanged()
     {
       mMinTextModifiedOffset = NOT_MODIFIED;
@@ -642,16 +644,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