Bug 1435730 - part 1: Make TSFTextStore::GetTextExt() not return TS_E_NOLAYOUT error to Japanist 10 when the range is in composition string r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 06 Feb 2018 14:45:22 +0900
changeset 751947 6bf22c1e08771c65b83ba6d09524b03de4bcee1e
parent 751903 e1954b02d9e39bdb7c1f17aa95ca9cad5d5c14ae
child 751948 3c9e125c6b58d71c241aa7aaf3e249ca14064e18
push id98100
push usermasayuki@d-toybox.com
push dateWed, 07 Feb 2018 07:33:08 +0000
reviewersm_kato
bugs1435730
milestone60.0a1
Bug 1435730 - part 1: Make TSFTextStore::GetTextExt() not return TS_E_NOLAYOUT error to Japanist 10 when the range is in composition string r?m_kato Similar to ATOK, Japanist 10 requests all or part of composition string. If we return TS_E_NOLAYOUT in this case, you'll see candidate window at top-left of the screen. For avoiding this issue, we should not return TS_E_NOLAYOUT to Japanist 10 when the query range is in composition string. MozReview-Commit-ID: 2OPafUO5PQC
modules/libpref/init/all.js
widget/windows/TSFTextStore.cpp
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -3960,21 +3960,26 @@ pref("intl.tsf.support_imm", true);
 // only when active keyboard layout is a legacy IMM-IME.
 pref("intl.tsf.associate_imc_only_when_imm_ime_is_active", false);
 
 // Enables/Disables hack for specific TIP.
 
 // Whether creates native caret for ATOK or not.
 pref("intl.tsf.hack.atok.create_native_caret", true);
 // Whether use available composition string rect for result of
-// ITfContextView::GetTextExt() even if the specified range is same as the
+// ITextStoreACP::GetTextExt() even if the specified range is same as the
 // range of composition string but some character rects of them are not
 // available.  Note that this is ignored if active ATOK is or older than 2016
 // and create_native_caret is true.
 pref("intl.tsf.hack.atok.do_not_return_no_layout_error_of_composition_string", true);
+// Whether use available composition string rect for result of
+// ITextStoreACP::GetTextExt() even if the specified range is same as or is in
+// the range of composition string but some character rects of them are not
+// available.
+pref("intl.tsf.hack.japanist10.do_not_return_no_layout_error_of_composition_string", 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 Microsoft Pinyin and Microsoft Wubi
 pref("intl.tsf.hack.ms_simplified_chinese.do_not_return_no_layout_error", true);
 // For Microsoft ChangJie and Microsoft Quick
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -1091,16 +1091,17 @@ public:
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsGoogleJapaneseInputActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOKActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOK2011Active)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOK2012Active)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOK2013Active)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOK2014Active)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOK2015Active)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsATOK2016Active)
+  DECL_AND_IMPL_IS_TIP_ACTIVE(IsJapanist10Active)
 
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSBopomofoActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSChangJieActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSPhoneticActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSQuickActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSNewChangJieActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSNewPhoneticActive)
   DECL_AND_IMPL_IS_TIP_ACTIVE(IsMSNewQuickActive)
@@ -1242,18 +1243,25 @@ private:
     return mActiveTIPGUID == kGUID;
   }
 
   // * ATOK 2017
   //   - {6DBFD8F5-701D-11E6-920F-782BCBA6348F}
   // * ATOK Passport (confirmed with version 31.1.2)
   //   - {A38F2FD9-7199-45E1-841C-BE0313D8052F}
 
-  // * Japanist 10
-  //   - {E6D66705-1EDA-4373-8D01-1D0CB2D054C7}
+  bool IsJapanist10ActiveInternal() const
+  {
+    // {E6D66705-1EDA-4373-8D01-1D0CB2D054C7}
+    static const GUID kGUID = {
+      0xE6D66705, 0x1EDA, 0x4373,
+        { 0x8D, 0x01, 0x1D, 0x0C, 0xB2, 0xD0, 0x54, 0xC7 }
+    };
+    return mActiveTIPGUID == kGUID;
+  }
 
   /****************************************************************************
    * Traditional Chinese TIP
    ****************************************************************************/
 
   bool IsMSBopomofoActiveInternal() const
   {
     // {B2F9C502-1742-11D4-9790-0080C882687E} (Win8.1, Win10)
@@ -1701,16 +1709,20 @@ public:
     ShouldSetInputScopeOfURLBarToDefault, true)
   DECL_AND_IMPL_BOOL_PREF(
     "intl.tsf.hack.atok.create_native_caret",
     NeedToCreateNativeCaretForLegacyATOK, true)
   DECL_AND_IMPL_BOOL_PREF(
     "intl.tsf.hack.atok.do_not_return_no_layout_error_of_composition_string",
     DoNotReturnNoLayoutErrorToATOKOfCompositionString, true)
   DECL_AND_IMPL_BOOL_PREF(
+    "intl.tsf.hack.japanist10."
+    "do_not_return_no_layout_error_of_composition_string",
+    DoNotReturnNoLayoutErrorToJapanist10OfCompositionString, true)
+  DECL_AND_IMPL_BOOL_PREF(
     "intl.tsf.hack.ms_simplified_chinese.do_not_return_no_layout_error",
     DoNotReturnNoLayoutErrorToMSSimplifiedTIP, true)
   DECL_AND_IMPL_BOOL_PREF(
     "intl.tsf.hack.ms_traditional_chinese.do_not_return_no_layout_error",
     DoNotReturnNoLayoutErrorToMSTraditionalTIP, true)
   DECL_AND_IMPL_BOOL_PREF(
     "intl.tsf.hack.free_chang_jie.do_not_return_no_layout_error",
     DoNotReturnNoLayoutErrorToFreeChangJie, true)
@@ -3355,17 +3367,17 @@ TSFTextStore::SetSelectionInternal(const
        "SelectionForTSFRef() failure", this));
     return E_FAIL;
   }
 
   // If actually the range is not changing, we should do nothing.
   // Perhaps, we can ignore the difference change because it must not be
   // important for following edit.
   if (selectionForTSF.EqualsExceptDirection(*pSelection)) {
-    MOZ_LOG(sTextStoreLog, LogLevel::Error,
+    MOZ_LOG(sTextStoreLog, LogLevel::Warning,
       ("0x%p   TSFTextStore::SetSelectionInternal() Succeeded but "
        "did nothing because the selection range isn't changing", this));
     selectionForTSF.SetSelection(*pSelection);
     return S_OK;
   }
 
   if (mComposition.IsComposing()) {
     if (aDispatchCompositionChangeEvent) {
@@ -4248,18 +4260,30 @@ TSFTextStore::GetTextExt(TsViewCookie vc
                          LONG acpStart,
                          LONG acpEnd,
                          RECT* prc,
                          BOOL* pfClipped)
 {
   MOZ_LOG(sTextStoreLog, LogLevel::Info,
     ("0x%p TSFTextStore::GetTextExt(vcView=%ld, "
      "acpStart=%ld, acpEnd=%ld, prc=0x%p, pfClipped=0x%p), "
+     "IsHandlingComposition()=%s, "
+     "mContentForTSF={ MinOffsetOfLayoutChanged()=%u, "
+     "LatestCompositionStartOffset()=%d, LatestCompositionEndOffset()=%d }, "
+     "mComposition= { IsComposing()=%s, mStart=%d, EndOffset()=%d }, "
      "mDeferNotifyingTSF=%s, mWaitingQueryLayout=%s",
      this, vcView, acpStart, acpEnd, prc, pfClipped,
+     GetBoolName(IsHandlingComposition()),
+     mContentForTSF.MinOffsetOfLayoutChanged(),
+     mContentForTSF.HasOrHadComposition() ?
+       mContentForTSF.LatestCompositionStartOffset() : -1,
+     mContentForTSF.HasOrHadComposition() ?
+       mContentForTSF.LatestCompositionEndOffset() : -1,
+     GetBoolName(mComposition.IsComposing()),
+     mComposition.mStart, mComposition.EndOffset(),
      GetBoolName(mDeferNotifyingTSF), GetBoolName(mWaitingQueryLayout)));
 
   if (!IsReadLocked()) {
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
       ("0x%p   TSFTextStore::GetTextExt() FAILED due to "
        "not locked (read)", this));
     return TS_E_NOLOCK;
   }
@@ -4373,16 +4397,29 @@ TSFTextStore::GetTextExt(TsViewCookie vc
     else if (TSFPrefs::DoNotReturnNoLayoutErrorToATOKOfCompositionString() &&
              TSFStaticSink::IsATOKActive() &&
              (!TSFStaticSink::IsATOKReferringNativeCaretActive() ||
               !TSFPrefs::NeedToCreateNativeCaretForLegacyATOK()) &&
              mContentForTSF.LatestCompositionStartOffset() == acpStart &&
              mContentForTSF.LatestCompositionEndOffset() == acpEnd) {
       dontReturnNoLayoutError = true;
     }
+    // Japanist 10 fails to handle TS_E_NOLAYOUT when it decides the position of
+    // candidate window.  In such case, Japanist shows candidate window at
+    // top-left of the screen.  So, we should return the nearest caret rect
+    // where we know.
+    else if (
+      TSFPrefs::DoNotReturnNoLayoutErrorToJapanist10OfCompositionString() &&
+      TSFStaticSink::IsJapanist10Active() &&
+      acpStart >= mContentForTSF.LatestCompositionStartOffset() &&
+      acpStart <= mContentForTSF.LatestCompositionEndOffset() &&
+      acpEnd >= mContentForTSF.LatestCompositionStartOffset() &&
+      acpEnd <= mContentForTSF.LatestCompositionEndOffset()) {
+      dontReturnNoLayoutError = true;
+    }
     // Free ChangJie 2010 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 (TSFPrefs::DoNotReturnNoLayoutErrorToFreeChangJie() &&
              TSFStaticSink::IsFreeChangJieActive()) {
       acpEnd = mContentForTSF.LatestCompositionStartOffset();
       acpStart = std::min(acpStart, acpEnd);
       dontReturnNoLayoutError = true;