[webkit-reviews] review granted: [Bug 27357] Support :valid/:invalid CSS selectors : [Attachment 34943] Patch v1

[bugzilla-daemon at webkit.org]

Adele Peterson <adele at apple.com> has granted Michelangelo De Simone
<micdesim at gmail.com>'s request for review:
Bug 27357: Support :valid/:invalid CSS selectors
https://bugs.webkit.org/show_bug.cgi?id=27357

Attachment 34943: Patch v1
https://bugs.webkit.org/attachment.cgi?id=34943&action=review

------- Additional Comments from Adele Peterson <adele at apple.com>
r=me.  A few comments about the log.

> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index da90953..e7dbea2 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,44 @@
> +2009-08-16  Michelangelo De Simone  <micdesim at gmail.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=27357
> +	   Support for :valid and :invalid CSS selectors, from HTML5 specs.
> +	   http://www.whatwg.org/specs/web-apps/current-work/#selector-valid
> +
> +	   Tests: fast/css/pseudo-invalid-001.html
> +		  fast/css/pseudo-invalid-002.html
> +		  fast/css/pseudo-valid-001.html
> +		  fast/css/pseudo-valid-002.html
> +		  fast/css/pseudo-valid-003.html
> +		  fast/css/pseudo-valid-004.html
> +		  fast/css/pseudo-valid-005.html
> +		  fast/css/pseudo-valid-006.html
> +		  fast/css/pseudo-valid-007.html
> +
> +	   * css/CSSSelector.cpp:
> +	   (WebCore::CSSSelector::extractPseudoType): extracts new pseudos

should say "pseudo-classes" instead of "pseudos" throughout the log

> +	   * css/CSSSelector.h:
> +	   (WebCore::CSSSelector::):
> +	   * css/CSSStyleSelector.cpp:
> +	   (WebCore::CSSStyleSelector::canShareStyleWithElement): style sharing

> +	   mechanism rests for valid/invalid form control elements

I think you want to say something like "Make sure we don't share style between
form controls that have different validity states" 

> +	   (WebCore::CSSStyleSelector::SelectorChecker::checkOneSelector):
applies
> +	   style associated to new pseudos
> +	   * dom/Element.h:
> +	   (WebCore::Element::willValidate): pushed up in hiearchy

typo.

> +	   (WebCore::Element::isValidFormControlElement): checks for valid form

> +	   control elements (wraps ValidityState::valid())
> +	   * html/HTMLFormControlElement.cpp:
> +	   (WebCore::HTMLFormControlElement::willValidate): minor clean up
> +	   (WebCore::HTMLFormControlElement::isValidFormControlElement): ditto
> +	   * html/HTMLFormControlElement.h:
> +	   * html/HTMLKeygenElement.h:
> +	   (WebCore::HTMLKeygenElement::willValidate): added
> +	   * html/HTMLObjectElement.h:
> +	   (WebCore::HTMLObjectElement::willValidate): added
> +	   * html/HTMLObjectElement.idl:
> +
>  2009-08-16  Nikolas Zimmermann  <nikolas.zimmermann at torchmobile.com>
>  
>	   Reviewed by George Staikos.