New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758443 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 744779



Sign in to add a comment

backspace on autocomplete text moves cursor before the last user typed character

Project Member Reported by changwan@chromium.org, Aug 24 2017

Issue description

Chrome Version: 62
OS: Android

What steps will reproduce the problem?
(1) Use Google Japanese Keyboard
(2) Go to omnibox.
(3) Type google.com and type enter twice to submit it.
(4) Go back to omnibox.
(5) Type 'go' and see autocomplete text at the end.
(6) Press the backspace key.

What is the expected result?
The autocomplete text 'ogle.com' gets deleted and cursor remains at the end of 'go'.

What happens instead?
The autocomplete text gets deleted, but the cursor moves between 'g' and 'o'.

 
I've tried the following Japanese keyboards:
1) Google Japanese Keyboard
2) ATOK trial
3) Simeji

And they seem to exhibit the same behavior, which is as follows:
1) User types 'go'.
2) Chrome puts 'ogle.com' at the end.
3) User presses backspace.
4) Chrome detects the text change from 'go' to 'g', and restores 'o', while removing the autocomplete text. Now the composition is 'g', and 'o' is not underlined. But the cursor is at the end of 'o'. g[o]_
5) Keyboard app calls setComposingText('g', 1).
6) Now the cursor moves to the end of 'g' because the cursor position '1' means at the end of the composition text. g_[o]

where _ means cursor position, [] means autocomplete text.

The problem with the above sequence is that Japanese keyboard apps do not finish composing text when text is inserted (pasted). I think Japanese keyboard's composition behavior is very distinctive and does not allow composition to be interrupted even for events like text paste, etc. I'll think of a workaround.
Status: Started (was: Assigned)
Ok, one idea is to call finishComposingText() in restoreBackspacedText(). So far it works on the following Japanese Keyboards:

Google Japanese Keyboard
ATOK trial
Simeji

I'll try this on a more extensive set of keyboard apps tomorrow.

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08ed69a327a50f7712b348ba9bf6df466f4fce5d

commit 08ed69a327a50f7712b348ba9bf6df466f4fce5d
Author: Changwan Ryu <changwan@chromium.org>
Date: Wed Sep 06 23:42:52 2017

Fix for Japanese keyboards

Japanese keyboards do not finish composition when we restore the deleted
text. Then some keyboards call setComposingText([text], 1) again which
moves the cursor before the restored text, and some keyboards do not
respond at all and therefore later composition change moves the cursor
before the restored text.

It seems that all the keyboard apps I tested calmly respond to the
finishComposingText() call, probably because it gets the call in various
situations such as focus loss.

BUG= 758443 

Change-Id: I752d9d86053a923d7ffc4ef248d088ca991aeae1
Reviewed-on: https://chromium-review.googlesource.com/636107
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Alexandre Elias <aelias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500122}
[modify] https://crrev.com/08ed69a327a50f7712b348ba9bf6df466f4fce5d/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java
[modify] https://crrev.com/08ed69a327a50f7712b348ba9bf6df466f4fce5d/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

Labels: Merge-Request-62
requesting merge of #5 into m62
Labels: -ReleaseBlock-Stable ReleaseBlock-Beta
Project Member

Comment 8 by sheriffbot@chromium.org, Sep 7 2017

Labels: -Merge-Request-62 Hotlist-Merge-Approved Merge-Approved-62
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 8 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b9f09c1e32c9d64069d463e2c6332a5e42d3d61

commit 7b9f09c1e32c9d64069d463e2c6332a5e42d3d61
Author: Changwan Ryu <changwan@chromium.org>
Date: Fri Sep 08 00:03:13 2017

Fix for Japanese keyboards

Japanese keyboards do not finish composition when we restore the deleted
text. Then some keyboards call setComposingText([text], 1) again which
moves the cursor before the restored text, and some keyboards do not
respond at all and therefore later composition change moves the cursor
before the restored text.

It seems that all the keyboard apps I tested calmly respond to the
finishComposingText() call, probably because it gets the call in various
situations such as focus loss.

BUG= 758443 

(cherry picked from commit 08ed69a327a50f7712b348ba9bf6df466f4fce5d)

Change-Id: I752d9d86053a923d7ffc4ef248d088ca991aeae1
Reviewed-on: https://chromium-review.googlesource.com/636107
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Alexandre Elias <aelias@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#500122}
Reviewed-on: https://chromium-review.googlesource.com/656515
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#74}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/7b9f09c1e32c9d64069d463e2c6332a5e42d3d61/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java
[modify] https://crrev.com/7b9f09c1e32c9d64069d463e2c6332a5e42d3d61/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

Status: Fixed (was: Started)
Status: Started (was: Fixed)
I found a case where this doesn't work. Working on another fix.
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad438915cdcc0cdac9a0badd3a4f74a43a2a99a5

commit ad438915cdcc0cdac9a0badd3a4f74a43a2a99a5
Author: Changwan Ryu <changwan@chromium.org>
Date: Tue Sep 12 21:35:36 2017

Finish composition first for Japanese keyboard

In my previous attempt to fix Japanese keyboard behavior
(https://chromium-review.googlesource.com/636107),
I was finishing composition text *after* restoring deleted text.
This works on most Japanese keyboards, but Google Japanese keyboard
behaves weirdly, as it calls setComposingText([text], 1) again which
moves the cursor position.

This can be fixed by finishing composition first, and then restoring
text.

BUG= 758443 

Change-Id: I1f3f4b64e1fa6aa6bdec198f4693be919e50681e
Reviewed-on: https://chromium-review.googlesource.com/663202
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501404}
[modify] https://crrev.com/ad438915cdcc0cdac9a0badd3a4f74a43a2a99a5/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java

Labels: Merge-Request-62
requesting merge of #12 into m62
Labels: -ReleaseBlock-Beta ReleaseBlock-Stable
The issue here is obvious when it occurs, only affects a small population, and a workaround is possible - thus changing to RB-Stable.

Will evaluate merge when canary coverage has been confirmed (e.g. issue fixed there, no crashes).
Project Member

Comment 16 by sheriffbot@chromium.org, Sep 14 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-62 Merge-Approved-62
Approved for M62 branch 3202.
Project Member

Comment 18 by bugdroid1@chromium.org, Sep 15 2017

Labels: -merge-approved-62
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21f2c828ac1880b6d7a37fcc54b2ea7b47858643

commit 21f2c828ac1880b6d7a37fcc54b2ea7b47858643
Author: Changwan Ryu <changwan@chromium.org>
Date: Fri Sep 15 19:29:37 2017

Finish composition first for Japanese keyboard

In my previous attempt to fix Japanese keyboard behavior
(https://chromium-review.googlesource.com/636107),
I was finishing composition text *after* restoring deleted text.
This works on most Japanese keyboards, but Google Japanese keyboard
behaves weirdly, as it calls setComposingText([text], 1) again which
moves the cursor position.

This can be fixed by finishing composition first, and then restoring
text.

BUG= 758443 

(cherry picked from commit ad438915cdcc0cdac9a0badd3a4f74a43a2a99a5)

Change-Id: I1f3f4b64e1fa6aa6bdec198f4693be919e50681e
Reviewed-on: https://chromium-review.googlesource.com/663202
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#501404}
Reviewed-on: https://chromium-review.googlesource.com/663948
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#257}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/21f2c828ac1880b6d7a37fcc54b2ea7b47858643/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Verified fix in 63.0.3219.0 and 62.0.3202.28 on Samsung galaxy Gran prime with japanese keyboard. Thanks!
Status: Started (was: Verified)
reopening as this actually caused a regression for Samsung keyboard - backward deletion is currently not working correctly on Samsung keyboard.
Labels: -M-62 M-63
Blocking: 744779
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/979c49edba4c5c56439b32072211f6edfd6a94f0

commit 979c49edba4c5c56439b32072211f6edfd6a94f0
Author: Changwan Ryu <changwan@chromium.org>
Date: Tue Oct 03 20:14:02 2017

Handle exceptional cases for spannable inline autocomplete

English typing on Japanese keyboards work in a similar way to Japanese
typing and this requires finishComposingText() before restoring deleted
text. Most keyboards react gracefully to finishComposingText() except
for the recent versions of Samsung keyboards.

As it is more difficult to list up all the Japanese keyboards, we will
always call finishComposingText() except for Samsung keyboards.

IQQI keyboards are in general behaves weirdly and cannot be worked
around, so blacklisted from inline autocomplete.

Manually tested on the following keyboards:

1) Google keyboards: Google Latin IME (English, Czech),
   Google Indic IME (English), Google Korean IME, Google Japanese IME
2) OEM keyboards: Samsung S7, Samsung Grand (English, Czech),
   LG V20, HTC Desire 310
3) From Play Store: SwiftKey, Swype, IQQI keyboards, Baidu IME, Sogou IME,
   GO keyboard, TouchPal, Ginger keyboard, Easy Urdu
4) More Japanese keyboards: ATOK trial, Shimeji, Japanese Keyboard by
   Umbrella

BUG=767016, 766888,  758443 

Change-Id: I725b450ee7a3f8a2047e9d6e5db1e5a516081a54
Reviewed-on: https://chromium-review.googlesource.com/694183
Reviewed-by: Alexandre Elias <aelias@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506152}
[modify] https://crrev.com/979c49edba4c5c56439b32072211f6edfd6a94f0/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java
[modify] https://crrev.com/979c49edba4c5c56439b32072211f6edfd6a94f0/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
[modify] https://crrev.com/979c49edba4c5c56439b32072211f6edfd6a94f0/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java
[modify] https://crrev.com/979c49edba4c5c56439b32072211f6edfd6a94f0/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

Labels: Merge-Request-62
Requesting merge of #24 into M62 - only beta users under experiment will be affected.
Project Member

Comment 26 by sheriffbot@chromium.org, Oct 3 2017

Labels: -Merge-Request-62 Merge-Review-62
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
can we have some tests to make sure this does not regress for any other thing? or how can we just ensure that this does not cause any other regression.
The code modified does not run at all when the experiment is deactivated, so this is very safe.
Labels: -Hotlist-Merge-Review -Merge-Review-62 Merge-Approved-62
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 4 2017

Labels: -merge-approved-62
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/224db22d1e06ecfbae1736eeade89e637588d7f2

commit 224db22d1e06ecfbae1736eeade89e637588d7f2
Author: Changwan Ryu <changwan@chromium.org>
Date: Wed Oct 04 16:18:50 2017

Handle exceptional cases for spannable inline autocomplete

English typing on Japanese keyboards work in a similar way to Japanese
typing and this requires finishComposingText() before restoring deleted
text. Most keyboards react gracefully to finishComposingText() except
for the recent versions of Samsung keyboards.

As it is more difficult to list up all the Japanese keyboards, we will
always call finishComposingText() except for Samsung keyboards.

IQQI keyboards are in general behaves weirdly and cannot be worked
around, so blacklisted from inline autocomplete.

Manually tested on the following keyboards:

1) Google keyboards: Google Latin IME (English, Czech),
   Google Indic IME (English), Google Korean IME, Google Japanese IME
2) OEM keyboards: Samsung S7, Samsung Grand (English, Czech),
   LG V20, HTC Desire 310
3) From Play Store: SwiftKey, Swype, IQQI keyboards, Baidu IME, Sogou IME,
   GO keyboard, TouchPal, Ginger keyboard, Easy Urdu
4) More Japanese keyboards: ATOK trial, Shimeji, Japanese Keyboard by
   Umbrella

BUG=767016, 766888,  758443 

(cherry picked from commit 979c49edba4c5c56439b32072211f6edfd6a94f0)

Change-Id: I725b450ee7a3f8a2047e9d6e5db1e5a516081a54
Reviewed-on: https://chromium-review.googlesource.com/694183
Reviewed-by: Alexandre Elias <aelias@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#506152}
Reviewed-on: https://chromium-review.googlesource.com/699643
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#578}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/224db22d1e06ecfbae1736eeade89e637588d7f2/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditText.java
[modify] https://crrev.com/224db22d1e06ecfbae1736eeade89e637588d7f2/chrome/android/java/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextModelBase.java
[modify] https://crrev.com/224db22d1e06ecfbae1736eeade89e637588d7f2/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java
[modify] https://crrev.com/224db22d1e06ecfbae1736eeade89e637588d7f2/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
From issue 773197#c8,

- iWnn is bundled on SHARP devices (10.4%) and Kyocera (7.6%).
- PO Box Plus is bundled on Sony devices (12.6%).

The above numbers show the market share in Japan @ 2016.
https://www.idcjapan.co.jp/Press/Current/20170221Apr.html
https://www.idcjapan.co.jp/Press/Current/20170221_4.gif
Note that Fujitsu bundles ATOK.

I'll have to test more extensively on Japanese keyboard.
changwan@, any update on this M63 blocker?
We don't have any sharp devices, but I will blacklist iWnn because the free version (Wnn Keyboard Lab) doesn't seem to work correctly, and
POBox Plus (com.sonymobile.pobox/.POBoxPlus) works correctly after the fix in #24. ATOK also works correctly after the fix in #24.
Package names for wnn / iwnn variants:

jp.co.sharp.android.iwnn
jp.co.omronsoft.wnnlab
jp.co.omronsoft.iwnnime.*
Project Member

Comment 36 by bugdroid1@chromium.org, Oct 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/523e6cee73f7729c2ba74ef89e416824d6050de3

commit 523e6cee73f7729c2ba74ef89e416824d6050de3
Author: Changwan Ryu <changwan@chromium.org>
Date: Tue Oct 31 16:28:20 2017

Blacklist iWnn and variants

Wnn and iWnn don't seem to work correctly with
SpannableInlineAutocomplete, therefore blacklisting the package names.

BUG= 758443 

Change-Id: Ib9d677a4d4e49a43b1d70de5fb89b80eea876913
Reviewed-on: https://chromium-review.googlesource.com/745726
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512848}
[modify] https://crrev.com/523e6cee73f7729c2ba74ef89e416824d6050de3/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java

Labels: Merge-Request-63
Requesting merge of #36 to m63. It's just adding a blacklist.
Project Member

Comment 38 by sheriffbot@chromium.org, Oct 31 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Does the change in #36 completely fix this issue?
We're using blacklisting as a bandaid to exceptional cases here. There are chances that  we may find more keyboard types of this sort and may need to add them.
Has this been verified in canary? Could you add some tests along with that change?
adding askatte@ for #41
Project Member

Comment 43 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6c2132b7260a819c9e3c3e362d7aabea8d49a95

commit a6c2132b7260a819c9e3c3e362d7aabea8d49a95
Author: Changwan Ryu <changwan@chromium.org>
Date: Tue Nov 07 22:25:25 2017

Test blacklist for spannable autocomplete model

TBR=tedchoc@chromium.org
BUG= 758443 

Change-Id: I016c52574e5ac1098661fe6784a691f8332d2412
Reviewed-on: https://chromium-review.googlesource.com/757660
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514615}
[modify] https://crrev.com/a6c2132b7260a819c9e3c3e362d7aabea8d49a95/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java

added a small test per #41. I'm requesting merge of #36 and #43 into m63
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Thanks changwan@. The test can stay in 64 though. I am approving the merge of the fix into M63 branch 3239. Please verify the fix after merging it. 
Project Member

Comment 46 by bugdroid1@chromium.org, Nov 8 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/33a5c71d32a6ecddbc1766a12039159c74d33ba9

commit 33a5c71d32a6ecddbc1766a12039159c74d33ba9
Author: Changwan Ryu <changwan@chromium.org>
Date: Wed Nov 08 17:34:49 2017

Blacklist iWnn and variants

Wnn and iWnn don't seem to work correctly with
SpannableInlineAutocomplete, therefore blacklisting the package names.

BUG= 758443 

Change-Id: Ib9d677a4d4e49a43b1d70de5fb89b80eea876913
Reviewed-on: https://chromium-review.googlesource.com/745726
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#512848}(cherry picked from commit 523e6cee73f7729c2ba74ef89e416824d6050de3)
Reviewed-on: https://chromium-review.googlesource.com/758958
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#418}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/33a5c71d32a6ecddbc1766a12039159c74d33ba9/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Verified that omnibox inline autocomplete doesn't work with Wnn keyboard anymore. Verified in 63.0.3239.53 and 64.0.3269.0. thanks.

Sign in to add a comment