Bug 1467373 - part 2: Skip hacks for most TIPs in TSFTextStore::HackNoErrorLayoutBugs() if running on Windows 10 build 17643 or newer r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 02 Aug 2018 14:55:52 +0900
changeset 825808 322105359656cf5b49c3dc8f1b39f4e3d2727641
parent 825807 82574607f52f7ca2e64d890a5f29c1ad155872c1
push id118175
push usermasayuki@d-toybox.com
push dateThu, 02 Aug 2018 11:06:35 +0000
reviewersm_kato
bugs1467373, 17643
milestone63.0a1
Bug 1467373 - part 2: Skip hacks for most TIPs in TSFTextStore::HackNoErrorLayoutBugs() if running on Windows 10 build 17643 or newer r?m_kato At Windows 10 build 17643, Microsoft fixed the bug of TSF which returns E_FAIL to TIP when GetTextExt() returns TS_E_NOLAYOUT. With this fix, most TIPs do not have any problems even if we return TS_E_NOLAYOUT. So, unless active TIP still needs the hack, the method can skip the hack if running on build 17643 or later. Note that we still need to support Japanist 10 and Microsoft Office IME 2010. It confirmed that Japanist 10 has a bug of handling TS_E_NOLAYOUT. On the other hand, we have not tested Microsoft Office IME 2010 since it's installable only into Win7 or Win8 and needs to upgrade it to Win10 for testing, but I do not have the license. After the fix comes into release channel, I'll be able to test it though (my main environment is Win10 and it was installed before upgraded). So, we need to be back after Microsoft releases the fix. MozReview-Commit-ID: 2BzkDvHTKyI
widget/windows/TSFTextStore.cpp
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -4614,24 +4614,36 @@ TSFTextStore::MaybeHackNoErrorLayoutBugs
 
   MOZ_ASSERT(!mComposition.IsComposing() ||
              mComposition.mStart ==
                mContentForTSF.LatestCompositionStartOffset());
   MOZ_ASSERT(!mComposition.IsComposing() ||
              mComposition.EndOffset() ==
                mContentForTSF.LatestCompositionEndOffset());
 
-  bool dontReturnNoLayoutError = false;
-  const Selection& selectionForTSF = SelectionForTSFRef();
   // The bug of Microsoft Office IME 2010 for Japanese is similar to
   // MS-IME for Win 8.1 and Win 10.  Newer version of MS Office IME is not
   // released yet.  So, we can hack it without prefs  because there must be
   // no developers who want to disable this hack for tests.
   const bool kIsMSOfficeJapaneseIME2010 =
     TSFStaticSink::IsMSOfficeJapaneseIME2010Active();
+
+  // If TSF does not have the bug, we need to hack only with a few TIPs.
+  // XXX We have not tested with Microsoft Office IME 2010 since it's
+  //     installable only with Win7 and Win8 (i.e., cannot install Win8.1 and
+  //     Win10), and requires upgrade to Win10.
+  static const bool sTSFHasTheBug = !IsWindows10BuildOrLater(17643);
+  if (!sTSFHasTheBug &&
+      !kIsMSOfficeJapaneseIME2010 &&
+      !TSFStaticSink::IsJapanist10Active()) {
+    return false;
+  }
+
+  bool dontReturnNoLayoutError = false;
+  const Selection& selectionForTSF = SelectionForTSFRef();
   // MS IME for Japanese doesn't support asynchronous handling at deciding
   // its suggest list window position.  The feature was implemented
   // starting from Windows 8.  And also we may meet same trouble in e10s
   // mode on Win7.  So, we should never return TS_E_NOLAYOUT to MS IME for
   // Japanese.
   if (kIsMSOfficeJapaneseIME2010 ||
       ((TSFPrefs::DoNotReturnNoLayoutErrorToMSJapaneseIMEAtFirstChar() ||
         TSFPrefs::DoNotReturnNoLayoutErrorToMSJapaneseIMEAtCaret()) &&
@@ -4678,29 +4690,30 @@ TSFTextStore::MaybeHackNoErrorLayoutBugs
            aACPStart <= mContentForTSF.LatestCompositionEndOffset() &&
            aACPEnd >= mContentForTSF.LatestCompositionStartOffset() &&
            aACPEnd <= mContentForTSF.LatestCompositionEndOffset()) {
     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.
+  // where we know.  This is Japanist's bug.  So, even after build 17643,
+  // we need this hack.
   else if (
     TSFPrefs::DoNotReturnNoLayoutErrorToJapanist10OfCompositionString() &&
     TSFStaticSink::IsJapanist10Active() &&
     aACPStart >= mContentForTSF.LatestCompositionStartOffset() &&
     aACPStart <= mContentForTSF.LatestCompositionEndOffset() &&
     aACPEnd >= mContentForTSF.LatestCompositionStartOffset() &&
     aACPEnd <= 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.
+  // This must be caused by the bug of TSF since Free ChangJie works fine on
+  // build 17643 and later.
   else if (TSFPrefs::DoNotReturnNoLayoutErrorToFreeChangJie() &&
            TSFStaticSink::IsFreeChangJieActive()) {
     aACPEnd = mContentForTSF.LatestCompositionStartOffset();
     aACPStart = std::min(aACPStart, aACPEnd);
     dontReturnNoLayoutError = true;
   }
   // Some Chinese TIPs of Microsoft doesn't show candidate window in e10s
   // mode on Win8 or later.