[webkit-reviews] review granted: [Bug 104685] Add Element-specific traversal functions : [Attachment 178882] patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 104685: Add Element-specific traversal functions
https://bugs.webkit.org/show_bug.cgi?id=104685

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178882&action=review


> Source/WebCore/ChangeLog:1529
> +	   Another build fix.
> +
> +	   * html/HTMLPropertiesCollection.cpp:
> +	   (WebCore::nextNodeWithProperty):

Please revert these changes.

> Source/WebCore/dom/ContainerNodeAlgorithms.h:319
> +    Element* element = (shouldIncludeRoot == IncludeRoot &&
root->isElementNode()) ? static_cast<Element*>(root) :
ElementTraversal::firstWithin(root);

I don't have toElement?

> Source/WebCore/dom/NodeTraversal.h:102
> +template <class NodeType>
> +inline Element* traverseNextElementTemplate(NodeType* current, const Node*
stayWithin)
> +{
> +    Node* node = NodeTraversal::next(current, stayWithin);
> +    while (node && !node->isElementNode())
> +	   node = NodeTraversal::nextSkippingChildren(node, stayWithin);
> +    return static_cast<Element*>(node);
> +}

I'm sad that we have to duplicate the template like this. Can we had a boolean
template argument and share the code between two?

> Source/WebCore/dom/NodeTraversal.h:126
> +template <class NodeType>
> +inline Element* traverseNextElementSkippingChildrenTemplate(NodeType*
current, const Node* stayWithin)
> +{
> +    Node* node = NodeTraversal::nextSkippingChildren(current, stayWithin);
> +    while (node && !node->isElementNode())
> +	   node = NodeTraversal::nextSkippingChildren(node, stayWithin);
> +    return static_cast<Element*>(node);
> +}

Ditto.