[webkit-reviews] review denied: [Bug 77525] SelectorChecker::checkSelector: move parameters into a struct : [Attachment 125283] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Feb 3 01:32:03 PST 2012
Antti Koivisto <koivisto at iki.fi> has denied Roland Steiner
<rolandsteiner at chromium.org>'s request for review:
Bug 77525: SelectorChecker::checkSelector: move parameters into a struct
https://bugs.webkit.org/show_bug.cgi?id=77525
Attachment 125283: Patch
https://bugs.webkit.org/attachment.cgi?id=125283&action=review
------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=125283&action=review
> Source/WebCore/css/CSSStyleSelector.cpp:2057
> + SelectorChecker::SelectorCheckingContext context = {
> + ruleData.selector(), // m_selector
> + m_element, // m_element
> + false, // m_isSubSelector
> + SelectorChecker::VisitedMatchEnabled, // m_visitedMatchType
> + style(), // m_elementStyle
> + m_parentNode ? m_parentNode->renderStyle() : 0 //
m_elementParentStyle
> + };
You should initialize using dot notation so yo don't need these comments
context.foo = bar;
SelectorCheckingContext should have constructor that initializes the mandatory
stuff (selector and element at least) and sets the rest to defaults.
Drop the m_ prefixes from the field names (those are not used with public
structs).
> Source/WebCore/css/SelectorChecker.cpp:454
> +SelectorChecker::SelectorMatch
SelectorChecker::checkSelector(SelectorCheckingContext context, PseudoId&
dynamicPseudo) const
This should use const SelectorCheckingContext&. To modify the context you
should make a copy explicitly.
More information about the webkit-reviews
mailing list