Bug 5978 – WebKIt+SVG should use SVGDocumentImpl for image/svg+xml

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Eric Seidel (no email)]

WebKIt+SVG should use SVGDocumentImpl for image/svg+xml

Currently we use DocumentImpl for everything.  When we're actually sent a file with the SVG MIME type, I 
believe we are supposed to instantiate an SVGDocumentImpl instead, and default unnamespaced elements 
to the svg namespace.

[Eric Seidel (no email)]

Specifically we need to support these additional interfaces:
http://www.w3.org/TR/SVG/struct.html#InterfaceSVGDocument

it seems "rootElement" is the only real addition.

[Rob Buis]

Created attachment 6947 [details]
My first try at this...

Builds using SVGDocumentImpl when svg mimetype is encountered.

[Rob Buis]

Created attachment 8048 [details]
Work in progress patch

I noticed the old patch didnt compile, this one does. It tries a new
approach, unfortunately while it fixes the testcase it fails the
webkit tests... Still trying to figure out why.
Cheers,

Rob.

[Rob Buis]

Created attachment 8396 [details]
Cleaner patch where run-webkit-tests does not crash

With this patch, the run-webkit-tests do not crash anymore. Handling of title
is left to WebCore:Document. Also handling of scripts is left to it.
Cheers,

Rob.

[Darin Adler]

Comment on attachment 8396 [details]
Cleaner patch where run-webkit-tests does not crash

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.

[Rob Buis]

Created attachment 8418 [details]
Improved patch

Hi Darin,

I tried to correct the mistakes, most were there because I had to rush the patch last night.
I think we can remove the unload dispatch since that is probably handled in the Frame.
The isSVGDocument is from an unrelated patch, I removed it.
Cheers,

Rob.

[Darin Adler]

Comment on attachment 8418 [details]
Improved patch

r=me