613973 - Do not hide the invalid form popup if the user types in the invalid form control

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mounir Lamouri (:mounir)]

This has been requested by limi some time ago.

I'm not sure if that's what you want but I'm going to write a patch which will remove the popup as soon as the form control becomes valid to prevent situations were the user did what had to be done but still see the message.

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch v1.1

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 492345 [details] [diff] [review]
Patch v1.1

>+    let blurHandler = function(e) {
>       gFormSubmitObserver.panel.hidePopup();
>     };

function blurHandler() {
  gFormSubmitObserver.panel.hidePopup();
}

>+    let inputHandler = function(e) {
>+      if (e.originalTarget.validity.valid) {
>+        gFormSubmitObserver.panel.hidePopup();
>+      }
>+    };

function inputHandler(event) {
  if (event.currentTarget.validity.valid) {
    gFormSubmitObserver.panel.hidePopup();
}

By the way, I don't understand why all this is in browser.js, i.e. front-end code, rather than several levels lower where the handling of HTML and form features belongs, with some very lightweight interface for the browser to display the panel.

Comment 4

[Mounir Lamouri (:mounir)]

(In reply to comment #3)
> Comment on attachment 492345 [details] [diff] [review]
> Patch v1.1
> 
> By the way, I don't understand why all this is in browser.js, i.e. front-end
> code, rather than several levels lower where the handling of HTML and form
> features belongs, with some very lightweight interface for the browser to
> display the panel.

Because that's not related to the content/HTML. The submission of the form is related to the content/HTML but not how an invalid submission should be handled. Depending of the browser/Gecko consumers, I would expect different handling. For example, Fennec might want to use its form manager.
In addition, it guarantees us that a browser that is not handling an invalid submission will not block the submission. Indeed, I've introduced an event for the invalid form submission and submissions aren't blocked if there are no observers for this event (thus, we do not break Fennec for example).

Comment 5

[Dão Gottwald [:dao]]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 492345 [details] [diff] [review] [details]
> > Patch v1.1
> > 
> > By the way, I don't understand why all this is in browser.js, i.e. front-end
> > code, rather than several levels lower where the handling of HTML and form
> > features belongs, with some very lightweight interface for the browser to
> > display the panel.
> 
> Because that's not related to the content/HTML. The submission of the form is
> related to the content/HTML but not how an invalid submission should be
> handled. Depending of the browser/Gecko consumers, I would expect different
> handling. For example, Fennec might want to use its form manager.

Maybe it's because I don't understand what Fennec using its form manager implies, but I don't see why a browser would want to handle this very bug differently. The browser should just be notified when it should show or hide the UI, whatever that UI might be. Having front-end code listen to blur and input events on content nodes seems like a poor and fragile solution.

Comment 6

[Benjamin Smedberg]

Comment on attachment 492345 [details] [diff] [review]
Patch v1.1

Clearing approval flag until reviews are complete.

Comment 7

[Alex Limi (:limi) — Firefox UX Team]

Comment on attachment 492345 [details] [diff] [review]
Patch v1.1

Works for me!

Comment 8

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/acb2f28f8e12

Comment 9

[[e-mail]]

So, I just tried this with an <input type=email required>. If you try to submit the form, you are prompted "Please fill out this field". As you start typing, the message remains until you type out something containing an @ and a . (since it's type=email). I don't think that's correct behaviour. I'd prefer:
a) the old behaviour, when you only get the tooltip on the actual submit
b) the message should remain for as long as the original error remains
c) the message should update dynamically as you type

Comment 10

[Mounir Lamouri (:mounir)]

(In reply to comment #9)
> So, I just tried this with an <input type=email required>. If you try to submit
> the form, you are prompted "Please fill out this field". As you start typing,
> the message remains until you type out something containing an @ and a . (since
> it's type=email). I don't think that's correct behaviour. I'd prefer:
> a) the old behaviour, when you only get the tooltip on the actual submit

The old behavior was bad. I don't think reverting to it is a good idea.

> b) the message should remain for as long as the original error remains
> c) the message should update dynamically as you type

I don't know which one might be the best between b and c. Probably c because it will help the user as long as the field isn't valid.

And thanks for this report! :)

Comment 11

[Mounir Lamouri (:mounir)]

I have open bug 623870 for the follow-up.