[webkit-reviews] review denied: [Bug 10687] object embedded in HTML: default background should be transparent. : [Attachment 68511] Patch for making SVG documents and XML documents with SVG root transparent when embedded by reference.

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Jeff Schiller
<jeffschiller at google.com>'s request for review:
Bug 10687: object embedded in HTML: default background should be transparent.
https://bugs.webkit.org/show_bug.cgi?id=10687

Attachment 68511: Patch for making SVG documents and XML documents with SVG
root transparent when embedded by reference.
https://bugs.webkit.org/attachment.cgi?id=68511&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=68511&action=review

> WebCore/dom/Document.cpp:2171
> +#endif

I don't think you need to cache this information.  Asking if the
documentElement() has the SVG tag name every time is fine.  It's a quick check
(it's AtomicString comparisons, which are very fast).

If you do still want to cache it, a better place would be in
cacheDocumentElement().

> WebCore/dom/Document.h:388
> +    virtual bool hasSVGRootNode() const { return m_hasSVGRootNode; }

I don't think this needs to be virtual, since the same check that works for
non-SVG documents (checking the root node) should also work for SVG documents. 
It's faster to just do the root node tag check than it is to call a virtual
function.

> WebCore/svg/SVGDocument.h:55
> +	   virtual bool hasSVGRootNode() const { return true; }

Just remove this.