[webkit-reviews] review denied: [Bug 5978] WebKIt+SVG should use SVGDocumentImpl for image/svg+xml : [Attachment 8396] Cleaner patch where run-webkit-tests does not crash

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Thu May 18 20:55:37 PDT 2006


Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 5978: WebKIt+SVG should use SVGDocumentImpl for image/svg+xml
http://bugzilla.opendarwin.org/show_bug.cgi?id=5978

Attachment 8396: Cleaner patch where run-webkit-tests does not crash
http://bugzilla.opendarwin.org/attachment.cgi?id=8396&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Generally we don't like to check in commented-out or #if 0'd out code. I'd like
to understand why we need to do that here, such as in ~SVGDocument and
SVGDocument::createElementNS.

We also don't use braces around single line if statements like this one:

+  if (d->m_request.m_responseMIMEType == "image/svg+xml") {
+    d->m_doc =
SVGDOMImplementation::instance()->createDocument(d->m_view.get());
+  } else

In SVGDOMImplementation, s_instance is no longer used so it should be removed.

What is isSVGDocument for? You add it here, but I don't see any uses of it.

Otherwise looks good.



More information about the webkit-reviews mailing list