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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 24 00:33:00 PST 2008

Eric Seidel <eric at webkit.org> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 16980: Support external SVG Fonts

Attachment 18631: Initial patch

------- 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:
+		 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?

More information about the webkit-reviews mailing list