Last Comment Bug 724471 - [IMM] two sets of composition events are fired for a composition of Korean IME
: [IMM] two sets of composition events are fired for a composition of Korean IME
Status: RESOLVED FIXED
: inputmethod
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla13
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
Depends on:
Blocks: 12253 543789 700897
  Show dependency treegraph
 
Reported: 2012-02-05 23:21 PST by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2012-02-07 15:04 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (2.27 KB, patch)
2012-02-06 08:07 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
VYV03354: review+
Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-05 23:21:00 PST
When I commit composition string of Korean IME on Win7, by space key, it causes strange DOM composition events.

First, the composition is committed by empty string. Next, composition restarts and is committed with the latest composition string. So, web applications see two composition for an actual composition.

The cause is, nsIMM32Handler has a hack for Korean IME which was implemented by an ancient bug (forgot the bug #). Korean IME sends WM_IME_ENDCOMPOSITION first, after that, sends WM_IME_COMPOSITION. It caused doubling the composition string at committing. For preventing that, current our code dispatches an empty text event and compositionend event at WM_IME_ENDCOMPOSITION.

This strange behavior can be confirmed on Win7 too. I think that WM_IME_ENDCOMPOSITION should check the message queue if there is WM_IME_COMPOSITION. If there is, we should ignore the WM_IME_ENDCOMPOSITION.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-06 05:29:04 PST
> The cause is, nsIMM32Handler has a hack for Korean IME which was implemented by an ancient bug (forgot the bug #).

Maybe, it's bug 12253. The original code was landed for the bug and bug 17710 and bug 18286.
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsWindow.cpp&branch=&root=/cvsroot&subdir=mozilla/widget/src/windows&command=DIFF_FRAMESET&rev1=3.218&rev2=3.219
Comment 2 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-06 08:07:42 PST
Created attachment 594714 [details] [diff] [review]
Patch
Comment 3 Masatoshi Kimura [:emk] 2012-02-07 00:19:35 PST
Comment on attachment 594714 [details] [diff] [review]
Patch

> +  if (::PeekMessageW(&compositionMsg, aWindow->GetWindowHandle(),
> +                     WM_IME_STARTCOMPOSITION, WM_IME_COMPOSITION,
> +                     PM_NOREMOVE) &&
> +      compositionMsg.message == WM_IME_COMPOSITION) {
Please also test |IS_COMMITTING_LPARAM(compositionMsg.lParam)|.
r=me with this.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-02-07 05:17:50 PST
Thank you!

https://hg.mozilla.org/integration/mozilla-inbound/rev/27e074645887
Comment 5 Marco Bonardo [::mak] 2012-02-07 15:04:44 PST
https://hg.mozilla.org/mozilla-central/rev/27e074645887

Note You need to log in before you can comment on or make changes to this bug.