Closed Bug 623870 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b12

People

(Reporter: mounir, Assigned: mounir)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files, 1 obsolete file)

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).
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?
(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.
Attached patch Patch v1 (obsolete) — Splinter Review
Very simple patch with its test.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #502277 - Flags: review?(dao)
Blocks: 624174
Whiteboard: [needs review][passed try]
Attachment #502277 - Flags: review?(gavin.sharp)
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).
Attachment #502277 - Flags: review?(gavin.sharp) → review+
Attachment #502277 - Flags: review?(dao)
Attached patch Patch v1.2Splinter Review
r=gavin
Attachment #502277 - Attachment is obsolete: true
Attachment #507819 - Flags: approval2.0?
Whiteboard: [needs review][passed try] → [passed try][needs approval]
Gavin: thank you for the review btw :)
Attachment #507819 - Flags: approval2.0? → approval2.0+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/2c29556f324b
http://hg.mozilla.org/mozilla-central/rev/89be93b83dce
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [passed try][needs approval]
Target Milestone: --- → Firefox 4.0b12
No longer blocks: FF2SM
Verified using Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.