[webkit-reviews] review granted: [Bug 136784] Make all CSSSelector data members private : [Attachment 238036] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 136784: Make all CSSSelector data members private
https://bugs.webkit.org/show_bug.cgi?id=136784

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=238036&action=review


Really neat. That is something I wanted for a while but I was too lazy to fix
everything :)

It is a little sad we need the public setters for the parser. I guess in the
future we could refactor those out.

> Source/WebCore/css/CSSSelector.cpp:87
>      // FIXME: Pseudo-elements and pseudo-classes do not have the same
specificity. This function
>      // isn't quite correct.

Hum, we should fix that at some point.

> Source/WebCore/css/CSSSelector.h:211
> +	       ASSERT(m_pseudoType == pseudoElementType);

I assume you are asserting in case the type become larger than the bit field?

> Source/WebCore/css/CSSSelector.h:214
> +	   void setPagePseudoType(PagePseudoClassType pagePseudoType)

You could move this one next to its getter.