Closed Bug 1453629 Opened 2 years ago Closed Last year

Assertion failure: aCompositionEvent->mMessage != eCompositionStart running dom/base/test/chrome/test_nsITextInputProcessor.xul test

Categories

(Core :: User events and focus handling, enhancement, P3)

x86_64
Linux
enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jonco, Assigned: masayuki)

Details

Attachments

(2 files)

I get an assertion failure running this test locally with:

MOZ_HEADLESS=1 ./mach mochitest -f chrome dom/base/test/chrome/test_nsITextInputProcessor.xul

I see TEST-UNEXPECTED-FAIL but the test run is reported as passing.
Attached file log.txt
Test output.
Component: DOM → Event Handling
Priority: -- → P3
Wow, this hits really long standing bug of IME testing path.

IMEStateManager::DispatchCompositionEvent() creates TextComposition with native IME context stored by eCompositionStart event. However, when the composition ends, it looks for ending TextComposition instance with result of nsIWidget::GetNativeIMEContext() and the widget is stored by eCompositionChange(AsIs) event. The former is initialized by TextEventDispatcher::InitEvent().  When TextEventDispatcher works with TextInputProcessor, this native IME context is set to pseudo IME context which is raw pointer of TextEventDispatcher and process ID.  On the other hand, nsIWidget::GetNativeIMEContext() never returns pseudo IME context.  So, we fail to remove TextComposition when we synthesize composition for tests!
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Some points:

The patch should be reviewed by smaug, but he's on vacation. So, I request the review to Makoto-san. The idea of IME context is, it depends on platforms how many composition we can have. For example, on Windows, IME can have only one but on Linux, we can have a composition per top level window. An IME context is shared between any widgets which is associated with native IME context. It was implemented in bug 1179632:
https://bugzilla.mozilla.org/attachment.cgi?id=8695807&action=diff
https://bugzilla.mozilla.org/attachment.cgi?id=8695915&action=diff

And even with fixing this bug, I see timeout of the test with headless widget.
Comment on attachment 8992859 [details]
Bug 1453629 - nsIWidget::GetNativeIMEContext() should return pseudo IME context if TextInputProcessor has a composition on the widget

https://reviewboard.mozilla.org/r/257708/#review264884
Attachment #8992859 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/bdf8618dc2c3
nsIWidget::GetNativeIMEContext() should return pseudo IME context if TextInputProcessor has a composition on the widget r=m_kato
https://hg.mozilla.org/mozilla-central/rev/bdf8618dc2c3
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.