630495 - Show a message to submit the form when the form is invalid because of hidden form controls

Status: NEW

(Firefox :: General, defect, P5)

Description

[Mounir Lamouri (:mounir)]

It's a follow-up from bug 595451: when a form control is invalid and hidden, instead of doing nothing, we should probably try to show something explaining the issue and asking the user of he/she still wants to submit the form (with all implications it might have).

It's probably a UX-related issue but I believe it can't be worse than what we currently have.

Comment 1

[Mounir Lamouri (:mounir)]

Atul, this is not doable in a couple of hours but would be much more like a few weeks project if you can only dedicate a few hours per week on it.

Comment 2

[Raymond Hung]

If this bug is still open, I would like to take this bug.

Comment 3

[Mounir Lamouri (:mounir)]

Sure. If you have any question, feel free to ask here or to send me an email. If you think you can already try to work a patch, do so and attach it here.

Comment 5

[Mounir Lamouri (:mounir)]

To attack that bug, you should have a look at |gFormSubmitObserver| in browser/base/content/browser.js and get familiarize on how it is working. What you should do is when aInvalidElements[0] is hidden, you should probably show the popup on the submit button. If hidden too, you should try to find a place to show it. The popup message would be different saying that there is a bug because there is an invalid hidden element in the form.
That would be a first step. Shouldn't be too hard. The second step would be to show a message asking if the user wants to submit the form anyway with a button to do so.

Comment 6

[Mounir Lamouri (:mounir)]

Attachment: Testcase

The form in the test case has a hidden required form. In Chrome/Opera, the form isn't submitted and nothing is shown. In Firefox, the form isn't submitted and an error message is shown at the wrong place.
We should at least show a message saying that something is wrong. Ideally, we should allow the user to still submit the form.

Comment 7

[Mounir Lamouri (:mounir)]

Also, look at the patch in bug 595451 to see how to check if an element is visible.

Comment 8

[Alice Lieutier]

Attachment: first proposed patch _ not final

I made this first patch, but I have a bug and I do not know why..

The behavior I wanted to get is the following :

1. If there are visible fields and invisible fields that are invalid at the time of submitting, show the validation message of the first visible field.

2. If only invisible fields are invalid, show a panel with a message saying: "One or more invisible fields are invalid, preventing this form from being submitted." 

Right now, in case 2, the panel does not show if the form has not been previously invalid because of a invalid visible field.

Comment 9

[Mounir Lamouri (:mounir)]

Comment on attachment 659565 [details] [diff] [review]
first proposed patch _ not final

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

Unfortunately, I can't really help you with the bug you saw. Maybe a Firefox person might be more helpful than me ;)

::: browser/base/content/browser.js
@@ +690,5 @@
>  
> +    let element = getFirstVisibleInvalidElement(aInvalidElements);
> +
> +    if (!element) {
> +      // Display an error message 

nit: there is a trailing white space there. Add a colon (:) or a dot (.) instead.

@@ +693,5 @@
> +    if (!element) {
> +      // Display an error message 
> +      // One or more invisible fields are invalid, preventing this form from being submitted.
> +      this.panel.firstChild.textContent = gNavigatorBundle.getString("formValidation.hiddenInvalid");
> +      this.panel.openPopup(gURLBar, "bottomleft topleft");

I think it might be good to show the panel on the identity button. In that case, that would be "centerbottom topleft", I think.

Comment 10

[Mina Almasry]

It's been more than 6 months with no activity here. If no one minds, I'll assign myself this, and take a go at it.

Comment 11

[Justin Dolske [:Dolske]]

Comment on attachment 659565 [details] [diff] [review]
first proposed patch _ not final

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

::: browser/base/content/browser.js
@@ +667,5 @@
> +        element = aInvalidElements.queryElementAt(i, Ci.nsISupports);
> +
> +        // If the element is visible, return it.
> +        let rect = element.getBoundingClientRect();
> +        if (!(rect.width == 0 && rect.height == 0 && rect.top == 0 && rect.left == 0)) {

I don't think you need to check .top or .left.

If width==height==0, that's should be sufficient?

Will this catch style="visibility:hidden"? (There are probably an infinite number of other ways to "hide" things, but I don't think this check needs to be absurdly robust.)

::: browser/locales/en-US/chrome/browser/browser.properties
@@ +401,5 @@
>  identity.loggedIn.signOut.label = Sign Out
>  identity.loggedIn.signOut.accessKey = O
> +
> +# Hidden Invalid Field
> +formValidation.hiddenInvalid=One or more invisible fields are invalid, preventing this form from being submitted.

I think this should point a bit of blame at the page, since the site has done something dumb (dare I say user-hostile), and it's not Firefox's fault.

"Well, this is awkward. There is an invalid form field somewhere on the page, but the site has hidden it from view."

Hmm. Bold question -- can we just submit it anyway, and ignore the invalidness (and error message)? Does the spec cover what should happen when an validation fails for an invalid field?

Comment 12

[Richard Neill]

I agree that the site has done something dumb (I am guilty of writing such things at least once). So, by all means "blame" the page - maybe even hint that the user should inform the webmaster. 

Perhaps "Well, this is awkward. There is an empty form field somewhere on the page, which you must fill in, but the site has hidden it from view. (You may wish to report this error to the site's webmaster)"

I'd suggest 4 cases you need to test for:
  display:none
  visibility:hidden
and the JS versions of these (i.e. where the visibility has been turned off in JS, rather than the HTML/CSS).

My suggestion would be to show the field that is missing and allow the user to fill it in, either by unhiding it, or just generating a dialog box where the user can type.

IMHO, it IS ok to submit the page anyway, ignoring the "required" field. That is what would happen in any browser that can't understand the HTML5 "required" option (and what would happen if the user uses something like Web-developer to force it). So the server really should cope.

Comment 13

[Mina Almasry]

Sorry it looks like I won't be able to do this now. I'm reassigning back to Alice.

Comment 14

[Simon Harte]

This is really a problem for me atm, because we have different sections with multiple fields in a form which get hidden/unhidden through user input. These sections contain required fields, but they only have to be filled in if the section is actually visible... Inputs which are not visible should be taken out of the validation routine imho because you should assume that those fields were hidden on purpose.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

If you want that then you should file a separate bug, or even better contact the spec author.

This bug is about having better notifications to the user (not the developer) when this situation arises.

Comment 16

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

I would like to work on this bug as it is a student project

I will start working on this by the end of 2015 or in the starting of 2016, I am assigning this one to myself.

Comment 17

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

Any help on this one ?

Comment 18

[Mounir Lamouri (:mounir)]

I don't have the bandwidth for this unfortunately. Added smaug who might be able to help.

Comment 19

[:Gijs (he/him)]

Doesn't look like this needinfo request from 4 years ago is still useful.

Comment 20

[Mark Banner (:standard8)]

Unfortunately we don't have someone ready to mentor this to help it move forward at the current time.