Implement novalidate attribute for form elements

RESOLVED FIXED in mozilla2.0b7

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

({dev-doc-complete, html5})

Trunk
mozilla2.0b7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 beta7+)

Details

()

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

9 years ago
If the novalidate attribute is present, the form has not to be validate when submitted.
Assignee

Comment 1

9 years ago
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
Assignee

Updated

9 years ago
Depends on: 561636
Assignee

Updated

9 years ago
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+
Assignee

Comment 3

9 years ago
Posted patch Patch v1 (obsolete) — Splinter Review
Attachment #473245 - Flags: review?(Olli.Pettay)
Assignee

Comment 4

9 years ago
Posted 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?
Assignee

Comment 6

9 years ago
(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?
Assignee

Comment 9

9 years ago
(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 :)
Assignee

Updated

9 years ago
Attachment #473328 - Attachment description: Patch Part2 v1.1 → Patch v1.1
Assignee

Comment 10

9 years ago
Posted patch Patch v1.2Splinter Review
r=smaug

Tests fixed.
Attachment #473328 - Attachment is obsolete: true
Assignee

Comment 11

9 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/8235bdf7c65e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b6
Assignee

Updated

9 years ago
Duplicate of this bug: 609033
You need to log in before you can comment on or make changes to this bug.