[Webkit-unassigned] [Bug 75302] ShadowContentElement should be able to use query.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=75302


MORITA Hajime <morrita at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #121828|review?                     |review-
               Flag|                            |




--- Comment #22 from MORITA Hajime <morrita at google.com>  2012-01-11 18:02:49 PST ---
(From update of attachment 121828)
View in context: https://bugs.webkit.org/attachment.cgi?id=121828&action=review

> Source/WebCore/dom/SelectorQuery.cpp:47
> +    m_selectors.clear();

Do we need clear() ? I guess we can just assert empty()-ness.

> Source/WebCore/dom/SelectorQuery.cpp:82
>  bool SelectorQuery::canUseIdLookup() const
>      // we would need to sort the results. For now, just traverse the document in that case.

It looks canUseIdLookUp() can be moved to SelectorDataList.

> Source/WebCore/dom/SelectorQuery.cpp:-100
> -                if (m_selectorChecker.checkSelector(m_selectors[i].selector, element, m_selectors[i].isFastCheckable)) {

I guess this check function can be encapsulated inside SelectorDataList so that we can remove an per-item accessor.

> Source/WebCore/dom/SelectorQuery.h:48
> +    CSSSelector* selector(int nth) const { return m_selectors[nth].selector; }

selectorAt() ?

> Source/WebCore/dom/ShadowContentSelectorQuery.cpp:72
> +bool ShadowContentSelectorQuery::matches(Node* targetNode, CSSSelector* selector, bool isFastCheckable) const

I hope this can be part of SelectorDataList to hide per-element accessor.

> Source/WebCore/dom/ShadowContentSelectorQuery.cpp:76
> +    if (!targetNode->isElementNode())

This check can be done by the caller.

> Source/WebCore/html/HTMLDetailsElement.cpp:68
> +const AtomicString DetailsSummaryElement::summaryQuerySelector = "summary:first-of-type";

Please make this inside some (possibly static) method to avoid static initializers.

> Source/WebCore/html/HTMLSummaryElement.cpp:46
> +        : ShadowContentElement(HTMLNames::divTag, document, WTF::emptyAtom)

You don't need WTF::. It is in using.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.