Bug 34733

I checked the current HTML5 draft, and I think we don't need to restrict validation to form controls inside a <form> element. Opera 10.10 updates :valid/:invalid for form controls without <form>.

(In reply to comment #1) > I checked the current HTML5 draft, and I think we don't need to restrict > validation to form controls inside a <form> element. I may be wrong but as far as I remember those two pseudoclasses apply only to "validable" elements; to be validable an element should be bound to a form, otherwise is de-facto non-validable (and, thus, can't be neither :valid or :invalid). CC'ing Hixie

> I may be wrong but as far as I remember those two pseudoclasses apply only to > "validable" elements; to be validable an element should be bound to a form, > otherwise is de-facto non-validable (and, thus, can't be neither :valid or > :invalid). I didn't find any clues for requirements of form element and name attribute. So I'll remove them from HTMLFormControlElement::willValidate().

Created attachment 48464 [details] Proposed patch

Created attachment 48465 [details] Proposed patch (rev.2)

Comment on attachment 48465 [details] Proposed patch (rev.2) Thanks and LGTM A reviewer still has to review this though.

I found. http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#association-of-controls-and-forms > Constraint validation: If an element has no form owner, it is barred > from constraint validation. http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#naming-form-controls > Constraint validation: If an element does not have a name attribute > specified, or its name attribute's value is the empty string, then > it is barred from constraint validation. http://www.whatwg.org/specs/web-apps/current-work/multipage/links.html#pseudo-classes > :valid > The :valid pseudo-class must match all elements that are candidates for > constraint validation and that satisfy their constraints. > > :invalid > The :invalid pseudo-class must match all elements that are candidates > for constraint validation but that do not satisfy their constraints. http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#candidate-for-constraint-validation > A listed form-associated element is a candidate for constraint > validation except when a condition has barred the element from > constraint validation. So, the current implementation conforms to the specification. Erik, if we want to change the behavior, we need to propose the change to WHATWG.

The specification has been changed so that existens of owner form and non-empty name attribute are ignorable.

Created attachment 52648 [details] Proposed patch (rev.3)

Comment on attachment 52648 [details] Proposed patch (rev.3) > - , m_willValidate(false) > + , m_willValidateInitialized(false) > + , m_willValidate(true) I am not sure that "initialized" is the right word here. Since you called the function "recalc" maybe you should reverse the sense and call it "m_willValidateNeedsRecalc" or "m_needsWillValidateRecalc"? Or maybe this is needed only during construction. Maybe we can make m_willValidateInitialized be a debug-only concept? > // FIXME: Check if the control does not have a datalist element as an ancestor. > - return m_form && m_hasName && !m_disabled && !m_readOnly; > + return !m_disabled && !m_readOnly; It would be good to make that existing FIXME clearer and more specific. Check that, and do what? Why check it? > bool newWillValidate = recalcWillValidate(); > - if (m_willValidate == newWillValidate) > + if (m_willValidateInitialized && m_willValidate == newWillValidate) > return; > + m_willValidateInitialized = true; This seems a little strange. Do we really need to recalc immediately here? There at least needs to be a comment explaining why this doesn't just set m_willValidateInitialized to false and return. r=me

Attachment 52648 [details] was posted by a committer and has review+, assigning to Kent Tamura for commit.

Thank you for reviewing. (In reply to comment #10) > (From update of attachment 52648 [details]) > > - , m_willValidate(false) > > + , m_willValidateInitialized(false) > > + , m_willValidate(true) > > I am not sure that "initialized" is the right word here. Since you called the > function "recalc" maybe you should reverse the sense and call it > "m_willValidateNeedsRecalc" or "m_needsWillValidateRecalc"? Or maybe this is > needed only during construction. Maybe we can make m_willValidateInitialized be > a debug-only concept? This is needed only during construction. So I think m_willValidateNeedsRecalc and m_needsWillValidateRecalc are not suitable. > > // FIXME: Check if the control does not have a datalist element as an ancestor. > > - return m_form && m_hasName && !m_disabled && !m_readOnly; > > + return !m_disabled && !m_readOnly; > > It would be good to make that existing FIXME clearer and more specific. Check > that, and do what? Why check it? Ok, I added what we should do and the reason. > > bool newWillValidate = recalcWillValidate(); > > - if (m_willValidate == newWillValidate) > > + if (m_willValidateInitialized && m_willValidate == newWillValidate) > > return; > > + m_willValidateInitialized = true; > > This seems a little strange. Do we really need to recalc immediately here? Yes. if willValidate value is changed, we need to add or remove :valid :invalid pseudo selectors. I added a comment. I'll commit this change with additional comments.

Landed as r57274.

http://trac.webkit.org/changeset/57274 might have broken SnowLeopard Intel Release (Tests)

Looks like it might be a real breakage: http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r57274%20(5752)/fast/css/pseudo-invalid-novalidate-001-pretty-diff.html

(In reply to comment #15) > Looks like it might be a real breakage: > http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r57274%20(5752)/fast/css/pseudo-invalid-novalidate-001-pretty-diff.html I'm investigating.

(In reply to comment #16) > (In reply to comment #15) > > Looks like it might be a real breakage: > > http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r57274%20(5752)/fast/css/pseudo-invalid-novalidate-001-pretty-diff.html > > I'm investigating. Ok, I have understood. I'll fix this soon.

r57279 fixes the test failure.

Thanks!