Closed Bug 556013 Opened 10 years ago Closed 10 years ago

Implement novalidate attribute for form elements

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: mounir, Assigned: mounir)

References

()

Details

(Keywords: dev-doc-complete, html5)

Attachments

(1 file, 2 obsolete files)

If the novalidate attribute is present, the form has not to be validate when submitted.
We can't ship Firefox 4 with form validation and without this attribute.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
blocking2.0: --- → ?
Keywords: dev-doc-needed
Depends on: 561636
Blocks: 566348
I'd say that this isn't a new feature but rather finishing up the whole validation thing. And it should be a safe patch as long as we get it in early enough.
blocking2.0: ? → beta6+
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #473245 - Flags: review?(Olli.Pettay)
Attached patch Patch v1.1 (obsolete) — Splinter Review
The assert was wrong and I've added some tests.
Attachment #473245 - Attachment is obsolete: true
Attachment #473328 - Flags: review?(Olli.Pettay)
Attachment #473245 - Flags: review?(Olli.Pettay)
Comment on attachment 473328 [details] [diff] [review]
Patch v1.1

I'd put HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate) to
CheckValidFormSubmission().
Or is there a reason not to?
(In reply to comment #5)
> Comment on attachment 473328 [details] [diff] [review]
> Patch Part2 v1.1
> 
> I'd put HasAttr(kNameSpaceID_None, nsGkAtoms::novalidate) to
> CheckValidFormSubmission().
> Or is there a reason not to?

Yes, we will have to check for formnovalidate on the submit control and  CheckValidFormSubmission() can't check that for us. So, better to have novalidate and formnovalidate checks at the same place.
I hate doing that but the base is already quite awful and we can't reasonably refactor the current code so close from releasing and without a good test coverage :-/
Comment on attachment 473328 [details] [diff] [review]
Patch v1.1


>+document.forms[0].addEventListener("submit", function(aEvent) {
>+  aEvent.target.removeAttribute("submit", arguments.callee, false);
removeAttribute? Also elsewhere.
Attachment #473328 - Flags: review?(Olli.Pettay) → review+
Shouldn't there be checks for the formnovalidate attribute somewhere? Or am I missing something?
(In reply to comment #8)
> Shouldn't there be checks for the formnovalidate attribute somewhere? Or am I
> missing something?

bug 589696(In reply to comment #7)

> Comment on attachment 473328 [details] [diff] [review]
> Patch Part2 v1.1
> 
> 
> >+document.forms[0].addEventListener("submit", function(aEvent) {
> >+  aEvent.target.removeAttribute("submit", arguments.callee, false);
> removeAttribute? Also elsewhere.

oooups :)
Attachment #473328 - Attachment description: Patch Part2 v1.1 → Patch v1.1
Attached patch Patch v1.2Splinter Review
r=smaug

Tests fixed.
Attachment #473328 - Attachment is obsolete: true
Pushed:
http://hg.mozilla.org/mozilla-central/rev/8235bdf7c65e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
Duplicate of this bug: 609033
You need to log in before you can comment on or make changes to this bug.