Closed Bug 551517 Opened 10 years ago Closed 10 years ago

Form autocomplete adds "undefined" into the field when user taps on parts of its UI

Categories

(Firefox for Android Graveyard :: General, defect)

Fennec 1.1
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: aakashd, Assigned: mfinkle)

Details

(Whiteboard: formfill)

Attachments

(1 file)

Build ID:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.2pre) Gecko/20100310 Namoroka/3.6.2pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a2pre) Gecko/20100310 Namoroka/3.7a2pre Fennec/1.1a2pre

Steps to Reproduce:
1. Go to bugzilla.mozilla.org
2. Tap on the search field
3. Enter a search term and press enter
4. On the resulting search results page, tap on the search field again
5. Tap on the empty white space on the form autocomplete UI.

Actual Results: 
"undefined" is populated into the search field

Expected Results:
Nothing should be populated into the search field
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: formfill
Attached patch patchSplinter Review
Only attempt to set a suggestion if it comes from a <label>. Ignore the rest.
Assignee: nobody → mark.finkle
Attachment #431677 - Flags: review?(21)
Flags: in-litmus?
Comment on attachment 431677 [details] [diff] [review]
patch

>   doAutoFill: function formHelperDoAutoFill(aElement) {
>     if (!this._currentElement)
>      return;
> 
>-    this._currentElement.value = aElement.value;
>+    // Suggestions are only in <label>s. Ignore the rest.
>+    if (aElement instanceof Ci.nsIDOMXULLabelElement)
>+      this._currentElement.value = aElement.value;
>   },

Maybe instead of passing aElement as an argument of doAutofill() we should directly pass a value and test that, this will allow extensions developpers to call this function?
Comment on attachment 431677 [details] [diff] [review]
patch

(In reply to comment #2)
> (From update of attachment 431677 [details] [diff] [review])
> >   doAutoFill: function formHelperDoAutoFill(aElement) {
> >     if (!this._currentElement)
> >      return;
> > 
> >-    this._currentElement.value = aElement.value;
> >+    // Suggestions are only in <label>s. Ignore the rest.
> >+    if (aElement instanceof Ci.nsIDOMXULLabelElement)
> >+      this._currentElement.value = aElement.value;
> >   },
> 
> Maybe instead of passing aElement as an argument of doAutofill() we should
> directly pass a value and test that, this will allow extensions developpers to
> call this function?

Thinking of it myself, this is not really needed because this is pretty cheap to access the current element of FormHelper
Attachment #431677 - Flags: review?(21) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/dec769500fa3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
verified FIXED on builds:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2.2pre) Gecko/20100312 Namoroka/3.6.2pre Fennec/1.1a1

and

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.2pre) Gecko/20100312 Namoroka/3.6.2pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a3pre) Gecko/20100312 Namoroka/3.7a3pre Fennec/1.1a2pre
Status: RESOLVED → VERIFIED
litmus testcase https://litmus.mozilla.org/show_test.cgi?id=11659 created to regression test this bug
Flags: in-litmus? → in-litmus+
You need to log in before you can comment on or make changes to this bug.