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
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 13 Mar 2015 21:51:00 +0900
changeset 233539 67faef0b6c20a787e22e9085fd48dc1d54ced13e
parent 233538 a2c1363842c7ab2c58b52e291ab90c251c31eb1c
child 233540 3d6e70f93eaf2382a80904dffb52640ec4d47018
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.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
widget/windows/nsTextStore.cpp
widget/windows/nsTextStore.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -3023,16 +3023,20 @@ pref("intl.tsf.hack.atok.create_native_c
 // 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);
+// 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.
 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
@@ -1156,30 +1156,38 @@ StaticRefPtr<ITfContext> nsTextStore::sD
 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;
+bool nsTextStore::sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret = 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"))
 
+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()
   : mEditCookie(0)
   , mSinkMask(0)
   , mLock(0)
   , mLockQueued(0)
   , mLockedContent(mComposition, mSelection)
@@ -3171,34 +3179,51 @@ 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 nsString& activeTIPKeyboardDescription =
     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
     // 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)) ||
@@ -4735,31 +4760,37 @@ nsTextStore::Initialize()
       "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);
+  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, "
      "sClientId=0x%08X, sDisplayAttrMgr=0x%p, "
      "sCategoryMgr=0x%p, sDisabledDocumentMgr=0x%p, sDisabledContext=%p, "
      "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
 void
 nsTextStore::Terminate(void)
 {
   PR_LOG(sTextStoreLog, PR_LOG_ALWAYS, ("TSF: nsTextStore::Terminate()"));
 
--- a/widget/windows/nsTextStore.h
+++ b/widget/windows/nsTextStore.h
@@ -636,16 +636,21 @@ protected:
       return mInitialized && (mMinTextModifiedOffset < aOffset);
     }
     // Returns true if layout of the content has been changed, i.e., the new
     // layout has not been calculated.
     bool IsLayoutChanged() const
     {
       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; }
 
   private:
     nsString mText;
     // mLastCompositionString stores the composition string when the document
     // is locked. This is necessary to compute mMinTextModifiedOffset.
@@ -787,11 +792,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;
+  static bool sDoNotReturnNoLayoutErrorToGoogleJaInputAtCaret;
 };
 
 #endif /*NSTEXTSTORE_H_*/