1302980 - iBus Korean character composition breaks in the awesome bar & the search bar

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Mark Kim [:markhkim]]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160909142649

Steps to reproduce:

1. Launch Firefox trunk on Linux Mint 18
2. Turn on the Korean IME (IBus 1.5.11)
3. Click either the awesome bar or the search bar
4. Type Korean characters in the following order: consonant, vowel, consonant, consonant, vowel. For example, ㄱㅏㅁㅅㅏ (rkatk)


Actual results:

The block structure of the second syllable breaks: 감ㅅㅏ


Expected results:

The last consonant-vowel combination should be combined into one syllable block: 감사

Comment 1

[Mark Kim [:markhkim]]

This does not happen on Firefox 48, the default version on Linux Mint 18.

Comment 2

[Loic]

Makoto, could you confirm the issue and the regression in 51?

Comment 3

[Mark Kim [:markhkim]]

mozregression tells me this:

 8:26.99 INFO: Narrowed inbound regression window from [ff3138a7, aa7f5f55] (3 revisions) to [74ed3bec, aa7f5f55] (2 revisions) (~1 steps left)
 8:26.99 INFO: Oh noes, no (more) inbound revisions :(
 8:26.99 INFO: Last good revision: 74ed3becf2d2b00f46ca48627f0badb3bc93ba4c
 8:26.99 INFO: First bad revision: aa7f5f55af9475431e200556681cca08a582c97f
 8:26.99 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=74ed3becf2d2b00f46ca48627f0badb3bc93ba4c&tochange=aa7f5f55af9475431e200556681cca08a582c97f

 8:27.60 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1280796

Comment 4

[Makoto Kato [:m_kato]]

2 issues.
- By bug 1280796, we calls OnSelectionChange even if composing.
- After bug 1184890, preedit_end might call ResetIME.

So by both, preedit_end will calls ResetIME.  But this behavior isn't correct for Korean IME.  So we have to block ResetIME by reedit_end.

Comment 5

[Makoto Kato [:m_kato]]

Attachment: Bug 1302980 - Don't reset IME context on preedit_end.

You know, when Korean IME commits string, then it sometimes set next preedit string.  So reseting context causes that next preedit string is committed.

So we shouldn't reset IME context with preedit_end.

Review commit: https://reviewboard.mozilla.org/r/79186/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79186/

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Comment on attachment 8791903 [details]
Bug 1302980 - Don't reset IME context on preedit_end.

https://reviewboard.mozilla.org/r/79186/#review77770

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4b454bdb13f
Don't reset IME context on preedit_end. r=masayuki

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b4b454bdb13f

Comment 9

[Makoto Kato [:m_kato]]

Comment on attachment 8791903 [details]
Bug 1302980 - Don't reset IME context on preedit_end.

Approval Request Comment
[Feature/regressing bug #]:
bug 1280796

[User impact if declined]:
On Linux, user cannot input korean via IME well.

[Describe test coverage new/current, TreeHerder]:
Laned in m-c

[Risks and why]: 
Low.  This issue is on IME users only.  We don't reset context with inputing composing text via IME by this change.

[String/UUID change made/needed]:
N/A

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 8791903 [details]
Bug 1302980 - Don't reset IME context on preedit_end.


Approval Request Comment
[Feature/regressing bug #]:
bug 1280796

[User impact if declined]:
On Linux, user cannot input korean via IME well.

[Describe test coverage new/current, TreeHerder]:
Laned in m-c

[Risks and why]: 
Low.  This issue is on IME users and Linux only.  We don't reset context during inputing composing text via IME by this change.

[String/UUID change made/needed]:
N/A

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello Mark, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8791903 [details]
Bug 1302980 - Don't reset IME context on preedit_end.

IME specific only, low risk, Aurora51+, Beta50+

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e5b57c31ef29

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/a4b2d61482b8

Comment 15

[Mark Kim [:markhkim]]

(In reply to Ritu Kothari (:ritu) from comment #11)
> Hello Mark, could you please verify this issue is fixed as expected on a
> latest Nightly build? Thanks!

It's fixed on 52.0a1 (2016-10-02), Linux Mint 18 Cinnamon. Thank you!