Closed Bug 735469 Opened 8 years ago Closed 8 years ago

Autocomplete list not filtered until space is entered

Categories

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

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 14
Tracking Status
firefox13 --- verified
firefox14 --- verified
blocking-fennec1.0 --- +

People

(Reporter: cpeterson, Assigned: bnicholson)

References

()

Details

Attachments

(1 file)

STR:
1. On a Galaxy Nexus, load http://people.mozilla.org/~mwargers/tests/autocomplete_forms/textinput_form.html
2. In the "Submit Query" text box, enter a phrase like "apple juice" and hit the submit button.
3. Now enter a different phase like "banana peel".

AR:
On my Galaxy Nexus, the autocomplete menu does not filter the suggestion list until I enter a space. I see the "apple juice" suggestion until I enter the space after the word "banana".

ER:
On my Galaxy S II, autocomplete seems to work correctly. The "apple juice" suggestion is filtered after typing just the letter "b".

---

This problem may be related to the ICS IME's own autocomplete suggestions preempting our own.
blocking-fennec1.0: --- → ?
Duplicate of this bug: 730478
(In reply to Chris Peterson (:cpeterson) from comment #0)

> This problem may be related to the ICS IME's own autocomplete suggestions
> preempting our own.

We're generating our autocomplete suggestions based on DOM events, so I don't think this problem would be related to the ICS IME's autocomplete suggestions, unless that's interfering with how we're updating element.value. As I said in bug 730478, I noticed that element.value was wrong when I added some logging in here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3050
Assignee: nobody → margaret.leibovic
blocking-fennec1.0: ? → +
Assignee: margaret.leibovic → bnicholson
I don't believe this is specific to the Galaxy Nexus; I have the same problem on my Droid Razr.
Summary: Galaxy Nexus does not filter autocomplete suggestion list until after a space is entered → Autocomplete list not filtered until space is entered
Attached patch patchSplinter Review
After playing around with the IME code for a few days, I came up with this patch which seems to improve/fix several things:
- the autocompletion space issue described in this bug
- text that is being composed is now underlined
- deleting words in swype is fixed (previously, holding backspace to delete words in Swype resulted in partial deletions and other weird glitches)

We were creating an IME_SET_TEXT event for ranges, which resulted in setting the text twice. I replaced this with IME_ADD_RANGE.

For some reason, we were sending an IME_DELETE_TEXT event when count == 0 in the TextWatcher. I found that changing IME_SET_TEXT->IME_ADD_RANGE as described above caused text to not appear after deleting a word, and this IME_DELETE_TEXT event was the culprit. Removing this fixed the problem (and had the side effect of fixing Swype deletions).

Finally, with these changes, I noticed that when typing a letter and then deleting it, the autocomplete list didn't reappear (which I imagine we want to do since we're returning the text field to its initial state). I found that manually calling endComposition() fixed this.
Attachment #607406 - Flags: review?(cpeterson)
Comment on attachment 607406 [details] [diff] [review]
patch

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

::: mobile/android/base/GeckoEvent.java
@@ +369,5 @@
>      public static GeckoEvent createIMERangeEvent(int offset, int count,
>                                                   int rangeType, int rangeStyles,
>                                                   int rangeForeColor, int rangeBackColor) {
>          GeckoEvent event = new GeckoEvent(IME_EVENT);
> +        event.InitIMERange(IME_ADD_RANGE, offset, count, rangeType, rangeStyles,

1. Should the other createIMERangeEvent(..., String text) method be changed from IME_SET_TEXT to IME_ADD_RANGE, too?

2. If so, then I think the createIMERangeEvent(..., String text) method should call this createIMERangeEvent() method instead of duplicating the InitIMERange() call.

3. And then InitIMERange() could be merge into its only caller, createIMERangeEvent().
Comment on attachment 607406 [details] [diff] [review]
patch

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

r=cpeterson.

You can ignore my previous feedback. I see that the other createIMERangeEvent() method should continue to use IME_SET_TEXT.
Attachment #607406 - Flags: review?(cpeterson) → review+
https://hg.mozilla.org/mozilla-central/rev/17b7af74d069
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 14
Verified/fixed on:

Aurora Fennec 13.0a2 (2012-03-27)
Nightly Fennec 14.0a1 (2012-03-27)
Device: Samsung Nexus S
OS: Android 2.3.6
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.