Bug 138769 – Implement :valid and :invalid matching for the fieldset element

• Component: New Bugs
• Status: RESOLVED FIXED

[Benjamin Poulain]

Implement :valid and :invalid matching for the fieldset element

[Benjamin Poulain]

Created attachment 241669 [details]
Patch

[Darin Adler]

Comment on attachment 241669 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241669&action=review

> Source/WebCore/ChangeLog:9
> +        a fielset element based on its descendants:

Typo: fieldset

> Source/WebCore/ChangeLog:35
> +        With the validity state fully encapsulated in HTMLForlControlElement, all I need

Typo: HTMLFormControlElement.

> Source/WebCore/html/HTMLFormControlElement.cpp:407
> +static void addInvalidElementToAncestors(HTMLFormControlElement& element)

Seems a shame to not share code with addInvalidElementToAncestorFromInsertionPoint. Can we refactor so they don’t have to be entirely separate? It’s annoying to have four separate functions, not even all that close to each other in the file, all knowing the special rule about HTMLFieldSetElement. Would be nicer to have just two.

> Source/WebCore/html/HTMLFormControlElement.cpp:473
>  void HTMLFormControlElement::setNeedsValidityCheck()

The name of this function is not good. It should be called updateValidity, since it does this, synchronously. It doesn’t just record that there’s a need for a validity check later!

> Source/WebCore/html/HTMLFormControlElement.h:202
> +inline bool HTMLFormControlElement::isValidFormControlElement() const
> +{
> +    // If the following assertion fails, setNeedsValidityCheck() is not called
> +    // correctly when something which changes validity is updated.
> +    ASSERT(m_isValid == valid());
> +    return m_isValid;
> +}

This doesn’t need to be in the header file. Lets move it to the .cpp file.

> Source/WebCore/html/HTMLKeygenElement.h:40
> +    virtual bool recalcWillValidate() const override { return false; }

Not a great name for this function. Elsewhere we use “compute” instead of “recalc” for function names like this one.

[Benjamin Poulain]

Committed r176174: <http://trac.webkit.org/changeset/176174>