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
--- 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();
}