[webkit-reviews] review denied: [Bug 3249] Investigate merging the WebCore DOM and KDOM : [Attachment 4706] khtml & kwq side of the merger

[bugzilla-request-daemon at opendarwin.org]

Dave Hyatt <hyatt at apple.com> has denied Eric Seidel <macdome at opendarwin.org>'s
request for review:
Bug 3249: Investigate merging the WebCore DOM and KDOM
http://bugzilla.opendarwin.org/show_bug.cgi?id=3249

Attachment 4706: khtml & kwq side of the merger
http://bugzilla.opendarwin.org/attachment.cgi?id=4706&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
(1) You need to add the SVG MIME type to khtml/ecma/xmlhttprequest.cpp.

(2) I don't like the name KRenderingDeviceQuartz used directly in RenderCanvas.
 I have several problems with this name.  First of all, the convention for
RenderTheme was to use RenderThemeMac (not Quartz).  I think it's better to
name the platform rather than the graphics library for platform-specific code. 
Second is this object really even platform-specific?  If it is, then
RenderCanvas should not contain direct references to platform-specific objects.
 If it isn't, then the file is misnamed.

(3) I think you should rename your KCanvasContainers to SVGRenderContainers and
the KCanvasRegistry to something like the SVGRenderingRegistry.

(4) You make an SVGRenderStyle for every style object.	These should either be
lazily allocated or the SVG styles should use DataRef structs like the other
CSS structs for efficient sharing.

(5) The SVG <style> tag support has a funny code style that doesn't match
guidelines.  No spaces after the ifs, braces on the wrong lines, etc.

(6) Typo in rendererIsNeeded:

"+    // SVG ignores arbitrary xml elements in its render tree in contrary to
the normal CSS/XML behavior.
"

Change "in contrary" to just be "contrary"