623870 - Update the message in the invalid form popup if the error message changes

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Mounir Lamouri (:mounir)]

1. Load the URL field and submit the form by pressing enter while focusing the text field. You should see a popup telling you that you should fill out the field ;
2. Type 'f' ;
3. The field doesn't suffer from being missing anymore but from a type mismatch (f isn't a valid email address)

Actual results: the message still asks you to fill out the field.

Expected results: the message should ask you to enter a valid email address (or alternatively disappear).

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

I get a Popup with "Please fill out this field.". "f" + "Enter" changes the Popup to "Please entern an email adress."
Or this about entering "f" without hitting "Enter" and the Popuptext should change?

Comment 2

[Mounir Lamouri (:mounir)]

(In reply to comment #1)
> I get a Popup with "Please fill out this field.". "f" + "Enter" changes the
> Popup to "Please entern an email adress."
> Or this about entering "f" without hitting "Enter" and the Popuptext should
> change?

Exactly. When you typed 'f', the field has been filed so the error message is outdated.

Comment 3

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Very simple patch with its test.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 502277 [details] [diff] [review]
Patch v1

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>     function inputHandler(e) {
>       if (e.originalTarget.validity.valid) {
>         gFormSubmitObserver.panel.hidePopup();
>+      } else {
>+        // If the element is now invalid for a new reason, we should update the
>+	// error message.
>+	if (gFormSubmitObserver.panel.firstChild.textContent !=
>+            e.originalTarget.validationMessage) {
>+	  gFormSubmitObserver.panel.firstChild.textContent =
>+	    e.originalTarget.validationMessage;
>+	}
>       }

indentation needs fixing here. I'm not sure it's worth checking the value before setting... I guess in the common case the set would be superfluous.

>diff --git a/browser/base/content/test/browser_bug561636.js b/browser/base/content/test/browser_bug561636.js

>     // Clean-up and next test.
>     gBrowser.removeTab(gBrowser.selectedTab, {animate: false});

animate:false is the default behavior, no need to specify it (here or in the line you're adding).

Comment 5

[Mounir Lamouri (:mounir)]

Attachment: Patch v1.2

r=gavin

Comment 6

[Mounir Lamouri (:mounir)]

Attachment: Remove animate: false from removeTab

Comment 7

[Mounir Lamouri (:mounir)]

Gavin: thank you for the review btw :)

Comment 8

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/2c29556f324b
http://hg.mozilla.org/mozilla-central/rev/89be93b83dce

Comment 9

[Teodosia Pop]

Verified using Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12.