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

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aakash Desai [:aakashd]]

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

Comment 1

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

Attachment: patch

Only attempt to set a suggestion if it comes from a <label>. Ignore the rest.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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

Comment 4

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

pushed:
http://hg.mozilla.org/mobile-browser/rev/dec769500fa3

Comment 5

[Aakash Desai [:aakashd]]

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

Comment 6

[Aakash Desai [:aakashd]]

litmus testcase https://litmus.mozilla.org/show_test.cgi?id=11659 created to regression test this bug