[webkit-reviews] review denied: [Bug 14631] <use> doesn't deep-expand <symbol> elements. : [Attachment 15530] Initial patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 14631: <use> doesn't deep-expand <symbol> elements.
http://bugs.webkit.org/show_bug.cgi?id=14631

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

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
>--- WebCore/ChangeLog	(revision 24303)
>+++ WebCore/ChangeLog	(working copy)
>+
>+	  Added testcase:
svg/custom/use-on-g-containing-foreignObject-and-image.svg (testcase for
treasure_map.svg)
>+			  svg/custom/use-on-disallowed-foreign-object-5.svg
(<use> on <symbol> containg <foreignObject>)
>+			  svg/custom/use-on-disallowed-foreign-object-6.svg
(<use> on <g> containing <svg> containing <foreignObject>)
>+			  svg/custom/use-on-disallowed-foreign-object-7.svg
(<use> on <g> containing <symbol> containing <foreignObject>)
>+			 
svg/custom/use-scripting-changes-to-nonexistant-href.svg

I wouldn't add the testcases here, but that is a taste thing I suppose.

>Index: LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image.svg
>===================================================================
>--- LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image.svg    
(revision 0)
>+++ LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image.svg    
(revision 0)
>@@ -0,0 +1,14 @@
>+<?xml version="1.0"?>
>+<svg xmlns="http://www.w3.org/2000/svg"
>+     xmlns:xlink="http://www.w3.org/1999/xlink">
>+
>+<defs>
>+    <g id="container">
>+	  <foreignObject x="10" y="10" width="580" height="380"
requiredExtensions="foobar"/>
>+	  <image xlink:href="resources/green-checker.png" width="75"
height="75" />
>+    </g>
>+</defs>
>+
>+<use x="25" y="25" xlink:href="#container"/>
>+
>+</svg>

I do not understand why you don't add a testcase like this but using <switch>
instead of <g>. AFAICS switch has some unique effect, in that it can
effectively
render the foreignObject in <use> valid, since the <switch> can decide to rule
out the foreignObject, which the above <g> doesn't do.
If we can work that one out (over irc maybe), then I'll r+ it.