Bug 1061604 part.1 nsTextStore::GetTextExt() should rReturn previous character rect instead of TS_E_NOLAYOUT when Google Japanese Input retrieves first character of selected clause at composing r=emk
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 13 Mar 2015 21:51:00 +0900
changeset 233538 a2c1363842c7ab2c58b52e291ab90c251c31eb1c
parent 233537 6fb4932dd12eed46ec221d6ec7bc10461093ebcd
child 233539 67faef0b6c20a787e22e9085fd48dc1d54ced13e
push id28417
push userryanvm@gmail.com
push dateFri, 13 Mar 2015 19:52:44 +0000
treeherdermozilla-central@977add19414a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemk
bugs1061604
milestone39.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 1061604 part.1 nsTextStore::GetTextExt() should rReturn previous character rect instead of TS_E_NOLAYOUT when Google Japanese Input retrieves first character of selected clause at composing r=emk
modules/libpref/init/all.js
widget/windows/nsTextStore.cpp
widget/windows/nsTextStore.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -3019,16 +3019,20 @@ pref("intl.tsf.support_imm", true);
 pref("intl.tsf.hack.atok.create_native_caret", true);
 // Whether use composition start position for the result of
 // ITfContextView::GetTextExt() if the specified range is larger than
 // composition start offset.
 // For Free ChangJie 2010
 pref("intl.tsf.hack.free_chang_jie.do_not_return_no_layout_error", true);
 // For Easy Changjei
 pref("intl.tsf.hack.easy_changjei.do_not_return_no_layout_error", true);
+// Whether use previous character rect for the result of
+// ITfContextView::GetTextExt() if the specified range is the first character
+// of selected clause of composition string.
+pref("intl.tsf.hack.google_ja_input.do_not_return_no_layout_error_at_first_char", true);
 #endif
 
 // See bug 448927, on topmost panel, some IMEs are not usable on Windows.
 pref("ui.panel.default_level_parent", false);
 
 pref("mousewheel.system_scroll_override_on_root_content.enabled", true);
 
 // High resolution scrolling with supported mouse drivers on Vista or later.
--- a/widget/windows/nsTextStore.cpp
+++ b/widget/windows/nsTextStore.cpp
@@ -1155,25 +1155,30 @@ StaticRefPtr<ITfDocumentMgr> nsTextStore
 StaticRefPtr<ITfContext> nsTextStore::sDisabledContext;
 StaticRefPtr<ITfInputProcessorProfiles> nsTextStore::sInputProcessorProfiles;
 StaticRefPtr<nsTextStore> nsTextStore::sEnabledTextStore;
 DWORD nsTextStore::sClientId  = 0;
 
 bool nsTextStore::sCreateNativeCaretForATOK = false;
 bool nsTextStore::sDoNotReturnNoLayoutErrorToFreeChangJie = false;
 bool nsTextStore::sDoNotReturnNoLayoutErrorToEasyChangjei = false;
+bool nsTextStore::sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar = false;
 
 #define TIP_NAME_BEGINS_WITH_ATOK \
   (NS_LITERAL_STRING("ATOK "))
 // NOTE: Free ChangJie 2010 missspells its name...
 #define TIP_NAME_FREE_CHANG_JIE_2010 \
   (NS_LITERAL_STRING("Free CangJie IME 10"))
 #define TIP_NAME_EASY_CHANGJEI \
   (NS_LITERAL_STRING( \
      "\x4E2D\x6587 (\x7E41\x9AD4) - \x6613\x9821\x8F38\x5165\x6CD5"))
+#define TIP_NAME_GOOGLE_JA_INPUT_JA \
+  (NS_LITERAL_STRING("Google \x65E5\x672C\x8A9E\x5165\x529B"))
+#define TIP_NAME_GOOGLE_JA_INPUT_EN \
+  (NS_LITERAL_STRING("Google Japanese Input"))
 
 #define TEXTSTORE_DEFAULT_VIEW (1)
 
 nsTextStore::nsTextStore()
   : mEditCookie(0)
   , mSinkMask(0)
   , mLock(0)
   , mLockQueued(0)
@@ -3158,35 +3163,58 @@ nsTextStore::GetTextExt(TsViewCookie vcV
 
   if (acpStart < 0 || acpEnd < acpStart) {
     PR_LOG(sTextStoreLog, PR_LOG_ERROR,
            ("TSF: 0x%p   nsTextStore::GetTextExt() FAILED due to "
             "invalid position", this));
     return TS_E_INVALIDPOS;
   }
 
-  // Free ChangJie 2010 and Easy Changjei 1.0.12.0 doesn't handle
-  // ITfContextView::GetTextExt() properly.  Prehaps, it's due to a bug of TSF.
-  // 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.
-  // TODO: On Win 9, we need to check this hack is still necessary.
+  // 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 nsString& activeTIPKeyboardDescription =
     TSFStaticSink::GetInstance()->GetActiveTIPKeyboardDescription();
-  if (((sDoNotReturnNoLayoutErrorToFreeChangJie &&
-       activeTIPKeyboardDescription.Equals(TIP_NAME_FREE_CHANG_JIE_2010)) ||
-      (sDoNotReturnNoLayoutErrorToEasyChangjei &&
-       activeTIPKeyboardDescription.Equals(TIP_NAME_EASY_CHANGJEI))) &&
-      mComposition.IsComposing() &&
-      mLockedContent.IsLayoutChangedAfter(acpEnd) &&
-      mComposition.mStart < acpEnd) {
-    acpEnd = mComposition.mStart;
-    acpStart = std::min(acpStart, acpEnd);
-    PR_LOG(sTextStoreLog, PR_LOG_DEBUG,
-           ("TSF: 0x%p   nsTextStore::GetTextExt() hacked the offsets for TIP "
-            "acpStart=%d, acpEnd=%d", this, acpStart, acpEnd));
+  if (mComposition.IsComposing() && mComposition.mStart < acpEnd &&
+      mLockedContent.IsLayoutChangedAfter(acpEnd)) {
+    // 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 (!IsWin10OrLater() &&
+        !mLockedContent.IsLayoutChangedAfter(acpStart) &&
+        acpStart < acpEnd &&
+        sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar &&
+        (activeTIPKeyboardDescription.Equals(TIP_NAME_GOOGLE_JA_INPUT_JA) ||
+         activeTIPKeyboardDescription.Equals(TIP_NAME_GOOGLE_JA_INPUT_EN))) {
+      acpEnd = acpStart;
+      PR_LOG(sTextStoreLog, PR_LOG_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));
+    }
+    // Free ChangJie 2010 and Easy Changjei 1.0.12.0 doesn't handle
+    // ITfContextView::GetTextExt() properly.  Prehaps, it's due to the bug of
+    // TSF.  We need to check if this is necessary on Windows 10 before
+    // disabling this on Windows 10.
+    else if ((sDoNotReturnNoLayoutErrorToFreeChangJie &&
+              activeTIPKeyboardDescription.Equals(
+                                             TIP_NAME_FREE_CHANG_JIE_2010)) ||
+             (sDoNotReturnNoLayoutErrorToEasyChangjei &&
+              activeTIPKeyboardDescription.Equals(TIP_NAME_EASY_CHANGJEI))) {
+      acpEnd = mComposition.mStart;
+      acpStart = std::min(acpStart, acpEnd);
+      PR_LOG(sTextStoreLog, PR_LOG_DEBUG,
+             ("TSF: 0x%p   nsTextStore::GetTextExt() hacked the offsets for "
+              "TIP acpStart=%d, acpEnd=%d", this, acpStart, acpEnd));
+    }
   }
 
   if (mLockedContent.IsLayoutChangedAfter(acpEnd)) {
     PR_LOG(sTextStoreLog, PR_LOG_ERROR,
            ("TSF: 0x%p   nsTextStore::GetTextExt() FAILED due to "
             "layout not recomputed at %d", this, acpEnd));
     mPendingOnLayoutChange = true;
     return TS_E_NOLAYOUT;
@@ -4703,29 +4731,35 @@ nsTextStore::Initialize()
   sCreateNativeCaretForATOK =
     Preferences::GetBool("intl.tsf.hack.atok.create_native_caret", true);
   sDoNotReturnNoLayoutErrorToFreeChangJie =
     Preferences::GetBool(
       "intl.tsf.hack.free_chang_jie.do_not_return_no_layout_error", true);
   sDoNotReturnNoLayoutErrorToEasyChangjei =
     Preferences::GetBool(
       "intl.tsf.hack.easy_changjei.do_not_return_no_layout_error", true);
+  sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar =
+    Preferences::GetBool(
+      "intl.tsf.hack.google_ja_input."
+        "do_not_return_no_layout_error_at_first_char", true);
 
   PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
     ("TSF:   nsTextStore::Initialize(), sThreadMgr=0x%p, "
      "sClientId=0x%08X, sDisplayAttrMgr=0x%p, "
      "sCategoryMgr=0x%p, sDisabledDocumentMgr=0x%p, sDisabledContext=%p, "
      "sCreateNativeCaretForATOK=%s, "
      "sDoNotReturnNoLayoutErrorToFreeChangJie=%s, "
-     "sDoNotReturnNoLayoutErrorToEasyChangjei=%s",
+     "sDoNotReturnNoLayoutErrorToEasyChangjei=%s, "
+     "sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar=%s",
      sThreadMgr.get(), sClientId, sDisplayAttrMgr.get(),
      sCategoryMgr.get(), sDisabledDocumentMgr.get(), sDisabledContext.get(),
      GetBoolName(sCreateNativeCaretForATOK),
      GetBoolName(sDoNotReturnNoLayoutErrorToFreeChangJie),
-     GetBoolName(sDoNotReturnNoLayoutErrorToEasyChangjei)));
+     GetBoolName(sDoNotReturnNoLayoutErrorToEasyChangjei),
+     GetBoolName(sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar)));
 }
 
 // static
 void
 nsTextStore::Terminate(void)
 {
   PR_LOG(sTextStoreLog, PR_LOG_ALWAYS, ("TSF: nsTextStore::Terminate()"));
 
@@ -4873,33 +4907,44 @@ void
 nsTextStore::Content::ReplaceTextWith(LONG aStart, LONG aLength,
                                       const nsAString& aReplaceString)
 {
   MOZ_ASSERT(mInitialized);
   const nsDependentSubstring replacedString =
     GetSubstring(static_cast<uint32_t>(aStart),
                  static_cast<uint32_t>(aLength));
   if (aReplaceString != replacedString) {
-    uint32_t firstDifferentOffset =
-      static_cast<uint32_t>(aStart) + FirstDifferentCharOffset(aReplaceString,
-                                                               replacedString);
-    mMinTextModifiedOffset =
-      std::min(mMinTextModifiedOffset, firstDifferentOffset);
+    uint32_t firstDifferentOffset = mMinTextModifiedOffset;
     if (mComposition.IsComposing()) {
       // Emulate text insertion during compositions, because during a
       // composition, editor expects the whole composition string to
       // be sent in NS_COMPOSITION_CHANGE, not just the inserted part.
       // The actual NS_COMPOSITION_CHANGE will be sent in SetSelection
       // or OnUpdateComposition.
       MOZ_ASSERT(aStart >= mComposition.mStart);
       MOZ_ASSERT(aStart + aLength <= mComposition.EndOffset());
       mComposition.mString.Replace(
         static_cast<uint32_t>(aStart - mComposition.mStart),
         static_cast<uint32_t>(aLength), aReplaceString);
+      // 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);
+      }
+    } else {
+      firstDifferentOffset =
+        static_cast<uint32_t>(aStart) +
+          FirstDifferentCharOffset(aReplaceString, replacedString);
     }
+    mMinTextModifiedOffset =
+      std::min(mMinTextModifiedOffset, firstDifferentOffset);
     mText.Replace(static_cast<uint32_t>(aStart),
                   static_cast<uint32_t>(aLength), aReplaceString);
   }
   // Selection should be collapsed at the end of the inserted string.
   mSelection.CollapseAt(
     static_cast<uint32_t>(aStart) + aReplaceString.Length());
 }
 
--- a/widget/windows/nsTextStore.h
+++ b/widget/windows/nsTextStore.h
@@ -591,24 +591,28 @@ protected:
       mComposition(aComposition), mSelection(aSelection)
     {
       Clear();
     }
 
     void Clear()
     {
       mText.Truncate();
+      mLastCompositionString.Truncate();
       mInitialized = false;
     }
 
     bool IsInitialized() const { return mInitialized; }
 
     void Init(const nsAString& aText)
     {
       mText = aText;
+      if (mComposition.IsComposing()) {
+        mLastCompositionString = mComposition.mString;
+      }
       mMinTextModifiedOffset = NOT_MODIFIED;
       mInitialized = true;
     }
 
     const nsDependentSubstring GetSelectedText() const;
     const nsDependentSubstring GetSubstring(uint32_t aStart,
                                             uint32_t aLength) const;
     void ReplaceSelectedTextWith(const nsAString& aString);
@@ -638,16 +642,19 @@ protected:
       return mInitialized && (mMinTextModifiedOffset != NOT_MODIFIED);
     }
 
     nsTextStore::Composition& Composition() { return mComposition; }
     nsTextStore::Selection& Selection() { return mSelection; }
 
   private:
     nsString mText;
+    // mLastCompositionString stores the composition string when the document
+    // is locked. This is necessary to compute mMinTextModifiedOffset.
+    nsString mLastCompositionString;
     nsTextStore::Composition& mComposition;
     nsTextStore::Selection& mSelection;
 
     // The minimum offset of modified part of the text.
     enum : uint32_t
     {
       NOT_MODIFIED = UINT32_MAX
     };
@@ -779,11 +786,12 @@ protected:
 
   // TSF client ID for the current application
   static DWORD sClientId;
 
   // Enables/Disables hack for specific TIP.
   static bool sCreateNativeCaretForATOK;
   static bool sDoNotReturnNoLayoutErrorToFreeChangJie;
   static bool sDoNotReturnNoLayoutErrorToEasyChangjei;
+  static bool sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar;
 };
 
 #endif /*NSTEXTSTORE_H_*/