[webkit-reviews] review denied: [Bug 16980] Support external SVG Fonts : [Attachment 18631] Initial patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 16980: Support external SVG Fonts
http://bugs.webkit.org/show_bug.cgi?id=16980

Attachment 18631: Initial patch
http://bugs.webkit.org/attachment.cgi?id=18631&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
Ok a few issues with this first patch:

Is it possible for m_fontElement not to be reset if a SVGFontFace element was
moved out of a <font> element between rebuildFontFace() calls?	I know that's
an edge case, but I wouldn't want it to not get reset and us to crash or some
other strangeness.

You probably should use KURL for this url parsing:
+		 // URL scheme example:
"file:///Users/nikoz/WebKit/LayoutTests/svg/W3C-SVG-1.1/resources/ext-TestComic
.svg#Font"
+		 int nameStart = fontName.find('#');
+		 if (nameStart != -1)
+		     fontName = fontName.substring(nameStart + 1,
fontName.length() - (nameStart + 1));

Are there obscure cases where we would need to decode the anchor before
performing lookup in the remote document?

Blindly adopting nodes from some external document into this one seems like a
bad idea.  What if those nodes were <script> nodes?  I'm not sure if scripts in
shadow trees run... i would hope not.  It's not entirely clear to me why you'd
want to adopt the font element into the document anyway (instead of just
holding it in the cache).  What happens when an SVG font gets reused from the
cache?	Does it work on reuse?