1037353 - Auto complete pop up doesn't go away

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

When logging into bugzilla I get two user name suggestions. Clicking on one fills in the password, but that's hidden by the fact that the suggestion pop up vibes right back up with only the suggestion that I just selected

Comment 1

[Brian Nicholson (:bnicholson)]

I can't reproduce. I have two user name suggestions appear in the popup, but when I click one, it goes away. Any more details?

Comment 2

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2014-07-25-17-45-25.png

Device: Samsung Galaxy Nexus 
OS: Android 4.2.1
Build: Firefox for Android 33.0a1 (2014-07-25)

I have two user name suggestions that appear in the popup: "Teo" and "Teodora".
After selecting "Teo", the password is filled in, but the other name "Teodora" remains under. Please see the attached screenshot

Comment 3

[Brian Nicholson (:bnicholson)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #2)
> Created attachment 8462599 [details]
> Screenshot_2014-07-25-17-45-25.png
> 
> Device: Samsung Galaxy Nexus 
> OS: Android 4.2.1
> Build: Firefox for Android 33.0a1 (2014-07-25)
> 
> I have two user name suggestions that appear in the popup: "Teo" and
> "Teodora".
> After selecting "Teo", the password is filled in, but the other name
> "Teodora" remains under. Please see the attached screenshot

Is this the same thing? This seems expected since there's still a partial autocomplete match (Teo), so the dropdown shows any autocomplete candidates starting with Teo (Teodora).

Comment 0 says the suggestion pops up with the same suggestion he just selected, which sounds like a worse bug since an autocomplete entry matching itself would never go away. But if so, I can't reproduce that.

Comment 4

[Brian Nicholson (:bnicholson)]

Brad, can you still reproduce this? Is one of your autocomplete values a substring of the other like in comment 2?

Comment 5

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

(In reply to Brian Nicholson (:bnicholson) from comment #4)
> Brad, can you still reproduce this? Is one of your autocomplete values a
> substring of the other like in comment 2?

Yes, I still see this. It's not a substring though, if I have "[e-mail]" entered as the user name, there continues to be a suggestion popup for "[e-mail]"

Comment 6

[Brian Nicholson (:bnicholson)]

Adding qawanted to see if anyone else can reproduce this.

Comment 7

[Brian Nicholson (:bnicholson)]

Brad just tried to reproduce this and said the issue is gone.

Comment 8

[Brian Nicholson (:bnicholson)]

I reproduced this last week by having two of the same entries with different capitalization, which I'm guessing is what happened in Brad's case. wesj also said he runs into this, so we should fix it. Let's prevent the autocomplete popup from reappearing after selecting an item until the user types something else.

Comment 9

[Brian Nicholson (:bnicholson)]

Attachment: Don't show autocomplete popup after selecting an autocomplete value

Fixing this has been frustrating. Here's the patch I have now that mostly works, but results in intermittent Java crashes when selecting an autocomplete value, and I can't figure out why.

Here's the intermittent crash:
E/GeckoAppShell(24323): java.lang.IndexOutOfBoundsException: setSpan (0 ... -3) has end before start
E/GeckoAppShell(24323): 	at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java:1009)
E/GeckoAppShell(24323): 	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:592)
E/GeckoAppShell(24323): 	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:588)
E/GeckoAppShell(24323): 	at android.text.TextUtils.copySpansFrom(TextUtils.java:1026)
E/GeckoAppShell(24323): 	at org.mozilla.gecko.GeckoEditable.onTextChange(GeckoEditable.java:893)
E/GeckoAppShell(24323): 	at org.mozilla.gecko.GeckoAppShell.notifyIMEChange(GeckoAppShell.java:513)
E/GeckoAppShell(24323): 	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
E/GeckoAppShell(24323): 	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
E/GeckoAppShell(24323): 	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:360)
E/GeckoAppShell(24323): 	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:186)

I have the autocomplete value "Abc" saved, and I can reproduce this crash pretty consistently with this patch applied by doing the following:
1) Click an input
2) Choose the "Abc" autocomplete value
3) Hit backspace so input is now "Ab"
4) Repeat steps 2 & 3 until crash

Jim, do you have any idea what's going on here? This patch seems pretty innocent to me...I guess the line where it breaks at "if (currentElement.value === this._currentInputValue)" is preventing something important from happening? But I have no idea what that would be.

Another problem I was hitting: if I typed "Ab", "Ab" was underlined since it's in the middle of a composition. When I clicked autocomplete suggestion "Abc", I first got an "input" event where the element value was "Ab", then a second "input" event where the element value was "Abc". I assume the first "input" event was triggered by the composition end that happened when I touched the autocomplete entry. This was causing the saved _currentInputValue to be useless since it caused _currentInputValue to first be updated to "Ab", so when the second "input" event came in, we showed the autocomplete popup again since "Ab" != "Abc". I fixed this by listening for "compositionupdate" instead of "input", which seems to work well enough.

Comment 10

[Brian Nicholson (:bnicholson)]

FWIW, I reproduced this crash in the debugger, and these are the values at these calls:

At GeckoAppShell.notifyIMEChange:
  editableListener.onTextChange(text, start, end, newEnd);
    text     c
    start    2
    end      2
    newEnd   3

At GeckoEditable.onTextChange:
  TextUtils.copySpansFrom(mText, start, Math.min(oldEnd, newEnd), Object.class, mChangedText, 0);
    mText        [A, b, 
    start        2
    oldEnd       2
    newEnd       3
    mChangedText c

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8476340 [details] [diff] [review]
Don't show autocomplete popup after selecting an autocomplete value

The crash is most likely caused by bug 1058127, which is a simple fix.

As discussed on IRC, I think we should go back to using "input" instead of "compositionupdate", and use some kind of flag to detect setting text vs user entering text. Also, there will be fewer intermediate input events once bug 1058136 is fixed.

Comment 12

[Brian Nicholson (:bnicholson)]

Attachment: Don't show autocomplete popup after selecting an autocomplete value, v2

It took awhile, but here's a patch that seems to work pretty well using "input". This uses both a flag and the previously entered input value to determine whether to show the autocomplete popup.

While debugging this, I found that "input" is fired more often than we want. For instance, simply loading http://bugzilla.mozilla.org fires two "input" events -- one for my username and another for my password -- without even touching any input fields. I added a check to make sure the element firing the event is focused to filter these out.

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8480875 [details] [diff] [review]
Don't show autocomplete popup after selecting an autocomplete value, v2

Review of attachment 8480875 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Comment 14

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/fx-team/rev/c8449f5148ed

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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