Bug 1061604 part.2 nsTextStore::GetTextExt() should rReturn previous …

…character rect of modified range instead of TS_E_NOLAYOUT when Google Japanese Input retrieves caret rect during composition r=emk

modules/libpref/init/all.js

@@ -3327,6 +3327,10 @@ pref("intl.tsf.hack.easy_changjei.do_not_return_no_layout_error", true);
 // 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);
+// Whether use previous character rect for the result of
+// ITfContextView::GetTextExt() if the specified range is the caret of
+// composition string.
+pref("intl.tsf.hack.google_ja_input.do_not_return_no_layout_error_at_caret", true);
 #endif
 
 // See bug 448927, on topmost panel, some IMEs are not usable on Windows.

widget/windows/nsTextStore.cpp

@@ -1097,6 +1097,7 @@ bool nsTextStore::sCreateNativeCaretForATOK = false;
 bool nsTextStore::sDoNotReturnNoLayoutErrorToFreeChangJie = false;
 bool nsTextStore::sDoNotReturnNoLayoutErrorToEasyChangjei = false;
 bool nsTextStore::sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar = false;
+bool nsTextStore::sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret = false;
 
 #define TIP_NAME_BEGINS_WITH_ATOK \
   (NS_LITERAL_STRING("ATOK "))
@@ -1111,6 +1112,13 @@ bool nsTextStore::sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar = false;
 #define TIP_NAME_GOOGLE_JA_INPUT_EN \
   (NS_LITERAL_STRING("Google Japanese Input"))
 
+static bool
+IsGoogleJapaneseInput(const nsAString& aTIPName)
+{
+  return aTIPName.Equals(TIP_NAME_GOOGLE_JA_INPUT_JA) ||
+         aTIPName.Equals(TIP_NAME_GOOGLE_JA_INPUT_EN);
+}
+
 #define TEXTSTORE_DEFAULT_VIEW (1)
 
 nsTextStore::nsTextStore()
@@ -3112,24 +3120,41 @@ nsTextStore::GetTextExt(TsViewCookie vcView,
     TSFStaticSink::GetInstance()->GetActiveTIPKeyboardDescription();
   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));
+    const Selection& currentSel = CurrentSelection();
+    if (!IsWin10OrLater()) {
+      // 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 &&
+          IsGoogleJapaneseInput(activeTIPKeyboardDescription)) {
+        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));
+      }
+      // 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 &&
+               IsGoogleJapaneseInput(activeTIPKeyboardDescription)) {
+        acpEnd = acpStart = mLockedContent.MinOffsetOfLayoutChanged();
+        PR_LOG(sTextStoreLog, PR_LOG_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
     // ITfContextView::GetTextExt() properly.  Prehaps, it's due to the bug of
@@ -4655,6 +4680,10 @@ nsTextStore::Initialize()
     Preferences::GetBool(
       "intl.tsf.hack.google_ja_input."
         "do_not_return_no_layout_error_at_first_char", true);
+  sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret =
+    Preferences::GetBool(
+      "intl.tsf.hack.google_ja_input.do_not_return_no_layout_error_at_caret",
+      true);
 
   PR_LOG(sTextStoreLog, PR_LOG_ALWAYS,
     ("TSF:   nsTextStore::Initialize(), sThreadMgr=0x%p, "
@@ -4663,13 +4692,15 @@ nsTextStore::Initialize()
      "sCreateNativeCaretForATOK=%s, "
      "sDoNotReturnNoLayoutErrorToFreeChangJie=%s, "
      "sDoNotReturnNoLayoutErrorToEasyChangjei=%s, "
-     "sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar=%s",
+     "sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar=%s, ",
+     "sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret=%s",
      sThreadMgr.get(), sClientId, sDisplayAttrMgr.get(),
      sCategoryMgr.get(), sDisabledDocumentMgr.get(), sDisabledContext.get(),
      GetBoolName(sCreateNativeCaretForATOK),
      GetBoolName(sDoNotReturnNoLayoutErrorToFreeChangJie),
      GetBoolName(sDoNotReturnNoLayoutErrorToEasyChangjei),
-     GetBoolName(sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar)));
+     GetBoolName(sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar),
+     GetBoolName(sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret)));
 }
 
 // static

widget/windows/nsTextStore.h

@@ -641,6 +641,11 @@ class nsTextStore final : public ITextStoreACP
     {
       return mInitialized && (mMinTextModifiedOffset != NOT_MODIFIED);
     }
+    // Returns minimum offset of modified text range.
+    uint32_t MinOffsetOfLayoutChanged() const
+    {
+      return mInitialized ? mMinTextModifiedOffset : NOT_MODIFIED;
+    }
 
     nsTextStore::Composition& Composition() { return mComposition; }
     nsTextStore::Selection& Selection() { return mSelection; }
@@ -792,6 +797,7 @@ class nsTextStore final : public ITextStoreACP
   static bool sDoNotReturnNoLayoutErrorToFreeChangJie;
   static bool sDoNotReturnNoLayoutErrorToEasyChangjei;
   static bool sDoNotReturnNoLayoutErrorToGoogleJaInputAtFirstChar;
+  static bool sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret;
 };
 
 #endif /*NSTEXTSTORE_H_*/