[webkit-reviews] review denied: [Bug 83446] Broken handling of pseudo-elements in selectors API : [Attachment 139984] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Arpita Bahuguna
<arpitabahuguna at gmail.com>'s request for review:
Bug 83446: Broken handling of pseudo-elements in selectors API
https://bugs.webkit.org/show_bug.cgi?id=83446

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=139984&action=review


r-, I 'd still like to see another version with style fixes.

> Source/WebCore/css/SelectorChecker.cpp:529
> +	   if ((context.elementStyle || m_selectorCheckerMode !=
ResolvingStyleRules) && dynamicPseudo != NOPSEUDO && dynamicPseudo != SELECTION


I think you should explicitly test against the modes that you want the behavior
in instead of using !=

> Source/WebCore/css/SelectorChecker.h:55
> +    enum Mode { ResolvingStyleRules = 0, CollectingRules, QueryingRules };

I  think my proposal ResolvingStyle was better. We don't really resolve style
rules, we resolve (element) style.

> Source/WebCore/css/SelectorChecker.h:101
> +    Mode selectorCheckerMode() const { return m_selectorCheckerMode; }
> +    void setSelectorCheckerMode(Mode mode) { m_selectorCheckerMode = mode; }


SelectorChecker::selectorCheckerMode and SelectorChecker::m_selectorCheckerMode
are similarly redundant. -> mode(), m_mode