[webkit-reviews] review denied: [Bug 52085] Add Document::setContent() : [Attachment 78336] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 52085: Add Document::setContent()
https://bugs.webkit.org/show_bug.cgi?id=52085

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78336&action=review

I think this needs one more round.  I like where this is going though!

> Source/WebCore/dom/Document.cpp:1179
> +    return wellFormed();

Is wellFormed what we'd want for non-XML content?  Is this an XML-only method?

> Source/WebCore/dom/Document.cpp:1186
> +    // FIXME: We should pass content directly to the parser insted of
decoding it here.
> +    // At the moment only SVGFonts use this method, so the xml mime type is
ok for now.
> +    setDecoder(TextResourceDecoder::create("application/xml"));

Why should we pass it directly to the parser?

It seems it would be better to put this decoding code in the SVGImage code for
now, unless you're about to post a follow-up patch to not decode?

> Source/WebCore/dom/Document.h:353
> +    bool setContent(const char*, int);

Don't you want size_t here?