Closed Bug 619278 Opened 9 years ago Closed 9 years ago

:-moz-ui-invalid should apply as soon as the user tries to submit the form

Categories

(Core :: DOM: Core & HTML, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla2.0b10

People

(Reporter: jk1700, Assigned: mounir)

References

()

Details

(Keywords: polish)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101214 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101214 Firefox/4.0b8pre

Open the URL - it has an text input with "required" and "autofocus" attributes - then press enter. The error message will appear but input should also have red shadow as an invalid field. Shadow appears only after blurring the input

Reproducible: Always
Blocks: 344614
Keywords: html5
Summary: Invalid input doesn't is not styled properly → Invalid input is not styled properly
Version: unspecified → Trunk
Indeed, when you try to submit the form, you should see the red glow with the error message. This is a minor issue though.
Blocks: 605124
No longer blocks: 344614
Severity: normal → minor
Status: UNCONFIRMED → NEW
Component: General → DOM: Core & HTML
Ever confirmed: true
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: general → general
Hardware: x86 → All
Summary: Invalid input is not styled properly → :-moz-ui-invalid should apply as soon as the user tries to submit the form
Keywords: html5
Keywords: polish
Attached patch Patch v1Splinter Review
I don't really like adding this new UpdateValidityUIBits method but I see no other way to fix that without regressing something else.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #500806 - Flags: review?(jonas)
Whiteboard: [needs-review]
Whiteboard: [needs-review] → [needs-review][passed-try]
Comment on attachment 500806 [details] [diff] [review]
Patch v1

># HG changeset patch
># Parent c09a52a853f032fd372152e4cad4839a86e4c1c4
># User Mounir Lamouri <mounir.lamouri@gmail.com>
># Date 1294068753 -3600
>
>diff --git a/content/html/content/src/nsHTMLFormElement.cpp b/content/html/content/src/nsHTMLFormElement.cpp
>--- a/content/html/content/src/nsHTMLFormElement.cpp
>+++ b/content/html/content/src/nsHTMLFormElement.cpp
>@@ -1752,16 +1752,24 @@ nsHTMLFormElement::CheckValidFormSubmiss
>            * be notified because we can't know.
>            * Submissions shouldn't happen during parsing so it _should_ be safe.
>            */
> 
>           MOZ_AUTO_DOC_UPDATE(doc, UPDATE_CONTENT_STATE, PR_TRUE);
> 
>           for (PRUint32 i = 0, length = mControls->mElements.Length();
>                i < length; ++i) {
>+            // Input elements can trigger a form submission and we want to
>+            // update the style in that case.
>+            if (mControls->mElements[i]->IsHTML(nsGkAtoms::input) &&
>+                nsContentUtils::IsFocusedContent(mControls->mElements[i])) {
>+              static_cast<nsHTMLInputElement*>(mControls->mElements[i])
>+                ->UpdateValidityUIBits(true);
>+            }

This will mean that calling formelement.checkValidity() will turn on the invalid UI on all invalid controls, right. That's actually sort of cool so I think we should leave that for now. But something that'd be good to get feedback from authors on.

But this function really should be renamed. It's doing a lot more than check validity at this point.

r=me, ideally with the function renamed to something more descriptive.
Attachment #500806 - Flags: review?(jonas) → review+
I think you mixed up nsHTMLFormElement::CheckValidity() and nsHTMLFormElement::CheckValidFormSubmission()... I had to add CheckValidFormSubmission() because the form submit event is sent by the form controls and if the form is invalid, the form submit event should not be sent. It might be something to rewrite for after Gecko 2.0 btw.
Attachment #500806 - Flags: approval2.0?
Whiteboard: [needs-review][passed-try] → [needs-approval][passed-try]
Blocks: 622558
Attachment #500806 - Flags: approval2.0? → approval2.0+
Whiteboard: [needs-approval][passed-try] → [passed-try][needs landing]
Pushed:
http://hg.mozilla.org/mozilla-central/rev/6c8c7ff7505a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [passed-try][needs landing]
Target Milestone: --- → mozilla2.0b10
Depends on: 1209035
You need to log in before you can comment on or make changes to this bug.