[e10s] Hits MOZ_ASSERT() when removing composition string of MS Pinyin on Win10

RESOLVED FIXED in Firefox 52

Status

()

Core
Event Handling
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla52
inputmethod
Points:
---

Firefox Tracking Flags

(e10s?, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

STR:

1. Type "ceshiceshi" in a textarea element in e10s mode with MS Pinyin
2. Commit a part of it (In my environment, "2" commits only "ceshi" as "测试"
3. Type Backspace key 5 times to remove remaining "ceshi".

Actual Result:
Hits MOZ_ASSERT in ContentEventHandler at removing "c".

> Assertion failure: startSet (The start of the range should've been set already), at a:/mozilla/src/dom/events/ContentEventHandler.cpp:1101
> #01: mozilla::ContentEventHandler::OnQueryTextRect (a:\mozilla\src\dom\events\contenteventhandler.cpp:2130)
> #02: mozilla::ContentEventHandler::OnQueryTextRectArray (a:\mozilla\src\dom\events\contenteventhandler.cpp:2092)
> #03: mozilla::ContentEventHandler::HandleQueryContentEvent (a:\mozilla\src\dom\events\contenteventhandler.cpp:1247)
> #04: mozilla::IMEContentObserver::HandleQueryContentEvent (a:\mozilla\src\dom\events\imecontentobserver.cpp:775)
> #05: mozilla::EventStateManager::HandleQueryContentEvent (a:\mozilla\src\dom\events\eventstatemanager.cpp:886)
> #06: mozilla::EventStateManager::PreHandleEvent (a:\mozilla\src\dom\events\eventstatemanager.cpp:610)
> #07: PresShell::HandleEventInternal (a:\mozilla\src\layout\base\nspresshell.cpp:8172)
> #08: PresShell::HandleEvent (a:\mozilla\src\layout\base\nspresshell.cpp:7898)
> #09: nsViewManager::DispatchEvent (a:\mozilla\src\view\nsviewmanager.cpp:816)
> #10: nsView::HandleEvent (a:\mozilla\src\view\nsview.cpp:1118)
> #11: mozilla::widget::PuppetWidget::DispatchEvent (a:\mozilla\src\widget\puppetwidget.cpp:356)
> #12: mozilla::ContentCacheInChild::QueryCharRectArray (a:\mozilla\src\widget\contentcache.cpp:310)
> #13: mozilla::ContentCacheInChild::CacheTextRects (a:\mozilla\src\widget\contentcache.cpp:351)
> #14: mozilla::ContentCacheInChild::CacheSelection (a:\mozilla\src\widget\contentcache.cpp:181)
> #15: mozilla::ContentCacheInChild::CacheText (a:\mozilla\src\widget\contentcache.cpp:271)
> #16: mozilla::widget::PuppetWidget::NotifyIMEOfTextChange (a:\mozilla\src\widget\puppetwidget.cpp:863)
> #17: mozilla::widget::PuppetWidget::NotifyIMEInternal (a:\mozilla\src\widget\puppetwidget.cpp:653)
> #18: nsBaseWidget::NotifyIME (a:\mozilla\src\widget\nsbasewidget.cpp:1806)
> #19: mozilla::IMEStateManager::NotifyIME (a:\mozilla\src\dom\events\imestatemanager.cpp:1358)
> #20: mozilla::IMEContentObserver::IMENotificationSender::SendTextChange (a:\mozilla\src\dom\events\imecontentobserver.cpp:1769)
> #21: mozilla::IMEContentObserver::IMENotificationSender::Run (a:\mozilla\src\dom\events\imecontentobserver.cpp:1561)
> #22: mozilla::IMEContentObserver::TryToFlushPendingNotifications (a:\mozilla\src\dom\events\imecontentobserver.cpp:1438)
> #23: PresShell::HandleEventInternal (a:\mozilla\src\layout\base\nspresshell.cpp:8211)
> #24: PresShell::HandleEvent (a:\mozilla\src\layout\base\nspresshell.cpp:7898)
> #25: nsViewManager::DispatchEvent (a:\mozilla\src\view\nsviewmanager.cpp:816)
> #26: nsView::HandleEvent (a:\mozilla\src\view\nsview.cpp:1118)
> #27: mozilla::widget::PuppetWidget::DispatchEvent (a:\mozilla\src\widget\puppetwidget.cpp:356)
> #28: mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent (a:\mozilla\src\gfx\layers\apz\util\apzccallbackhelper.cpp:471)
> #29: mozilla::dom::TabChild::RecvCompositionEvent (a:\mozilla\src\dom\ipc\tabchild.cpp:2180)
> #30: mozilla::dom::PBrowserChild::OnMessageReceived (a:\mozilla\fx-dbg\ipc\ipdl\pbrowserchild.cpp:4199)
> #31: mozilla::dom::PContentChild::OnMessageReceived (a:\mozilla\fx-dbg\ipc\ipdl\pcontentchild.cpp:6564)
> #32: mozilla::ipc::MessageChannel::DispatchAsyncMessage (a:\mozilla\src\ipc\glue\messagechannel.cpp:1664)
> #33: mozilla::ipc::MessageChannel::DispatchMessageW (a:\mozilla\src\ipc\glue\messagechannel.cpp:1603)
> #34: mozilla::ipc::MessageChannel::OnMaybeDequeueOne (a:\mozilla\src\ipc\glue\messagechannel.cpp:1570)
> #35: mozilla::detail::RunnableMethodImpl<bool (__thiscall mozilla::ipc::MessageChannel::*)(void),0,1>::Run (a:\mozilla\fx-dbg\dist\include\nsthreadutils.h:766)
> #36: mozilla::ipc::MessageChannel::DequeueTask::Run (a:\mozilla\fx-dbg\dist\include\mozilla\ipc\messagechannel.h:561)
> #37: nsThread::ProcessNextEvent (a:\mozilla\src\xpcom\threads\nsthread.cpp:1082)
> #38: NS_ProcessNextEvent (a:\mozilla\src\xpcom\glue\nsthreadutils.cpp:290)
> #39: mozilla::ipc::MessagePump::Run (a:\mozilla\src\ipc\glue\messagepump.cpp:96)
> #40: mozilla::ipc::MessagePumpForChildProcess::Run (a:\mozilla\src\ipc\glue\messagepump.cpp:301)
> #41: MessageLoop::RunInternal (a:\mozilla\src\ipc\chromium\src\base\message_loop.cc:232)
> #42: MessageLoop::RunHandler (a:\mozilla\src\ipc\chromium\src\base\message_loop.cc:226)
> #43: MessageLoop::Run (a:\mozilla\src\ipc\chromium\src\base\message_loop.cc:206)
> #44: nsBaseAppShell::Run (a:\mozilla\src\widget\nsbaseappshell.cpp:158)
> #45: nsAppShell::Run (a:\mozilla\src\widget\windows\nsappshell.cpp:264)
> #46: XRE_RunAppShell (a:\mozilla\src\toolkit\xre\nsembedfunctions.cpp:875)
> #47: mozilla::ipc::MessagePumpForChildProcess::Run (a:\mozilla\src\ipc\glue\messagepump.cpp:269)
> #48: MessageLoop::RunInternal (a:\mozilla\src\ipc\chromium\src\base\message_loop.cc:232)
> #49: MessageLoop::RunHandler (a:\mozilla\src\ipc\chromium\src\base\message_loop.cc:226)
> #50: MessageLoop::Run (a:\mozilla\src\ipc\chromium\src\base\message_loop.cc:206)
> #51: XRE_InitChildProcess (a:\mozilla\src\toolkit\xre\nsembedfunctions.cpp:709)
> #52: content_process_main (a:\mozilla\src\ipc\contentproc\plugin-container.cpp:197)
> #53: NS_internal_main (a:\mozilla\src\browser\app\nsbrowserapp.cpp:391)
> #54: wmain (a:\mozilla\src\toolkit\xre\nswindowswmain.cpp:118)
> #55: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
> #56: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x162c4]
> #57: RtlSubscribeWnfStateChangeNotification[C:\WINDOWS\SYSTEM32\ntdll.dll +0x60609]
> #58: RtlSubscribeWnfStateChangeNotification[C:\WINDOWS\SYSTEM32\ntdll.dll +0x605d4]

At removing "c", TIP generates following events:

1. eCompositionEnd for committing the "c".
2. eCompositionStart with the range which includes only "c"
3. eCompositionChange with empty string and 2 ranges.
4. eCompositionCommit

So, #2 - #4 tries to remove the "c" with composition committing with empty string.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
FYI: It hits the new NS_WARNING_ASSERTION() of part.1 even with following patches. However, it's not actual problem for now because it must hit after dispatching eCompositionUpdate but we don't do nothing after dispatching following eCompositionChange event. I read the code of TextComposition again, then, I think that we need to sort out the members like mString/mRanges vs. mLastData/mLastRanges. So, I should work on it in another bug for keeping the change purpose clear.

Updated

a year ago
tracking-e10s: --- → ?

Comment 7

a year ago
mozreview-review
Comment on attachment 8795156 [details]
Bug 1305355 part.1 TextComposition::GetSelectionStartOffset() should query normal selection range when composition string is empty

https://reviewboard.mozilla.org/r/81280/#review79994

::: dom/events/TextComposition.cpp:445
(Diff revision 1)
>    nsCOMPtr<nsIWidget> widget = mPresContext->GetRootWidget();
>    WidgetQueryContentEvent selectedTextEvent(true, eQuerySelectedText, widget);
> -  if (mRanges && mRanges->HasClauses()) {
> +  // Due to a bug of widget, mRanges may not be nullptr even though composition
> +  // string is empty.  So, we need to check it here for avoiding to return
> +  // odd start offset.
> +  if (!mLastData.IsEmpty() && mRanges && mRanges->HasClauses()) {

What would it take to change mRanges handling so that mRanges is set to null whenever mLastData is empty?
Attachment #8795156 - Flags: review?(bugs) → review+

Comment 8

a year ago
mozreview-review
Comment on attachment 8795158 [details]
Bug 1305355 part.3 IMEInputHandler shouldn't append any ranges when composition string is empty

https://reviewboard.mozilla.org/r/81284/#review80150
Attachment #8795158 - Flags: review?(m_kato) → review+

Comment 9

a year ago
mozreview-review
Comment on attachment 8795157 [details]
Bug 1305355 part.2 TSFTextStore shouldn't append any ranges when composition string is empty

https://reviewboard.mozilla.org/r/81282/#review80152
Attachment #8795157 - Flags: review?(m_kato) → review+
(Assignee)

Comment 10

a year ago
mozreview-review-reply
Comment on attachment 8795156 [details]
Bug 1305355 part.1 TextComposition::GetSelectionStartOffset() should query normal selection range when composition string is empty

https://reviewboard.mozilla.org/r/81280/#review79994

> What would it take to change mRanges handling so that mRanges is set to null whenever mLastData is empty?

mLastData is not empty means either:
* there is a non-empty composition string with clause information
* there is commit string without clause information (or only with caret information, but it won't be set with following patches)

So, in other words, there is composition string only when mLastData is not empty and there are some clause information.

Comment 11

a year ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/f4cedc85d48d
part.1 TextComposition::GetSelectionStartOffset() should query normal selection range when composition string is empty r=smaug
https://hg.mozilla.org/integration/autoland/rev/fa9da7230277
part.2 TSFTextStore shouldn't append any ranges when composition string is empty r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fb4fd0ff5215
part.3 IMEInputHandler shouldn't append any ranges when composition string is empty r=m_kato

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f4cedc85d48d
https://hg.mozilla.org/mozilla-central/rev/fa9da7230277
https://hg.mozilla.org/mozilla-central/rev/fb4fd0ff5215
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.