1510527 - GV with e10s sometimes commits composing string unexpectedly when using POBOX

Status: RESOLVED FIXED

(GeckoView Graveyard :: Sandboxing, defect, P1)

Description

[Makoto Kato [:m_kato]]

- Env
GV 65 (changeset a12d80e08655)
POBox (in XPERIA X Compact)

- Step
1. Run GeckoView example
2. Set focus to any <input type="text">
3. Input [あ] [か] [さ] using virtual keyboard of POBox

- Result
When inputting [さ], "あ" and "か" are committed.  This words should keep composing state like Fennec.

If not e10s, this doesn't occur.  And this doesn't occur on ATOK.  (and I guess that this doesn't also occur on Gboard).  I think that this will occurs on some IME that bug 1490391 occurs.

Comment 1

[Chris Peterson [:cpeterson]]

Makoto, does this bug only affect users who are both using Japanese language *and* using POBOX? If the Focus can limit the GV beta test to just users using languages other than Japanese, could we start the beta test without waiting for this POBOX fix?

Comment 2

[Chris Peterson [:cpeterson]]

In anticipation of the creation of the GECKOVIEW_64_RELBRANCH next week, I am moving this bug from status-firefox64=affected to status-geckoview64=affected.

Comment 3

[Makoto Kato [:m_kato]]

(In reply to Chris Peterson [:cpeterson] from comment #1)
> Makoto, does this bug only affect users who are both using Japanese language
> *and* using POBOX?

As long as I test, this occurs on POBOX by this reproduce step.  POBOX is default IME of XPERIA searies.

> If the Focus can limit the GV beta test to just users
> using languages other than Japanese, could we start the beta test without
> waiting for this POBOX fix?

Yes.

Comment 4

[Makoto Kato [:m_kato]]

12-11 16:28:25.392 13192 13754 D GeckoEditable: offer: Action(TYPE_REPLACE_TEXT)
12-11 16:28:25.393 13192 13754 D GeckoEditable: icSendComposition("さは", 0, 2)
12-11 16:28:25.393 13192 13754 D GeckoEditable:  range = 0-2
12-11 16:28:25.393 13192 13754 D GeckoEditable:  selection = 2-2
12-11 16:28:25.394 13192 13754 D GeckoEditable:  found 3 spans @ 0-1
12-11 16:28:25.395 13192 13754 D GeckoEditable:  added 4 : 7 : ff000000 : cc7fcac3
12-11 16:28:25.395 13192 13754 D GeckoEditable:  found 3 spans @ 1-2
12-11 16:28:25.396 13192 13754 D GeckoEditable:  added 4 : 7 : ff000000 : cc009688
12-11 16:28:25.400 13192 13754 D GeckoEditable: replace(0, 1, "\u3055\u306f") = GeckoEditable
12-11 16:28:25.401 13192 13754 D GeckoInputConnection: < setComposingText: true
12-11 16:28:25.405 13215 13245 I GeckoEditableSupport: IME: IME_SET_TEXT: text="さ", length=1, range=2
12-11 16:28:25.420 13215 13245 I GeckoEditableSupport: IME: NotifyIMEOfTextChange: s=0, oe=1, ne=1
12-11 16:28:25.420 13215 13245 I GeckoEditableSupport: IME: NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED
12-11 16:28:25.453 13215 13245 D GeckoEditableChild: onTextChange(さ, 0, 1, 1)

GeckoEditableSupport::DoUpdateComposition doesn't handle composing string well since it uses TextComposition that isn't updated yet...  Should we call OnImeSynchronize after icSendComposition?

Comment 5

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60309c440a0bd924ea5bd552a9ec1d8e442ee6ce

Comment 6

[Makoto Kato [:m_kato]]

Attachment: Bug 1510527 - Active composition count may be mismatched when updating composition. r?

To avoid FlushIMEChanges per updating IME composition, we calculate composition count in DoReplaceText. But when using GV+e10s, this calculation is sometimes invalid since NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED event isn't received per PendingComposition.  Because, IMEStateManager will merge this completed events due to optimization.

Also, DoUpdateComposition calls SetPendingComposition, but it doesn't touch mIMEActiveCompositionCount,

So when using some IME, this value is minus or forever non-zero on some IMEs.

So we shouldn't use atomic count. When receiving NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED, we should reset it and allow IMEFlushChanges since Gecko has already handled all IME composition events in event queues.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66e8d5fb19cb
Active composition count may be mismatched when updating composition. r=esawin

Comment 8

[Arthur Iakab [arthur_iakab]]

Backed out changeset 66e8d5fb19cb (bug 1510527) for failing rc4 tests on testInputConnection|Can backspace with shift+backspace CLOSED TREE

Backout revision https://hg.mozilla.org/integration/autoland/rev/199568e6d6b0dc56eed9c20cc35f74b17629e857

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=218080131&revision=66e8d5fb19cb0e5bb00dda9bd6d9e361ae1d44a7

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=218080131&repo=autoland

m_kato: Could you please take a look?

Comment 9

[Makoto Kato [:m_kato]]

I look this.

Comment 10

[Chris Peterson [:cpeterson]]

P1 for Focus 9.0 and Fenix

Comment 11

[Chris Peterson [:cpeterson]]

Makoto, do you have any updates on this POBOX bug?

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a464d751ed4
Active composition count may be mismatched when updating composition. r=esawin

Comment 13

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/8a464d751ed4

Comment 14

[Chris Peterson [:cpeterson]]

Makoto, after you've verified that your fix has landed and fixed Fennec and GV Nightly, can you please request uplift to 65 Beta?

The Focus team would like this fix in Focus 9.0 (using GV 65) so they can re-enable GV for Japanese Focus users.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Too late for Gecko 65 given where we are in the cycle, but we can certainly land it on a GV65 relbranch next week after the uplift :)

Comment 16

[Chris Peterson [:cpeterson]]

65=wontfix. Vesta and I talked and we don't want to uplift this IME fix to GV 65.

Comment 17

[Makoto Kato [:m_kato]]

Comment on attachment 9031657 [details]
Bug 1510527 - Active composition count may be mismatched when updating composition. r?

GeckoView Uplift Approval Request

If this is not a sec:{high,crit} bug, please state case for consideration

When using GV with e10s mode, user cannot input Japanese text well using some IME that is toggle style input.

User impact if declined

When using GV with e10s, some Japanese IME (POBOX, Google Japanese IME and etc) commits composition per 2 character.
This is only e10s mode only.

Fix Landed on Version

66

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

This issue is e10s only. Before landing this, Gecko doesn't synchronize IME data with OS's IME data correctly. It is always broken on e10s.

String or UUID changes made by this patch

no

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9031657 [details]
Bug 1510527 - Active composition count may be mismatched when updating composition. r?

Approved for GV65.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/c0786400907d (GECKOVIEW_65_RELBRANCH)

Comment 20

[Chris Peterson [:cpeterson]]

Moving some e10s bugs to the new GeckoView::Sandboxing component.