[webkit-reviews] review denied: [Bug 47749] serializeNodesWithNamespaces should be in MarkupAccumulator : [Attachment 70925] cleanup

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied  review:
Bug 47749: serializeNodesWithNamespaces should be in MarkupAccumulator
https://bugs.webkit.org/show_bug.cgi?id=47749

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70925&action=review

> WebCore/editing/MarkupAccumulator.cpp:129
> +size_t MarkupAccumulator::sumOfStringLengths(const Vector<String>& strings)
const

This method doesn't need to be part of MarkupAccumulator.  You can make it a
static function (outside the class).

> WebCore/editing/MarkupAccumulator.h:114
> +    void serializeNodes(Node*, Node* nodeToSkip, EChildrenOnly, const
Namespaces*);

Shouldn't this return a String like your ChangeLog says?

> WebCore/editing/markup.cpp:734
> +    return accumulator.serializeNodes(const_cast<Node*>(node),
deleteButtonContainerElement, childrenOnly);

Doesn't serializeNodes return void and we want to return a string?