Closed Bug 596785 Opened 12 years ago Closed 12 years ago

"ASSERTION: Something went seriously wrong" with html5 form @required

Categories

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

defect
Not set
normal

Tracking

()

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

People

(Reporter: jruderman, Assigned: mounir)

References

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file testcase
###!!! ASSERTION: Something went seriously wrong!: 
'mInvalidElementsCount >= 0', 
file content/html/content/src/nsHTMLFormElement.cpp, line 1745

nsHTMLFormElement::UpdateValidity [nsHTMLFormElement.cpp:1751]
nsIConstraintValidation::SetValidityState [nsIConstraintValidation.cpp:143]
nsHTMLInputElement::UpdateValueMissingValidityState [nsHTMLInputElement.cpp:3820]
nsHTMLInputElement::UpdateAllValidityStates [nsHTMLInputElement.cpp:3839]
nsHTMLInputElement::HandleTypeChange [nsHTMLInputElement.cpp:2599]
nsHTMLInputElement::ParseAttribute [nsHTMLInputElement.cpp:2662]
nsGenericElement::SetAttr [content/base/src/nsGenericElement.cpp:4651]
nsGenericHTMLElement::SetAttr [nsGenericHTMLElement.cpp:1185]
nsGenericHTMLElement::SetAttr [nsGenericHTMLElement.h:167]
nsGenericHTMLElement::SetAttribute [nsGenericHTMLElement.cpp:352]
nsHTMLInputElement::SetAttribute [nsHTMLInputElement.h:136]
nsIDOMElement_SetAttribute [dom_quickstubs.cpp:4918]
[javascript]
blocking2.0: --- → ?
Assignee: nobody → mounir.lamouri
blocking2.0: ? → final+
OS: Mac OS X → All
Hardware: x86 → All
Attached patch Patch v1Splinter Review
This patch is the safest thing we can do. Another solution would be to have form controls not notify the form when their validity changes because of a type change but there is a lot of call of SetBarredFromConstraintValidation. That would be worse to add an attribute here for just two calls.

The best solution would be to not add and remove form controls when there type change but call something like "UpdateElement" which wouldn't call the form validity stuff. Could be done with bug 600754.
Attachment #479666 - Flags: review?(Olli.Pettay)
Status: NEW → ASSIGNED
So why are we getting the assertion?
(In reply to comment #2)
> So why are we getting the assertion?

The form update the count of invalid elements when an element is added or removed. When the type changes this code is also run in BeforeSetAttr and AfterSetAttr. If the element was invalid, RemoveElement decrement the counter and AfterSetAttr does nothing. However, the element, because it's type has changed, update it's validity and inform the form. So, again, the counter is decremented and it happens to be negative.
Note that the opposite situation can happen: an element is no longer barred for constraint validation because of a type change. On AddElement, the counter will be incremented and the element will inform the form. In that case, we do not assert but the counter will be equals to 2 which is wrong.

So, the two simple solutions were to make the form not update its counter on type changes or make the elements not informing the form. I think the former is simple because RemoveElement and AddElement are both called twice. In addition, the best fix, I think, would be to have some kind of UpdateElement in BeforeSetAttr which would move the element from the two lists if needed according to the current type and the new one.
It is still unclear to me why Add/RemoveElement can't handle this situation
automatically.
Comment on attachment 479666 [details] [diff] [review]
Patch v1

But ok, this is reasonable simple way to fix this.
Attachment #479666 - Flags: review?(Olli.Pettay) → review+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/aa06119e745f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.