[e10s][Cocoa] Google Japanese IME's suggest window is positioned at wrong position if current composition was started with committing previous composition

RESOLVED FIXED in Firefox 56

Status

()

RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla56
x86_64
Mac OS X
inputmethod
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(2 attachments)

Similar to bug 1376407, Google Japanese IME's suggest window is positioned at previous composition if current composition is started with committing the previous composition.

STR:
0. Set key assign of Google Japanese Input is ATOK.
1. Type something in an editor in e10s mode and convert the word (but do no commit it).
2. Type new string.

ER:
Suggest window of Google Japanese Input should be positioned proper position.

AR:
It's positioned based on the previous composition offset.

> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::DispatchCompositionCommitEvent, aCommitString=0x0x7fff5e062a60 ("あい"), Destroyed()=FALSE, mView=0x10e6f0130, mWidget=0x120909c00, inputContext=0x1090bc7a0, mIsIMEComposing=TRUE
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::SelectedRange, Destroyed()=FALSE, mSelectedRange={ location=2, length=0 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::HasMarkedText, mMarkedRange={ location=9223372036854775807, length=0 }
> ...
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::SetMarkedText, aAttrString="う", aSelectedRange={ location=1, length=0 }, aReplacementRange=0x7fff5e061af0 { location=9223372036854775807, length=0 }, Destroyed()=FALSE, IgnoreIMEComposition()=FALSE, IsIMEComposing()=FALSE, mMarkedRange={ location=9223372036854775807, length=0 }, keyevent=0x1c2572840, keydownHandled=FALSE, keypressDispatched=FALSE, causedOtherKeyEvents=FALSE, compositionDispatched=TRUE
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::SelectedRange, Destroyed()=FALSE, mSelectedRange={ location=2, length=0 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::DispatchCompositionStartEvent, mSelectedRange={ location=2, length=0 }, Destroyed()=FALSE, mView=0x10e6f0130, mWidget=0x120909c00, inputContext=0x1090bc7a0, mIsIMEComposing=FALSE
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::SelectedRange, Destroyed()=FALSE, mSelectedRange={ location=2, length=0 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::DispatchCompositionChangeEvent, aText="う", aAttrString="う", aSelectedRange={ location=1, length=0 }, Destroyed()=FALSE, mView=0x10e6f0130, mWidget=0x120909c00, inputContext=0x1090bc7a0, mIsIMEComposing=TRUE
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::GetRangeCount, aAttrString="う", count=1
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::ConvertToTextRangeType, aUnderlineStyle=1, aSelectedRange.length=0,
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::CreateTextRangeArray, range={ mStartOffset=0, mEndOffset=1, mRangeType=TextRangeType::eRawClause }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::CreateTextRangeArray, range={ mStartOffset=1, mEndOffset=1, mRangeType=TextRangeType::eCaret }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 TextInputHandler::HandleKeyDownEvent, called interpretKeyEvents
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 TextInputHandler::HandleKeyDownEvent, wasComposing=TRUE, IsIMEComposing()=TRUE
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 TextInputHandler::HandleKeyDownEvent, keydown handled=FALSE, keypress handled=FALSE, causedOtherKeyEvents=FALSE, compositionDispatched=TRUE
> [Main Thread]: I/TextInputHandlerWidgets 
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::HasMarkedText, mMarkedRange={ location=2, length=1 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::MarkedRange, mMarkedRange={ location=2, length=1 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::HasMarkedText, mMarkedRange={ location=2, length=1 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::FirstRectForCharacterRange, Destroyed()=FALSE, aRange={ location=2, length=1 }, aActualRange=0x7fff5e0657c8 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::FirstRectForCharacterRange, useCaretRect=FALSE rect={ x=694.500000, y=405.500000, width=13.000000, height=16.000000 }, actualRange={ location=0, length=1 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::FirstRectForCharacterRange, Destroyed()=FALSE, aRange={ location=2, length=1 }, aActualRange=0x7fff5e0654e0 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::FirstRectForCharacterRange, useCaretRect=FALSE rect={ x=694.500000, y=405.500000, width=13.000000, height=16.000000 }, actualRange={ location=0, length=1 }
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::GetAttributedSubstringFromRange, aRange={ location=2, length=1 }, aActualRange=0x7fff5e0653f8, Destroyed()=FALSE
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::GetAttributedSubstringFromRange, computed with mIMECompositionString (result string="な")
> [Main Thread]: I/TextInputHandlerWidgets 0x10e698a20 IMEInputHandler::OnSelectionChange

Different from bug 1376407, IMEInputHandler stores and modifies its selection cache as I expected. However, FirstRectForCharacterRange() is called too early for the main process.

I *guess* that in this case, ContentCache can know the next character rect of previous composition string (even if there is no character). If so, we should return the rect for FirstRectForCharacterRange().
Depends on: 1376424
Summary: [e10s][Cocoa] Google Japanese IME's suggest window is positioned at wrong position if current composition is started with committing previous composition → [e10s][Cocoa] Google Japanese IME's suggest window is positioned at wrong position if current composition was started with committing previous composition
I found invalidateCharacterCoordinates of NSTextInputContext.
https://developer.apple.com/documentation/appkit/nstextinputcontext/1535165-invalidatecharactercoordinates?language=objc

We should call this at every NOTIFY_IME_OF_POSITION_CHANGE.  However, even if we do so, position of candidate window isn't modified by any IMEs. Additionally, native applications and Safari doesn't update the position too. It seems that macOS doesn't update candidate window position when invalidateCharacterCoordinates is called.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8882149 [details]
Bug 1376417 - part1: IMEInputHandler should notify Cocoa of layout change with [NSTextInputContext invalidateCharacterCoordinates]

https://reviewboard.mozilla.org/r/153276/#review158434

Humm, WebKit and Blink don't use it.  Although nyaruru-san tries it for http://crbug.com/580808 on blink, no range request after it.
But NSTextView seems to call it, it might be useful.
Attachment #8882149 - Flags: review?(m_kato) → review+
(In reply to Makoto Kato [:m_kato] from comment #5)
> no range request after it.

Yeah, I experimented it too. I reported a bug to Apple (id:33048731) that invalidateCharacterCoordinates should retry to get position with markedRange or selectedRange and firstRectForCharacterRect. I guess that Cocoa developers don't know about this issue because this is hard to report and check. If we'll do this, Apple can check the symptom with Nightly (or Firefox 56~) easier.

Comment 7

2 years ago
mozreview-review
Comment on attachment 8882150 [details]
Bug 1376417 - part2: ContentCache should adjust composition start offset when querying content with relative offset

https://reviewboard.mozilla.org/r/153278/#review158460
Attachment #8882150 - Flags: review?(m_kato) → review+
Status: NEW → ASSIGNED

Comment 8

2 years ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/4b627ad5d95a
part1: IMEInputHandler should notify Cocoa of layout change with [NSTextInputContext invalidateCharacterCoordinates] r=m_kato
https://hg.mozilla.org/integration/autoland/rev/55ffef6341b7
part2: ContentCache should adjust composition start offset when querying content with relative offset r=m_kato

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4b627ad5d95a
https://hg.mozilla.org/mozilla-central/rev/55ffef6341b7
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1418747
You need to log in before you can comment on or make changes to this bug.