Closed Bug 1510527 Opened 1 year ago Closed 1 year ago

GV with e10s sometimes commits composing string unexpectedly when using POBOX

Categories

(GeckoView :: General, defect, P1)

Unspecified
Android
defect

Tracking

(geckoview64 wontfix, geckoview65+ fixed, geckoview66+ fixed, firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

RESOLVED FIXED
mozilla66
Tracking Status
geckoview64 --- wontfix
geckoview65 + fixed
geckoview66 + fixed
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

(Whiteboard: [geckoview:fenix:p1])

Attachments

(1 file)

- 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.
Assignee: nobody → m_kato
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?
Flags: needinfo?(m_kato)
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.
(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.
Flags: needinfo?(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?
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.
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/66e8d5fb19cb
Active composition count may be mismatched when updating composition. r=esawin
I look this.
Product: Firefox for Android → GeckoView
P1 for Focus 9.0 and Fenix
Whiteboard: [geckoview:fenix:p1]

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

Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/8a464d751ed4
Active composition count may be mismatched when updating composition. r=esawin
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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.

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 :)

Flags: needinfo?(m_kato)

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

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

Attachment #9031657 - Flags: approval-mozilla-geckoview65?

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

Approved for GV65.

Attachment #9031657 - Flags: approval-mozilla-geckoview65? → approval-mozilla-geckoview65+
You need to log in before you can comment on or make changes to this bug.