758820 - Form autocomplete of partially-entered words will replace the entered prefix characters with the autocompleted suffix characters

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Chris Peterson [:cpeterson]]

STR:
1. Open a web page with a text input form. (My favorite test page is the "Firefox: Support" page linked from about:home.)
2. Type the first few characters of a phrase that is already saved in your form history. For example, my form history contains the phrase "dog", so I type "d" or "do".
3. Touch the form suggestions to autocomplete the word you were typing.

AR:
The characters you were typing are erased and replaced by the *remaining* characters from the autocomplete word. For example, "d" will be replaced by "og" and "do" will be replaced by "g".

ER:
"dog"

I tested this on Galaxy Nexus with stock VKB and Swype IMEs.

Comment 1

[Chris Peterson [:cpeterson]]

qawanted, is this a regression? I see this bug in both Nightly 15 and Aurora 14, but I'm surprised no one found this problem earlier.

Comment 2

[Adrian Tamas (:AdrianT)]

This is a regression. The issue is not reproducible on Nightly 14.0a1 2012-04-27 on the HTC Desire running Android 2.2.

Comment 3

[Adrian Tamas (:AdrianT)]

This may have been a website issue. I am unable to reproduce the issue today on either Samsung Galaxy Nexus (Android 4.0.2) or HTC Desire (Android 2.2) on Nightly or Aurora on the builds from the 25th,26th,27th or 28th. 

Can you please confirm that you are no longer experiencing the issue?

Comment 4

[Chris Peterson [:cpeterson]]

I can still reproduce this on Nightly 15.0a1 2012-05-28. This bug is not specific to any particular website. Any page with a text input form is affected.

Comment 5

[Adrian Tamas (:AdrianT)]

I managed to reproduce the issue on the latest Nightly on HTC Desire(Android 2.2) but only if predictive input is enabled for the input method. I will work on narrowing down the regression window

Comment 6

[Adrian Tamas (:AdrianT)]

good build: May 17th
bad build: May 18th

Possible regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=762e95608da3&tochange=e794cef56df6

Comment 7

[Chris Peterson [:cpeterson]]

Adrian, thanks for tracking down the regression window. I suspect the regression was caused by cset 43aa1f392da4 for bug 751864.

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Removing qawanted as regression window was found.

Comment 9

[Chris Peterson [:cpeterson]]

I can still repro this bug on Aurora and Nightly.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

Even without 43aa1f392da4, the behavior isn't quite right. For instance with that backed out if I:

1) type "wh"
1a) which get the form auto-complete suggestion "why you no support tablets?"
1b) which gets the keyboard auto-complete suggestions "wh", "who" and "what"
2) click the form autocomplete suggestion

AR "why you not support tablets?" is in the input box with "wh" highlighted
ER no highlight

3) type another letter, say "!"

AR "wh!"
ER "why you not support tablets?!"

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: wip

Comment 12

[Chris Peterson [:cpeterson]]

blassey, your WIP patch reintroduces bug 751864 on my Galaxy Nexus. Removing the mCommittingText check causes composition state problems.

Comment 13

[Chris Peterson [:cpeterson]]

Attachment: squelch-autocomplete-popup.patch

This patch is NOT a proper fix for this bug. It is a temporary workaround for the beta release to prevent the bug from happening (at the cost of some functionality of form auto-complete).

If the user selects an item from the form auto-complete popup when we have an active composition string, IME gets in a bad state because both Gecko and Java are modifying the composition string. For now, just hide the form auto-complete popup during the composition of each word.

Form auto-complete will still be displayed and functional when the user is not composing a word, such as when the user first taps an empty text form or after the user presses space to finish composing a word.

Comment 14

[Brad Lassey [:blassey] (use needinfo?)]

I'd rather go the other way and hide the composition suggestions when a form-autocomplete pop up is shown. Can we do that?

Comment 15

[Chris Peterson [:cpeterson]]

(In reply to Brad Lassey [:blassey] from comment #14)
> I'd rather go the other way and hide the composition suggestions when a
> form-autocomplete pop up is shown. Can we do that?

The form auto-complete in browser.js is the problem, not the VKB's suggestions.

If the form auto-complete popup injects a new string value when Gecko is in the composing state, both Gecko and Java IME get confused about the current IME state and text value.

Masayuki says Gecko does not support setting value from script during composition (bug 549674).

Makoto fixed a similar Firefox problem (bug 632744) by detecting the composing state from JS and forces Gecko's to commit the incomplete composition string.

* [whatwg] set input.value when input element has composition string:
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2011-February/030656.html

Comment 16

[Chris Peterson [:cpeterson]]

This bug is basically XUL Fennec's bug 632744.

Comment 17

[Chris Peterson [:cpeterson]]

Attachment: suppress-autocomplete-popup.patch

Wallpaper patch: Suppress form auto-complete popup during IME composition.

This change suppresses the popup in browser.js and is pretty reliable. The form auto-complete popup is still displayed when we have no composition string (empty input form and after each word).

Comment 18

[Chris Peterson [:cpeterson]]

Attachment: commit-composition-before-autocomplete.patch

Commit active composition string before sending auto-complete notification.

This is a real fix, nearly identical to XUL Fennec's fix for this same problem (bug 632744). With this patch, auto-completing partial composition strings works, but is flaky on some IMEs due to other bugs. In particular, Swype's behavior is pretty bad because of bug 755909.

This patch is the real fix, but I think it is too flaky for our Fennec 14 schedule. I recommend we:

  1. Land my wallpaper patch (suppress-autocomplete-popup.patch) for Fennec 14
  2. Land this real fix for Fennec 15
  3. File and fix the IME-specific bugs for Fennec 15

Comment 19

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 632034 [details] [diff] [review]
suppress-autocomplete-popup.patch

I'm liking the other work around better

Comment 20

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 632036 [details] [diff] [review]
commit-composition-before-autocomplete.patch

changing my mind, reverting to ?

Comment 21

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 632036 [details] [diff] [review]
commit-composition-before-autocomplete.patch

If this is the better fix, for UX, I'd rather take it. It is not that much different, in size and type-of-change, then the other patch. The risk doesn't look that much different either.

Comment 22

[Chris Peterson [:cpeterson]]

(In reply to Mark Fickle (:mfinkle) from comment #21)
> If this is the better fix, for UX, I'd rather take it. It is not that much
> different, in size and type-of-change, then the other patch. The risk
> doesn't look that much different either.

But, as I noted above, the second patch causes some wonky problems with Swype (bug 755909). Swype will intermittently select the auto-completed word or move the cursor to the beginning of the auto-completed word.

Comment 23

[Chris Peterson [:cpeterson]]

Attachment: suppress-popup-after-typing.patch (option #3)

Suppress form auto-complete popup after the user starts typing.

Comment 24

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e781ec9cc444

Comment 25

[Chris Peterson [:cpeterson]]

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

Comment 26

[Chris Peterson [:cpeterson]]

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

Comment 27

[Joe Drew (not getting mail)]

The target should be 16, I think, since that's where you're landing it on central.

Comment 28

[Chris Peterson [:cpeterson]]

We can land the proper fix after bug 755909 is fixed.

Comment 29

[Chris Peterson [:cpeterson]]

I opened bug 764193 to track re-enabling the form autocomplete popup after the user starts typing.

Comment 30

[Aaron Train [:aaronmt]]

Verifying suppression of popup on typing (element value > 0) with 14.0b7 via option-patch #3. Confirming popup visible only on empty fields (nil value length).

Comment 31

[Ed Morley [:emorley]]

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

Comment 32

[Paul Feher]

Can't verify until Bug 764193 is not fixed

Comment 33

[Adrian Tamas (:AdrianT)]

For FireFox Mobile 15.0b3 the autocomplete popup is not displayed - Bug 764193 was marked as won't fix for 15. On Aurora and Nightly the issue is no longer reproducible and the correct behavior is seen.

Tested on:
Firefox Mobile 15.0b3/Nightly 17.0a1 2012-08-02/Aurora 16.0a2 2012-08-02
Samsung Galaxy Tab (Android 3.1)/ HTC Desire Z (Android 2.3.3)