Bug 1422230 - part 4: TSFTextStore::GetTextExt() should refer composition string in content r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 12 Jan 2018 15:31:04 +0900
changeset 719988 319e641c3efdf660190b4888c846873ba96a5ef2
parent 719987 bab5a287202d52da592c65aecd87ed8bbd359a2b
child 745948 f4734a56b5d429e85e1289dbc68273af10b0fc1a
push id95417
push usermasayuki@d-toybox.com
push dateSat, 13 Jan 2018 00:49:22 +0000
reviewersm_kato
bugs1422230
milestone59.0a1
Bug 1422230 - part 4: TSFTextStore::GetTextExt() should refer composition string in content r?m_kato Currently, TSFTextStore::GetTextExt() refers mComposition for doing its own hack. However, this means that it refers composition in TIP. However, query event is computed with content information. So, even if TSFTextStore dispatched eCompositionCommit event, it may not be handled by content yet. In this case, we need information relative to last composition string. So, TSFTextStore::GetTextExt() should refer IsHandlingComposition() and last composition string information stored by mContentForTSF. MozReview-Commit-ID: KMqrDmnUldU
widget/windows/TSFTextStore.cpp
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -4275,24 +4275,41 @@ TSFTextStore::GetTextExt(TsViewCookie vc
     MOZ_LOG(sTextStoreLog, LogLevel::Error,
       ("0x%p   TSFTextStore::GetTextExt() FAILED due to "
        "invalid position", this));
     return TS_E_INVALIDPOS;
   }
 
   mWaitingQueryLayout = false;
 
-  // 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.
+  // When ITextStoreACP::GetTextExt() returns TS_E_NOLAYOUT, TSF returns E_FAIL
+  // to its caller (typically, active TIP).  Then, most TIPs abort current job
+  // or treat such application as non-GUI apps.  E.g., some of them give up
+  // showing candidate window, some others show candidate window at top-left of
+  // the screen.  For avoiding this issue, when there is composition (until
+  // composition is actually committed in remote content), we should not
+  // return TS_E_NOLAYOUT error for TIPs whose some features are broken by
+  // this issue.
+  // Note that ideally, this issue should be avoided by each TIP since this
+  // won't be fixed at least on non-latest Windows.  Actually, Google Japanese
+  // Input (based on Mozc) does it.  When GetTextExt() returns E_FAIL, TIPs
+  // should try to check result of GetRangeFromPoint() because TSF returns
+  // TS_E_NOLAYOUT correctly in this case. See:
+  // https://github.com/google/mozc/blob/6b878e31fb6ac4347dc9dfd8ccc1080fe718479f/src/win32/tip/tip_range_util.cc#L237-L257
 
   bool dontReturnNoLayoutError = false;
 
-  if (mComposition.IsComposing() && mComposition.mStart < acpEnd &&
+  if (IsHandlingComposition() && mContentForTSF.HasOrHadComposition() &&
       mContentForTSF.IsLayoutChangedAt(acpEnd)) {
+    MOZ_ASSERT(!mComposition.IsComposing() ||
+               mComposition.mStart ==
+                 mContentForTSF.LatestCompositionStartOffset());
+    MOZ_ASSERT(!mComposition.IsComposing() ||
+               mComposition.EndOffset() ==
+                 mContentForTSF.LatestCompositionEndOffset());
     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();
     // MS IME for Japanese doesn't support asynchronous handling at deciding
@@ -4345,40 +4362,39 @@ TSFTextStore::GetTextExt(TsViewCookie vc
     //     refers native caret rect on windows whose window class is one of
     //     Mozilla window classes and we stop creating native caret for ATOK
     //     because creating native caret causes ATOK refers caret position
     //     when GetTextExt() returns TS_E_NOLAYOUT.
     else if (TSFPrefs::DoNotReturnNoLayoutErrorToATOKOfCompositionString() &&
              TSFStaticSink::IsATOKActive() &&
              (!TSFStaticSink::IsATOKReferringNativeCaretActive() ||
               !TSFPrefs::NeedToCreateNativeCaretForLegacyATOK()) &&
-             mComposition.mStart == acpStart &&
-             mComposition.EndOffset() == acpEnd) {
+             mContentForTSF.LatestCompositionStartOffset() == acpStart &&
+             mContentForTSF.LatestCompositionEndOffset() == acpEnd) {
       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 = mComposition.mStart;
+      acpEnd = mContentForTSF.LatestCompositionStartOffset();
       acpStart = std::min(acpStart, acpEnd);
       dontReturnNoLayoutError = true;
     }
     // Some Chinese TIPs of Microsoft doesn't show candidate window in e10s
     // mode on Win8 or later.
-    else if (
-      IsWin8OrLater() &&
-      ((TSFPrefs::DoNotReturnNoLayoutErrorToMSTraditionalTIP() &&
-        (TSFStaticSink::IsMSChangJieActive() ||
-         TSFStaticSink::IsMSQuickActive())) ||
-       (TSFPrefs::DoNotReturnNoLayoutErrorToMSSimplifiedTIP() &&
-         (TSFStaticSink::IsMSPinyinActive() ||
-          TSFStaticSink::IsMSWubiActive())))) {
-      acpEnd = mComposition.mStart;
+    else if (IsWin8OrLater() &&
+             ((TSFPrefs::DoNotReturnNoLayoutErrorToMSTraditionalTIP() &&
+               (TSFStaticSink::IsMSChangJieActive() ||
+                TSFStaticSink::IsMSQuickActive())) ||
+             (TSFPrefs::DoNotReturnNoLayoutErrorToMSSimplifiedTIP() &&
+               (TSFStaticSink::IsMSPinyinActive() ||
+                TSFStaticSink::IsMSWubiActive())))) {
+      acpEnd = mContentForTSF.LatestCompositionStartOffset();
       acpStart = std::min(acpStart, acpEnd);
       dontReturnNoLayoutError = true;
     }
 
     // If we hack the queried range for active TIP, that means we should not
     // return TS_E_NOLAYOUT even if hacked offset is still modified.  So, as
     // far as possible, we should adjust the offset.
     if (dontReturnNoLayoutError) {
@@ -4396,18 +4412,18 @@ TSFTextStore::GetTextExt(TsViewCookie vc
       // useful because it may return caret rect or old character's rect which
       // the user still see.  That must be useful information for TIP.
       int32_t firstModifiedOffset =
         static_cast<int32_t>(mContentForTSF.MinOffsetOfLayoutChanged());
       LONG lastUnmodifiedOffset = std::max(firstModifiedOffset - 1, 0);
       if (mContentForTSF.IsLayoutChangedAt(acpStart)) {
         // If TSF queries text rect in composition string, we should return
         // rect at start of the composition even if its layout is changed.
-        if (acpStart >= mComposition.mStart) {
-          acpStart = mComposition.mStart;
+        if (acpStart >= mContentForTSF.LatestCompositionStartOffset()) {
+          acpStart = mContentForTSF.LatestCompositionStartOffset();
         }
         // Otherwise, use first character's rect.  Even if there is no
         // characters, the query event will return caret rect instead.
         else {
           acpStart = lastUnmodifiedOffset;
         }
         MOZ_ASSERT(acpStart <= acpEnd);
       }
@@ -4448,16 +4464,22 @@ TSFTextStore::GetTextExt(TsViewCookie vc
   int64_t startOffset = acpStart;
   if (mComposition.IsComposing()) {
     // If there is a composition, TSF must want character rects related to
     // the composition.  Therefore, we should use insertion point relative
     // query because the composition might be at different position from
     // the position where TSFTextStore believes it at.
     options.mRelativeToInsertionPoint = true;
     startOffset -= mComposition.mStart;
+  } else if (IsHandlingComposition() && mContentForTSF.HasOrHadComposition()) {
+    // If there was a composition and it hasn't been committed in the content
+    // yet, ContentCacheInParent is still open for relative offset query from
+    // the latest composition.
+    options.mRelativeToInsertionPoint = true;
+    startOffset -= mContentForTSF.LatestCompositionStartOffset();
   } else if (!CanAccessActualContentDirectly()) {
     // If TSF/TIP cannot access actual content directly, there may be pending
     // text and/or selection changes which have not been notified TSF yet.
     // Therefore, we should use relative to insertion point query since
     // TSF/TIP computes the offset from the cached selection.
     options.mRelativeToInsertionPoint = true;
     startOffset -= mSelectionForTSF.StartOffset();
   }