Closed Bug 1037353 Opened 5 years ago Closed 5 years ago

Auto complete pop up doesn't go away

Categories

(Firefox for Android :: General, defect)

Other
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 34
Tracking Status
fennec 34+ ---

People

(Reporter: blassey, Assigned: bnicholson)

References

Details

Attachments

(1 file, 2 obsolete files)

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
tracking-fennec: ? → 34+
Assignee: nobody → 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?
Keywords: qawanted
Attached image Screenshot_2014-07-25-17-45-25.png (obsolete) —
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
Keywords: qawanted
Status: NEW → ASSIGNED
(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.
Brad, can you still reproduce this? Is one of your autocomplete values a substring of the other like in comment 2?
Flags: needinfo?(blassey.bugs)
(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 "brad@lassey.us" entered as the user name, there continues to be a suggestion popup for "brad@lassey.us"
Flags: needinfo?(blassey.bugs)
Adding qawanted to see if anyone else can reproduce this.
Keywords: qawanted
Brad just tried to reproduce this and said the issue is gone.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: qawanted
Resolution: --- → WORKSFORME
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.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
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.
Attachment #8462599 - Attachment is obsolete: true
Attachment #8476340 - Flags: feedback?(nchen)
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 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.
Attachment #8476340 - Flags: feedback?(nchen) → feedback+
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.
Attachment #8476340 - Attachment is obsolete: true
Attachment #8480875 - Flags: review?(nchen)
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!
Attachment #8480875 - Flags: review?(nchen) → review+
https://hg.mozilla.org/mozilla-central/rev/c8449f5148ed
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
You need to log in before you can comment on or make changes to this bug.