[e10s] Candidate box of Chinese input method disappears after choosing part of the word

RESOLVED FIXED in Firefox 52

Status

()

defect
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: xidorn, Assigned: masayuki)

Tracking

(Depends on 1 bug, {inputmethod, regression})

Trunk
mozilla52
All
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox49 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 fixed)

Details

(Whiteboard: tpi:+)

Attachments

(6 attachments)

90.64 KB, video/mp4
Details
58 bytes, text/x-review-board-request
m_kato
: review+
Details
58 bytes, text/x-review-board-request
m_kato
: review+
Details
58 bytes, text/x-review-board-request
m_kato
: review+
Details
58 bytes, text/x-review-board-request
m_kato
: review+
Details
58 bytes, text/x-review-board-request
m_kato
: review+
Details
Posted video screencast
Please see the screencast attached.

This is on Windows 10, and the input method is Microsoft Pinyin.

It seems the target area is just a simple <textarea>.
It seems to me this happens only with e10s. I cannot reproduce it in non-e10s window.
See Also: → 1304625
Curious Xidorn, how serious is this?
Flags: needinfo?(xidorn+moz)
Keywords: regression
I would say it is serious. This is highly reproducible on certain websites. I have no idea about the exact condition for this to happen vs. bug 1304625, but I guess these two bugs may be closely related.

I got this report from some friend, and can reprodice this myself on wikipedia's plaintext editor.
Flags: needinfo?(xidorn+moz)
Hmm, I cannot reproduce this bug on debug build. Let's see the result of fixing bug 1304624.
Priority: -- → P2
Whiteboard: tpi:+
Xidorn:

Do you still reproduce this bug on latest Nightly?
Flags: needinfo?(xidorn+moz)
Yes, I still reproduce this bug on latest Nightly. I can reproduce this pretty reliably within data:text/html,<textarea> on e10s with the steps in bug 1304625. That says, I cannot really reproduce bug 1304625 because it now becomes this bug.

FWIW, I'm testing a local debug build, and I confirmed that it contains the fix from bug 1304624. And I see lots of warnings related to content cache, which may help:
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: '!GetUnionTextRects(aEvent.mInput.mOffset, aEvent.mInput.mLength, isRelativeToInsertionPoint, aEvent.mReply.mRect)', file c:/mozilla-source/central/widget/ContentCache.cpp, line 736
> [Parent 9116] WARNING: '!mContentCache.HandleQueryContentEvent(aEvent, widget)', file c:/mozilla-source/central/dom/ipc/TabParent.cpp, line 2245
> [Parent 9116] WARNING: '!event.mSucceeded', file c:/mozilla-source/central/widget/windows/TSFTextStore.cpp, line 3892
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> --DOMWINDOW == 11 (00000246A008F400) [pid = 9116] [serial = 14] [outer = 0000000000000000] [url = about:blank]
> --DOMWINDOW == 10 (0000024697E22000) [pid = 9116] [serial = 11] [outer = 0000000000000000] [url = about:blank]
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file c:/mozilla-source/central/widget/windows/WinUtils.cpp, line 1560
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: '!GetUnionTextRects(aEvent.mInput.mOffset, aEvent.mInput.mLength, isRelativeToInsertionPoint, aEvent.mReply.mRect)', file c:/mozilla-source/central/widget/ContentCache.cpp, line 736
> [Parent 9116] WARNING: '!mContentCache.HandleQueryContentEvent(aEvent, widget)', file c:/mozilla-source/central/dom/ipc/TabParent.cpp, line 2245
> [Parent 9116] WARNING: '!event.mSucceeded', file c:/mozilla-source/central/widget/windows/TSFTextStore.cpp, line 3892
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: '!GetUnionTextRects(aEvent.mInput.mOffset, aEvent.mInput.mLength, isRelativeToInsertionPoint, aEvent.mReply.mRect)', file c:/mozilla-source/central/widget/ContentCache.cpp, line 736
> [Parent 9116] WARNING: '!mContentCache.HandleQueryContentEvent(aEvent, widget)', file c:/mozilla-source/central/dom/ipc/TabParent.cpp, line 2245
> [Parent 9116] WARNING: '!event.mSucceeded', file c:/mozilla-source/central/widget/windows/TSFTextStore.cpp, line 3892
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: '!GetUnionTextRects(aEvent.mInput.mOffset, aEvent.mInput.mLength, isRelativeToInsertionPoint, aEvent.mReply.mRect)', file c:/mozilla-source/central/widget/ContentCache.cpp, line 736
> [Parent 9116] WARNING: '!mContentCache.HandleQueryContentEvent(aEvent, widget)', file c:/mozilla-source/central/dom/ipc/TabParent.cpp, line 2245
> [Parent 9116] WARNING: '!event.mSucceeded', file c:/mozilla-source/central/widget/windows/TSFTextStore.cpp, line 3892
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: '!GetUnionTextRects(aEvent.mInput.mOffset, aEvent.mInput.mLength, isRelativeToInsertionPoint, aEvent.mReply.mRect)', file c:/mozilla-source/central/widget/ContentCache.cpp, line 736
> [Parent 9116] WARNING: '!mContentCache.HandleQueryContentEvent(aEvent, widget)', file c:/mozilla-source/central/dom/ipc/TabParent.cpp, line 2245
> [Parent 9116] WARNING: '!event.mSucceeded', file c:/mozilla-source/central/widget/windows/TSFTextStore.cpp, line 3892
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart != UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 531
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
> [Parent 9116] WARNING: mCompositionStart: 'mCompositionStart == UINT32_MAX', file c:/mozilla-source/central/widget/ContentCache.cpp, line 534
Flags: needinfo?(xidorn+moz)
Thank you for your check. I'll take this. I guess that this is a bug of ContentCache with TextComposition when the composition string becomes empty temporarily.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Hardware: Unspecified → All
It seems the above try build works fine as far as I test. I'm not able to reproduce any of the three bugs I reported anymore. Thanks for fixing this!
Comment on attachment 8800515 [details]
Bug 1304620 part.1 Rename ContentCacheInParent::mIsComposing to mWidgetHasComposition

https://reviewboard.mozilla.org/r/85382/#review84060
Attachment #8800515 - Flags: review?(m_kato) → review+
Comment on attachment 8800516 [details]
Bug 1304620 part.2 ContentCacheInParent should manage if there is pending composition in the remote process

https://reviewboard.mozilla.org/r/85384/#review84070

When content process crashes during dispatching composition event, this count (including mPendingEventsNeedingAck) may keep invaild count.  But it is ignore because it is illigal case...  We need any detector if this situation (the count is invalid by crash etc) occurs.
Attachment #8800516 - Flags: review?(m_kato) → review+
(In reply to Makoto Kato [:m_kato] from comment #18)
> Comment on attachment 8800516 [details]
> Bug 1304620 part.2 ContentCacheInParent should manage if there is pending
> composition in the remote process
> 
> https://reviewboard.mozilla.org/r/85384/#review84070
> 
> When content process crashes during dispatching composition event, this
> count (including mPendingEventsNeedingAck) may keep invaild count.  But it
> is ignore because it is illigal case...  We need any detector if this
> situation (the count is invalid by crash etc) occurs.

Ah, indeed. We've already been using similar counter to notify widget/IME of content changes. So, that's important feature.
Comment on attachment 8800518 [details]
Bug 1304620 part.4 ContentCacheInParent::mCompositionStart should be set to better value for mWidgetHasComposition state

https://reviewboard.mozilla.org/r/85388/#review84080
Attachment #8800518 - Flags: review?(m_kato) → review+
Comment on attachment 8800517 [details]
Bug 1304620 part.3 The start offset of TextComposition instance in the parent process shouldn't be updated with older composition in the remote process

https://reviewboard.mozilla.org/r/85386/#review84086

Related to part 2 fix.  We will support multi-content process on Gecko, so we might have to check whether dispatching composition is current (focused) content process.  Even if content process that is dispatching composition is busy, we can change to focus that is another contennt process.  Then its dispatching count might keep 1.   But it will be feature issue...
Attachment #8800517 - Flags: review?(m_kato) → review+
Comment on attachment 8800519 [details]
Bug 1304620 part.5 ContentCacheInParent should store the latest composition start offset with mCompositionStartInChild

https://reviewboard.mozilla.org/r/85390/#review84090
Attachment #8800519 - Flags: review?(m_kato) → review+
(In reply to Makoto Kato [:m_kato] from comment #21)
> Related to part 2 fix.  We will support multi-content process on Gecko, so
> we might have to check whether dispatching composition is current (focused)
> content process.

IIRC, TabParent is created per tab (not per process) in the main process. If so, TabParent and ContentCacheInParent are created for each tab.  So, when active tab is changed, the focus move should cause committing composition at least in TabParent.  We need to check it, though.

> Even if content process that is dispatching composition is
> busy, we can change to focus that is another contennt process.  Then its
> dispatching count might keep 1.   But it will be feature issue...

So, I think that it's okay.  When it occurs, TabParent should try to commit composition in the child process and it should cause cleaning up the content cache in the parent.
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/8bea9647dc86
part.1 Rename ContentCacheInParent::mIsComposing to mWidgetHasComposition r=m_kato
https://hg.mozilla.org/integration/autoland/rev/97cb51652c86
part.2 ContentCacheInParent should manage if there is pending composition in the remote process r=m_kato
https://hg.mozilla.org/integration/autoland/rev/64f030acc66b
part.3 The start offset of TextComposition instance in the parent process shouldn't be updated with older composition in the remote process r=m_kato
https://hg.mozilla.org/integration/autoland/rev/aa2c12432274
part.4 ContentCacheInParent::mCompositionStart should be set to better value for mWidgetHasComposition state r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d3cf43813233
part.5 ContentCacheInParent should store the latest composition start offset with mCompositionStartInChild r=m_kato
Sorry, I completely forgot to request to uplift of them.

Could you test the tryserver build with Chinese IMEs?
https://archive.mozilla.org/pub/firefox/try-builds/masayuki@d-toybox.com-260950b3206c5bbad79f844ba0dea5a6935ba0e2/try-win32/
Flags: needinfo?(xidorn+moz)
Note that it's aurora build and make sure to enable e10s mode for testing Chinese IME behavior.
I think you said you didn't want to uplift those patches... But fine, I'll test that.
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #30)
> I think you said you didn't want to uplift those patches... But fine, I'll
> test that.

The fix for bug 1304624 is pretty risky but this is not so.
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #31)
> (In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #30)
> > I think you said you didn't want to uplift those patches... But fine, I'll
> > test that.
> 
> The fix for bug 1304624 is pretty risky but this is not so.

That one is actually most of people was complaining, though :)

I tested the build in comment 28, and unfortunately, I can reproduce both this bug and bug 1304625 on it (most of time this bug, sometimes the other one), so it doesn't seem to work.
Flags: needinfo?(xidorn+moz)
And I tested the nightly build again, and confirmed that both issues have been fixed there. So probably the code here depends on your fix for bug 1304624?
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #33)
> And I tested the nightly build again, and confirmed that both issues have
> been fixed there. So probably the code here depends on your fix for bug
> 1304624?

Perhaps, yes. Bug 1304624 fixed low level API, ContentEventHandler, which are used from IMEContentObserver, ContentCache and each native IME handler.  So, I couldn't imagine if this depends on the fix.

Thank you for your confirmation and sorry for not being able to uplift both of them...
Blocks: 1304625
Depends on: 1368554
You need to log in before you can comment on or make changes to this bug.