Closed Bug 1280182 Opened 8 years ago Closed 8 years ago

[e10s][TSF] ContentCache should return rect as far as possible if the query event is relative to insertion point

Categories

(Core :: Widget, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
e10s ? ---
firefox50 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod)

Attachments

(1 file)

Bug 1275528 will implement "insertion point relative query". It's used by native IME handlers if the input offset is relative to composition (or selection if there is no composition). In such case, IMEs don't expect that content doesn't have expected rect but it's possible in e10s mode because composition events and keyboard events are handled asynchronously.

Therefore, ContentCache should return non-empty rect as far as possible if the query event is relative to insertion point.

This fixes what TIP's windows (e.g., suggest list window) are positioned at odd position at typing composition string in TSF mode.
ContentCache may store composition string's rects which are inserted by one or more older composition event.  Even in such case, native IME, especially TIP of TSF, expects non-empty rects.

Therefore, if native IME handler uses "insertion point relative query", ContentCache should return non-empty rect as far as possible.  For example, even if query offset or range is not in its rect array of composition string, ContentCache should adjust the offset into the stored range.

Review commit: https://reviewboard.mozilla.org/r/59682/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/59682/
Attachment #8763501 - Flags: review?(m_kato)
https://reviewboard.mozilla.org/r/59682/#review57014

::: widget/ContentCache.cpp:1164
(Diff revision 1)
> +  if (aRoundToExistingOffset && startOffset > EndOffset()) {
> +    startOffset = EndOffset();
> +  }

Oops, perhaps, if |startOffset => EndOffset()|, startOffset should be |EndOffset() - 1|?

::: widget/ContentCache.cpp:1168
(Diff revision 1)
> +  if (aRoundToExistingOffset && endOffset < mStart + 1) {
> +    endOffset = mStart + 1;
> +  }

and similarly, if |endOffset < mStartOffset|, endOffset should be |mStartOffset|?
https://reviewboard.mozilla.org/r/59682/#review57014

> and similarly, if |endOffset < mStartOffset|, endOffset should be |mStartOffset|?

Er, no. This is probably correct. Only above case is buggy. (I'm still being confused.
Comment on attachment 8763501 [details]
Bug 1280182 ContentCache should return non-empty rect as far as possible if query content event is relative to insertion point

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59682/diff/1-2/
Comment on attachment 8763501 [details]
Bug 1280182 ContentCache should return non-empty rect as far as possible if query content event is relative to insertion point

https://reviewboard.mozilla.org/r/59682/#review57034
Attachment #8763501 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3c00a069236
ContentCache should return non-empty rect as far as possible if query content event is relative to insertion point r=m_kato
https://hg.mozilla.org/mozilla-central/rev/e3c00a069236
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: