559925 - formfill does not pan through all buttons at tekstaro.com

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.5pre) Gecko/20100414 Namoroka/3.6.5pre Fennec/1.1b1

Steps to Reproduce:
1. Go to www.tekstaro.com/
2. Click on the button labeled, "Serci".
3. Look at the formfill bar

Actual Results:
The "Next" button on the formfill bar will be grayed out and unclickable.

Expected Results:
The "Next" button should be clickable and the helper should go to "Ciuj" when clicked.

Comment 1

[Matt Brubeck (:mbrubeck)]

"Serĉi" is an <input type=submit> element, while the other buttons are <button> elements.

FormHelper will include <input type=submit> but does not include <input type=button> or <button> elements.  We should probably change this to be more consistent, since all three of these look the same to the user.

Comment 2

[Matt Brubeck (:mbrubeck)]

Correction:  The other buttons on http://www.tekstaro.com/ are <button type=button>.

FormHelper does include <button> but does not include <input type=button> or <button type=button>.

Comment 3

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

(In reply to comment #2)
> Correction:  The other buttons on http://www.tekstaro.com/ are <button
> type=button>.
> 
> FormHelper does include <button> but does not include <input type=button> or
> <button type=button>.

As said on IRC, FormHelper includes what you're supposed to fill (textarea, textbox, input type="file") _and_ the button used to submit this form (input type="image", input type="submit", button).

I'm agree that this can be odd because except the label of the button there is no visual difference between these and the normal button.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Here's a patch to include type="button" elements in FormHelper prev/next navigation.

Comment 5

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

Madhava - I think we need a written spec for the way this should work.

* What elements cause the Form Assistant to appear?
* What elements are in the prev/next cycle, after the Form Assistant is active?

Comment 6

[Madhava Enros [:madhava]]

So, the general intention of the form assistant is to replace tab/shift-tab functionality that would be available to someone with a full keyboard, and to deal with a situation particular to small screens - that, 

(a) at initial zoom, it's too hard to see what you're doing in a small field, so auto-zooming into the field makes sense, but
(b) that once you're that zoomed in, you use your context of where you are relative to the rest of the form, but zooming out then in every time is not a great solution

Which means that being able to have prev/next functionality, to guide you through the form, is very helpful.  Add a mobile-friendly select-box widget and an autocomplete bar to this, and you have the fennec form assistant.


* What elements cause the Form Assistant to appear?

Any form item other than a button (where pressing it actually causes some action, rather than just focusing) should cause the form assistant to appear.  Maybe that means that checkboxes and radio buttons also don't invoke the form assistant? Not sure.

* What elements are in the prev/next cycle, after the Form Assistant is active?

Everything should be (given that the purpose is to let people not have to feel like they have to zoom back out to know if they've completed everything in a form).  Checkboxes should be each focused, given that each one is a decision.  Radiobutton _sets_ should be included, but not each radiobutton, because a user can only make a decision at the radiobutton set level.  For now, we can focus the currently selected item; if none is selected, we can focus the first one.  In the future, we might want to provide some helper UI, as we do for selectboxes.

Comment 7

[Madhava Enros [:madhava]]

correction for comment 6:

"(b) that once you're that zoomed in, you use your context of where you are
relative to the rest of the form, but zooming out then in every time is not a
great solution"

should be "you LOSE your context of where you are"

Comment 8

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

Comment on attachment 439631 [details] [diff] [review]
patch

I think we need a new patch based on Madhava's "spec"

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Code and tests updated to match the specs in comment 6.

> Maybe that means that checkboxes and radio buttons also don't
> invoke the form assistant? Not sure.

This patch keeps the previous behavior as regards this question.  (Checkboxes and radio buttons do *not* invoke the form assistant.)

Comment 10

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

Comment on attachment 440624 [details] [diff] [review]
patch v2

>diff -r dd6d03cbc981 chrome/content/browser-ui.js

>-    let formExceptions = ["submit", "image", "file"];
>-    if (aElement instanceof HTMLInputElement && formExceptions.indexOf(aElement.type) != -1)
>+    let formExceptions = {button:1, checkbox:1, file:1, image:1, radio:1, reset:1, submit:1};
>+    if (aElement instanceof HTMLInputElement && formExceptions[aElement.type])

I think we should use | true | instead of | 1 | in formExceptions. It conveys the intent better.

Great job updating the tests too!

I'll change and push. Tests all pass.

Comment 11

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

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

Comment 12

[Aakash Desai [:aakashd]]

verified FIXED On build:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.5pre) Gecko/20100423 Namoroka/3.6.5pre Fennec/1.1a2pre

Comment 13

[Aakash Desai [:aakashd]]

We really should have an automated test (or set of tests) for this.

Comment 14

[Matt Brubeck (:mbrubeck)]

The patch in this bug added browser-chrome tests for this behavior.

Comment 15

[Aakash Desai [:aakashd]]

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