Reviewed by Rob.

Fixes: http://bugs.webkit.org/show_bug.cgi?id=13611 (Crash in setAttributeNS setting href of SVG <use> to nonexistent symbol)
Fixes: http://bugs.webkit.org/show_bug.cgi?id=14631 (<use> doesn't deep-expand <symbol> elements.)
Rework <use> on <foreignObject> cases, to not just ignore these cases, but actually proceed and skip <fO> objects
from the resulting cloned tree. This fixes parts of "treasure_map.svg" (no bug report availabe on that one, private "testcase").

Fix assertion happening with <use> on <g> containing <symbol>. Introduce expandSymbolElementsInShadowTree()
concept, just like it's done for <use> on <use>, to deep-replace all <symbol> elements by <svg>, as demanded
by the spec. This only worked on <use> on <symbol> direct cases so far.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@26426 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index ac56c33bf4fb4c04ec050ecca8f65e2590ee5525..de2f92c726d354f86c8558595ece09f9d73d8df7 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,27 @@
+2007-07-16  Nikolas Zimmermann  <zimmermann@kde.org>
+
+        Reviewed by Rob.
+
+        Land new layout tests related to new <use> behaviour on disallowed elements & symbols.
+        Add new layout tests checking new <use> on <foreignObject> behaviour
+    
+        * svg/custom/use-scripting-changes-to-nonexistant-href-expected.checksum: Added.
+        * svg/custom/use-scripting-changes-to-nonexistant-href-expected.png: Added.
+        * svg/custom/use-scripting-changes-to-nonexistant-href-expected.txt: Added.
+        * svg/custom/use-scripting-changes-to-nonexistant-href.svg: Added.
+        * svg/custom/use-on-g-containing-foreignObject-and-image-expected.checksum: Added.
+        * svg/custom/use-on-g-containing-foreignObject-and-image-expected.png: Added.
+        * svg/custom/use-on-g-containing-foreignObject-and-image-expected.txt: Added.
+        * svg/custom/use-on-g-containing-foreignObject-and-image.svg: Added.
+        * svg/custom/use-on-disallowed-foreign-object-5-expected.checksum: Added.
+        * svg/custom/use-on-disallowed-foreign-object-5-expected.png: Added.
+        * svg/custom/use-on-disallowed-foreign-object-5-expected.txt: Added.
+        * svg/custom/use-on-disallowed-foreign-object-5.svg: Added.
+        * svg/custom/use-on-disallowed-foreign-object-6-expected.checksum: Added.
+        * svg/custom/use-on-disallowed-foreign-object-6-expected.png: Added.
+        * svg/custom/use-on-disallowed-foreign-object-6-expected.txt: Added.
+        * svg/custom/use-on-disallowed-foreign-object-6.svg: Added.
+
 2007-07-15  Nikolas Zimmermann  <zimmermann@kde.org>
 
         Reviewed by Rob.

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-1-expected.txt b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-1-expected.txt
index 5919608848baad09f7c65435060a9f22402d0242..6497587fbaa1d646163105a8725a0759331cc1e8 100644 (file)
--- a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-1-expected.txt
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-1-expected.txt
@@ -7,3 +7,4 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,0) size 244x18
           text run at (0,0) width 244: "You should only see this string ONCE"
     RenderSVGContainer {use} at (250,-50) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-50.00)}]
+      RenderSVGContainer {g} at (250,-35.86) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-2-expected.txt b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-2-expected.txt
index b634685265fe959731c4f47d131faef4707388eb..4ca5ccf45d9fe50857c663376d22094676e80a97 100644 (file)
--- a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-2-expected.txt
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-2-expected.txt
@@ -9,3 +9,6 @@ layer at (0,0) size 800x600
             RenderText {#text} at (0,0) size 244x18
               text run at (0,0) width 244: "You should only see this string ONCE"
     RenderSVGContainer {use} at (250,-50) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-50.00)}]
+      RenderSVGContainer {g} at (250,-35.86) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
+        RenderSVGContainer {g} at (250,-35.86) size 0x0
+          RenderSVGContainer {g} at (250,-35.86) size 0x0

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-3-expected.txt b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-3-expected.txt
index f9877f5574b571c5f501fdfbb1da40c58f8a2fd5..b7ddab27ffe24f3716329cd5831413eb96a0dc17 100644 (file)
--- a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-3-expected.txt
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-3-expected.txt
@@ -7,4 +7,7 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,0) size 244x18
           text run at (0,0) width 244: "You should only see this string ONCE"
     RenderSVGContainer {use} at (250,-50) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-50.00)}]
+      RenderSVGContainer {g} at (250,-35.86) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
     RenderSVGContainer {use} at (250,-50) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-50.00)}]
+      RenderSVGContainer {g} at (250,-35.86) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
+        RenderSVGContainer {g} at (452.13,115.56) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-35.86)}]

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-4-expected.txt b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-4-expected.txt
index 86ea2fea97bc5e102b1a6578588896499089e53f..919de411c683910efcc1eba8775111b5ffedd826 100644 (file)
--- a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-4-expected.txt
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-4-expected.txt
@@ -9,4 +9,11 @@ layer at (0,0) size 800x600
             RenderText {#text} at (0,0) size 244x18
               text run at (0,0) width 244: "You should only see this string ONCE"
     RenderSVGContainer {use} at (250,-50) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-50.00)}]
+      RenderSVGContainer {g} at (250,-35.86) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
+        RenderSVGContainer {g} at (250,-35.86) size 0x0
+          RenderSVGContainer {g} at (250,-35.86) size 0x0
     RenderSVGContainer {use} at (250,-50) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-50.00)}]
+      RenderSVGContainer {g} at (250,-35.86) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
+        RenderSVGContainer {g} at (452.13,115.56) size 0x0 [transform={m=((0.71,0.71)(-0.71,0.71)) t=(250.00,-35.86)}]
+          RenderSVGContainer {g} at (452.13,115.56) size 0x0
+            RenderSVGContainer {g} at (452.13,115.56) size 0x0

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.checksum b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.checksum
new file mode 100644 (file)
index 0000000..b37244e
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.checksum
@@ -0,0 +1 @@
+e81802f52a73f3e6016c64d62aa87239
\ No newline at end of file

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.png b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.png
new file mode 100644 (file)
index 0000000..898ba89
Binary files /dev/null and b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.png differ

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.txt b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.txt
new file mode 100644 (file)
index 0000000..914ef48
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5-expected.txt
@@ -0,0 +1,16 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderSVGRoot {svg} at (0,0) size 0x0
+    RenderSVGHiddenContainer {defs} at (0,0) size 0x0
+    RenderForeignObject {foreignObject} at (0,0) size 580x380
+      RenderBlock {xhtml:div} at (0,0) size 580x18
+        RenderText {#text} at (0,0) size 244x18
+          text run at (0,0) width 244: "You should only see this string ONCE"
+    RenderSVGContainer {use} at (0,0) size 0x0
+      RenderSVGContainer {g} at (25,25) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(25.00,25.00)}]
+        RenderSVGContainer {svg} at (25,25) size 0x0
+    RenderSVGContainer {use} at (0,0) size 0x0
+      RenderSVGContainer {g} at (10,10) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
+        RenderSVGContainer {g} at (35,35) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(25.00,25.00)}]
+          RenderSVGContainer {svg} at (35,35) size 0x0

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5.svg b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5.svg
new file mode 100644 (file)
index 0000000..a4a6bfa
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-5.svg
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     xmlns:xhtml="http://www.w3.org/1999/xhtml">
+
+<defs>
+    <symbol id="symbol" overflow="visible">
+        <foreignObject x="10" y="10" width="580" height="380" transform="scale(4) skewY(5) skewX(5)">
+        <xhtml:div>You should only see this string ONCE</xhtml:div>
+        </foreignObject>
+    </symbol>
+</defs>
+
+<foreignObject x="10" y="10" width="580" height="380" transform="scale(5) skewY(5) skewX(5)">
+<xhtml:div>You should only see this string ONCE</xhtml:div>
+</foreignObject>
+
+<use id="test" x="25" y="25" xlink:href="#symbol"/>
+<use x="10" y="10" xlink:href="#test"/>
+
+</svg>

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.checksum b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.checksum
new file mode 100644 (file)
index 0000000..b37244e
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.checksum
@@ -0,0 +1 @@
+e81802f52a73f3e6016c64d62aa87239
\ No newline at end of file

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.png b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.png
new file mode 100644 (file)
index 0000000..898ba89
Binary files /dev/null and b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.png differ

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.txt b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.txt
new file mode 100644 (file)
index 0000000..6c3ffaf
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6-expected.txt
@@ -0,0 +1,21 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderSVGRoot {svg} at (0,0) size 0x0
+    RenderSVGHiddenContainer {defs} at (0,0) size 0x0
+      RenderSVGContainer {svg} at (0,0) size 0x0
+        RenderForeignObject {foreignObject} at (0,0) size 580x380
+          RenderBlock {xhtml:div} at (0,0) size 580x18
+            RenderText {#text} at (0,0) size 244x18
+              text run at (0,0) width 244: "You should only see this string ONCE"
+    RenderForeignObject {foreignObject} at (0,0) size 580x380
+      RenderBlock {xhtml:div} at (0,0) size 580x18
+        RenderText {#text} at (0,0) size 244x18
+          text run at (0,0) width 244: "You should only see this string ONCE"
+    RenderSVGContainer {use} at (0,0) size 0x0
+      RenderSVGContainer {g} at (25,25) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(25.00,25.00)}]
+        RenderSVGContainer {svg} at (25,25) size 0x0
+    RenderSVGContainer {use} at (0,0) size 0x0
+      RenderSVGContainer {g} at (10,10) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(10.00,10.00)}]
+        RenderSVGContainer {g} at (35,35) size 0x0 [transform={m=((1.00,0.00)(0.00,1.00)) t=(25.00,25.00)}]
+          RenderSVGContainer {svg} at (35,35) size 0x0

diff --git a/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6.svg b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6.svg
new file mode 100644 (file)
index 0000000..abbd1fe
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-disallowed-foreign-object-6.svg
@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+<svg xmlns="http://www.w3.org/2000/svg"
+     xmlns:xlink="http://www.w3.org/1999/xlink"
+     xmlns:xhtml="http://www.w3.org/1999/xhtml">
+
+<defs>
+    <svg id="svg" overflow="visible">
+        <foreignObject x="10" y="10" width="580" height="380" transform="scale(4) skewY(5) skewX(5)">
+        <xhtml:div>You should only see this string ONCE</xhtml:div>
+        </foreignObject>
+    </svg>
+</defs>
+
+<foreignObject x="10" y="10" width="580" height="380" transform="scale(5) skewY(5) skewX(5)">
+<xhtml:div>You should only see this string ONCE</xhtml:div>
+</foreignObject>
+
+<use id="test" x="25" y="25" xlink:href="#svg"/>
+<use x="10" y="10" xlink:href="#test"/>
+
+</svg>

diff --git a/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.checksum b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.checksum
new file mode 100644 (file)
index 0000000..44b7017
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.checksum
@@ -0,0 +1 @@
+0f23a1fbd345da676e6bb2083c2a4af6
\ No newline at end of file

diff --git a/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.png b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.png
new file mode 100644 (file)
index 0000000..63580ac
Binary files /dev/null and b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.png differ

diff --git a/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.txt b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.txt
new file mode 100644 (file)
index 0000000..54e93c9
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image-expected.txt
@@ -0,0 +1,12 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderSVGRoot {svg} at (25,25) size 75x75
+    RenderSVGHiddenContainer {defs} at (0,0) size 0x0
+      RenderSVGContainer {g} at (0,0) size 75x75
+        RenderForeignObject {foreignObject} at (0,0) size 580x380
+        RenderImage {image} at (0,0) size 75x75
+    RenderSVGContainer {use} at (25,25) size 75x75
+      RenderSVGContainer {g} at (25,25) size 75x75 [transform={m=((1.00,0.00)(0.00,1.00)) t=(25.00,25.00)}]
+        RenderSVGContainer {g} at (25,25) size 75x75
+          RenderImage {image} at (0,0) size 75x75

diff --git a/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image.svg b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image.svg
new file mode 100644 (file)
index 0000000..a158c07
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-g-containing-foreignObject-and-image.svg
@@ -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"/>
+        <image xlink:href="resources/green-checker.png" width="75" height="75" />
+    </g>
+</defs>
+
+<use x="25" y="25" xlink:href="#container"/>
+
+</svg>

diff --git a/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.checksum b/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.checksum
new file mode 100644 (file)
index 0000000..67119fe
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.checksum
@@ -0,0 +1 @@
+e33107562daa5c7a5397ed8403fdbbbc
\ No newline at end of file

diff --git a/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.png b/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.png
new file mode 100644 (file)
index 0000000..b976ca9
Binary files /dev/null and b/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.png differ

diff --git a/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.txt b/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.txt
new file mode 100644 (file)
index 0000000..be5462c
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-g-containing-symbol-expected.txt
@@ -0,0 +1,16 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderSVGRoot {svg} at (20,45) size 480x83
+    RenderSVGHiddenContainer {defs} at (0,0) size 0x0
+      RenderSVGContainer {g} at (0,0) size 0x0
+    RenderSVGContainer {g} at (245,45) size 130x30 [transform={m=((1.00,0.00)(0.00,1.00)) t=(125.00,25.00)}]
+      RenderPath {rect} at (245,45) size 130x30 [stroke={[type=SOLID] [color=#000080] [stroke width=5.00]}] [fill={[type=SOLID] [color=#FF0000]}] [data="M0.00,0.00L60.00,0.00L60.00,10.00L0.00,10.00"]
+    RenderSVGText {text} at (10,60) size 240x18 contains 1 chunk(s)
+      RenderSVGInlineText {#text} at (0,-14) size 240x18
+        chunk 1 text run 1 at (10.00,60.00) startOffset 0 endOffset 38 width 240.00: "The two objects should look identical."
+    RenderSVGContainer {use} at (45,45) size 130x30
+      RenderSVGContainer {g} at (45,45) size 130x30 [transform={m=((1.00,0.00)(0.00,1.00)) t=(25.00,25.00)}]
+        RenderSVGContainer {g} at (45,45) size 130x30
+          RenderSVGContainer {svg} at (45,45) size 130x30
+            RenderPath {rect} at (45,45) size 130x30 [stroke={[type=SOLID] [color=#000080] [stroke width=5.00]}] [fill={[type=SOLID] [color=#FF0000]}] [data="M0.00,0.00L60.00,0.00L60.00,10.00L0.00,10.00"]

diff --git a/LayoutTests/svg/custom/use-on-g-containing-symbol.svg b/LayoutTests/svg/custom/use-on-g-containing-symbol.svg
new file mode 100644 (file)
index 0000000..ef7addc
--- /dev/null
+++ b/LayoutTests/svg/custom/use-on-g-containing-symbol.svg
@@ -0,0 +1,19 @@
+<?xml version="1.0" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg width="800" height="600" viewBox="0 0 400 300" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<defs>
+    <g id="symbol">
+        <symbol overflow="visible">
+            <rect stroke-width="5px" fill="red" stroke="navy" width="60" height="10"/>
+        </symbol>
+    </g>
+</defs>
+
+<g transform="translate(125 25)">
+    <rect stroke-width="5px" fill="red" stroke="navy" width="60" height="10"/>
+</g>
+
+<text x="10" y="60">The two objects should look identical.</text>
+<use x="25" y="25" xlink:href="#symbol"/>
+</svg>
+

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index c737aaa31260780c5cf2470a490a7871e742bd10..ad4c6b5dbaa35cec134325d485b72b33b4265a10 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,34 @@
+2007-07-16  Nikolas Zimmermann  <zimmermann@kde.org>
+
+        Reviewed by Rob.
+
+        Fixes: http://bugs.webkit.org/show_bug.cgi?id=13611 (Crash in setAttributeNS setting href of SVG <use> to nonexistent symbol)
+        Fixes: http://bugs.webkit.org/show_bug.cgi?id=14631 (<use> doesn't deep-expand <symbol> elements.)
+
+        Rework <use> on <foreignObject> cases, to not just ignore these cases, but actually proceed and skip <fO> objects
+        from the resulting cloned tree. This fixes parts of "treasure_map.svg" (no bug report availabe on that one, private "testcase").
+
+        Fix assertion happening with <use> on <g> containing <symbol>. Introduce expandSymbolElementsInShadowTree()
+        concept, just like it's done for <use> on <use>, to deep-replace all <symbol> elements by <svg>, as demanded
+        by the spec. This only worked on <use> on <symbol> direct cases so far.
+
+        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
+
+        * ksvg2/svg/SVGUseElement.cpp:
+        (WebCore::isDisallowedElement):
+        (WebCore::subtreeContainsDisallowedElement):
+        (WebCore::SVGUseElement::buildPendingResource):
+        (WebCore::SVGUseElement::buildInstanceTree):
+        (WebCore::SVGUseElement::removeDisallowedElementsFromSubtree):
+        (WebCore::SVGUseElement::buildShadowTree):
+        (WebCore::SVGUseElement::expandUseElementsInShadowTree):
+        (WebCore::SVGUseElement::expandSymbolElementsInShadowTree):
+        * ksvg2/svg/SVGUseElement.h:
+
 2007-07-15  Nikolas Zimmermann  <zimmermann@kde.org>
 
         Reviewed by Rob.

diff --git a/WebCore/ksvg2/svg/SVGUseElement.cpp b/WebCore/ksvg2/svg/SVGUseElement.cpp
index 2619deddcc0d14e403812cc7ecd2122258a93a75..e9f86e79822f892a41d16b23ad056d0b8e502d10 100644 (file)
--- a/WebCore/ksvg2/svg/SVGUseElement.cpp
+++ b/WebCore/ksvg2/svg/SVGUseElement.cpp
@@ -223,9 +223,18 @@ void dumpInstanceTree(unsigned int& depth, String& text, SVGElementInstance* tar
 }
 #endif
 
+static bool isDisallowedElement(Node* element)
+{
+    // <foreignObject> should never be contained in a <use> tree. Too dangerous side effects possible.
+    if (element->hasTagName(SVGNames::foreignObjectTag))
+        return true;
+
+    return false;
+}
+
 static bool subtreeContainsDisallowedElement(Node* start)
 {
-    if (start->hasTagName(SVGNames::foreignObjectTag))
+    if (isDisallowedElement(start))
         return true;
 
     for (Node* cur = start->firstChild(); cur; cur = cur->nextSibling()) {
@@ -252,11 +261,13 @@ void SVGUseElement::buildPendingResource()
     Element* targetElement = ownerDocument()->getElementById(id); 
     SVGElement* target = svg_dynamic_cast(targetElement);
 
-    // Do not allow self-referencing. Also explicitely disallow
-    // <use> on <foreignObject>, as that could lead to nasty bugs.
+    // Do not allow self-referencing.
     // 'target' may be null, if it's a non SVG namespaced element.
-    if (!target || target == this || subtreeContainsDisallowedElement(target))
+    if (!target || target == this) {
+        m_targetElementInstance = 0;
+        m_shadowTreeRootElement = 0;
         return;
+    }
 
     // Why a seperated instance/shadow tree? SVG demands it:
     // The instance tree is accesable from JavaScript, and has to
@@ -307,6 +318,11 @@ void SVGUseElement::buildPendingResource()
     // Expand all <use> elements in the shadow tree.
     // Expand means: replace the actual <use> element by what it references.
     expandUseElementsInShadowTree(m_shadowTreeRootElement.get());
+
+    // Expand all <symbol> elements in the shadow tree.
+    // Expand means: replace the actual <symbol> element by the <svg> element.
+    expandSymbolElementsInShadowTree(m_shadowTreeRootElement.get());
+
 #endif
 
     // Now that the shadow tree is completly expanded, we can associate
@@ -356,11 +372,6 @@ void SVGUseElement::buildInstanceTree(SVGElement* target, SVGElementInstance* ta
     ASSERT(target);
     ASSERT(targetInstance);
 
-    if (subtreeContainsDisallowedElement(target)) {
-        foundProblem = true;
-        return;
-    }
-
     // A general description from the SVG spec, describing what buildInstanceTree() actually does.
     //
     // Spec: If the 'use' element references a 'g' which contains two 'rect' elements, then the instance tree
@@ -372,7 +383,7 @@ void SVGUseElement::buildInstanceTree(SVGElement* target, SVGElementInstance* ta
         SVGElement* element = svg_dynamic_cast(node);
 
         // Skip any non-svg nodes or any disallowed element.
-        if (!element)
+        if (!element || isDisallowedElement(element))
             continue;
 
         // Create SVGElementInstance object, for both container/non-container nodes.
@@ -433,48 +444,6 @@ void SVGUseElement::handleDeepUseReferencing(SVGElement* use, SVGElementInstance
     buildInstanceTree(target, newInstance, foundProblem);
 }
 
-PassRefPtr<SVGSVGElement> SVGUseElement::buildShadowTreeForSymbolTag(SVGElement* target, SVGElementInstance* targetInstance)
-{
-    ExceptionCode ec = 0;
-
-    String widthString = String::number(width().value());
-    String heightString = String::number(height().value());
- 
-    // Spec: The referenced 'symbol' and its contents are deep-cloned into the generated tree,
-    // with the exception that the 'symbol' is replaced by an 'svg'. This generated 'svg' will
-    // always have explicit values for attributes width and height. If attributes width and/or
-    // height are provided on the 'use' element, then these attributes will be transferred to
-    // the generated 'svg'. If attributes width and/or height are not specified, the generated
-    // 'svg' element will use values of 100% for these attributes.
-    RefPtr<SVGSVGElement> svgElement = new SVGSVGElement(SVGNames::svgTag, document());
-
-    // Transfer all attributes from <symbol> to the new <svg> element
-    *svgElement->attributes() = *target->attributes();
-
-    // Explicitly re-set width/height values
-    svgElement->setAttribute(SVGNames::widthAttr, hasAttribute(SVGNames::widthAttr) ? widthString : "100%");
-    svgElement->setAttribute(SVGNames::heightAttr, hasAttribute(SVGNames::heightAttr) ? heightString : "100%");
-
-    // Only clone symbol children, and add them to the new <svg> element    
-    if (targetInstance) {
-        // Called from buildShadowTree()
-        for (SVGElementInstance* instance = targetInstance->firstChild(); instance; instance = instance->nextSibling()) {
-            RefPtr<Node> newChild = instance->correspondingElement()->cloneNode(true);
-            svgElement->appendChild(newChild.release(), ec);
-            ASSERT(ec == 0);
-        }
-    } else {
-        // Called from expandUseElementsInShadowTree()
-        for (Node* child = target->firstChild(); child; child = child->nextSibling()) {
-            RefPtr<Node> newChild = child->cloneNode(true);
-            svgElement->appendChild(newChild.release(), ec);
-            ASSERT(ec == 0);
-        }
-    }
-
-    return svgElement;
-}
-
 void SVGUseElement::alterShadowTreeForSVGTag(SVGElement* target)
 {
     String widthString = String::number(width().value());
@@ -487,21 +456,44 @@ void SVGUseElement::alterShadowTreeForSVGTag(SVGElement* target)
         target->setAttribute(SVGNames::heightAttr, heightString);
 }
 
-void SVGUseElement::buildShadowTree(SVGElement* target, SVGElementInstance* targetInstance)
+void SVGUseElement::removeDisallowedElementsFromSubtree(Node* element)
 {
     ExceptionCode ec = 0;
 
-    RefPtr<Node> newChild;
+    for (RefPtr<Node> child = element->firstChild(); child; child = child->nextSibling()) {
+        if (isDisallowedElement(child.get())) {
+            ASSERT(child->parent());
+            child->parent()->removeChild(child.get(), ec);
+            ASSERT(ec == 0);
 
-    // Handle use referencing <symbol> special case
-    if (target->hasTagName(SVGNames::symbolTag))
-        newChild = buildShadowTreeForSymbolTag(target, targetInstance);
-    else
-        newChild = targetInstance->correspondingElement()->cloneNode(true);
+            continue;
+        }
+
+        if (child->hasChildNodes())
+            removeDisallowedElementsFromSubtree(child.get());
+    }
+}
+
+void SVGUseElement::buildShadowTree(SVGElement* target, SVGElementInstance* targetInstance)
+{
+    // For instance <use> on <foreignObject> (direct case).
+    if (isDisallowedElement(target))
+        return;
+
+    RefPtr<Node> newChild = targetInstance->correspondingElement()->cloneNode(true);
+
+    // We don't walk the target tree element-by-element, and clone each element,
+    // but instead use cloneNode(deep=true). This is an optimization for the common
+    // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
+    // Though if there are disallowed elements in the subtree, we have to remove them.
+    // For instance: <use> on <g> containing <foreignObject> (indirect case).
+    if (subtreeContainsDisallowedElement(newChild.get()))
+        removeDisallowedElementsFromSubtree(newChild.get());
 
     SVGElement* newChildPtr = svg_dynamic_cast(newChild.get());
     ASSERT(newChildPtr);
 
+    ExceptionCode ec = 0;
     m_shadowTreeRootElement->appendChild(newChild.release(), ec);
     ASSERT(ec == 0);
 
@@ -550,18 +542,32 @@ void SVGUseElement::expandUseElementsInShadowTree(Node* element)
                 }
             }
 
-            RefPtr<Node> newChild;
+            ExceptionCode ec = 0;
+ 
+            // For instance <use> on <foreignObject> (direct case).
+            if (isDisallowedElement(target)) {
+                // We still have to setup the <use> replacment (<g>). Otherwhise
+                // associateInstancesWithShadowTreeElements() makes wrong assumptions.
+                // Replace <use> with referenced content.
+                ASSERT(use->parentNode()); 
+                use->parentNode()->replaceChild(cloneParent.release(), use, ec);
+                ASSERT(ec == 0);
+                return;
+            }
+
+            RefPtr<Node> newChild = target->cloneNode(true);
 
-            // Handle use referencing <symbol> special case
-            if (target->hasTagName(SVGNames::symbolTag))
-                newChild = buildShadowTreeForSymbolTag(target, 0);
-            else
-                newChild = target->cloneNode(true);
+            // We don't walk the target tree element-by-element, and clone each element,
+            // but instead use cloneNode(deep=true). This is an optimization for the common
+            // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
+            // Though if there are disallowed elements in the subtree, we have to remove them.
+            // For instance: <use> on <g> containing <foreignObject> (indirect case).
+            if (subtreeContainsDisallowedElement(newChild.get()))
+                removeDisallowedElementsFromSubtree(newChild.get());
 
             SVGElement* newChildPtr = svg_dynamic_cast(newChild.get());
             ASSERT(newChildPtr);
 
-            ExceptionCode ec = 0;
             cloneParent->appendChild(newChild.release(), ec);
             ASSERT(ec == 0);
 
@@ -583,6 +589,59 @@ void SVGUseElement::expandUseElementsInShadowTree(Node* element)
     for (RefPtr<Node> child = element->firstChild(); child; child = child->nextSibling())
         expandUseElementsInShadowTree(child.get());
 }
+
+void SVGUseElement::expandSymbolElementsInShadowTree(Node* element)
+{
+    if (element->hasTagName(SVGNames::symbolTag)) {
+        // Spec: The referenced 'symbol' and its contents are deep-cloned into the generated tree,
+        // with the exception that the 'symbol' is replaced by an 'svg'. This generated 'svg' will
+        // always have explicit values for attributes width and height. If attributes width and/or
+        // height are provided on the 'use' element, then these attributes will be transferred to
+        // the generated 'svg'. If attributes width and/or height are not specified, the generated
+        // 'svg' element will use values of 100% for these attributes.
+        RefPtr<SVGSVGElement> svgElement = new SVGSVGElement(SVGNames::svgTag, document());
+
+        // Transfer all attributes from <symbol> to the new <svg> element
+        *svgElement->attributes() = *element->attributes();
+
+        // Explicitly re-set width/height values
+        String widthString = String::number(width().value());
+        String heightString = String::number(height().value()); 
+
+        svgElement->setAttribute(SVGNames::widthAttr, hasAttribute(SVGNames::widthAttr) ? widthString : "100%");
+        svgElement->setAttribute(SVGNames::heightAttr, hasAttribute(SVGNames::heightAttr) ? heightString : "100%");
+
+        ExceptionCode ec = 0;
+
+        // Only clone symbol children, and add them to the new <svg> element    
+        for (Node* child = element->firstChild(); child; child = child->nextSibling()) {
+            RefPtr<Node> newChild = child->cloneNode(true);
+            svgElement->appendChild(newChild.release(), ec);
+            ASSERT(ec == 0);
+        }
+    
+        // We don't walk the target tree element-by-element, and clone each element,
+        // but instead use cloneNode(deep=true). This is an optimization for the common
+        // case where <use> doesn't contain disallowed elements (ie. <foreignObject>).
+        // Though if there are disallowed elements in the subtree, we have to remove them.
+        // For instance: <use> on <g> containing <foreignObject> (indirect case).
+        if (subtreeContainsDisallowedElement(svgElement.get()))
+            removeDisallowedElementsFromSubtree(svgElement.get());
+
+        // Replace <symbol> with <svg>.
+        ASSERT(element->parentNode()); 
+        element->parentNode()->replaceChild(svgElement.release(), element, ec);
+        ASSERT(ec == 0);
+
+        // Immediately stop here, and restart expanding.
+        expandSymbolElementsInShadowTree(m_shadowTreeRootElement.get());
+        return;
+    }
+
+    for (RefPtr<Node> child = element->firstChild(); child; child = child->nextSibling())
+        expandSymbolElementsInShadowTree(child.get());
+}
+
 #endif
     
 void SVGUseElement::attachShadowTree()

diff --git a/WebCore/ksvg2/svg/SVGUseElement.h b/WebCore/ksvg2/svg/SVGUseElement.h
index af25eb8660c91f218c3bd63f2346b676216766b7..494cb7f09e4e1f26fe117eca6c5cbba00743c058 100644 (file)
--- a/WebCore/ksvg2/svg/SVGUseElement.h
+++ b/WebCore/ksvg2/svg/SVGUseElement.h
@@ -90,10 +90,12 @@ namespace WebCore
         // Shadow tree handling
         PassRefPtr<SVGSVGElement> buildShadowTreeForSymbolTag(SVGElement* target, SVGElementInstance* targetInstance);
         void alterShadowTreeForSVGTag(SVGElement* target);
+        void removeDisallowedElementsFromSubtree(Node* element);
 
         void buildShadowTree(SVGElement* target, SVGElementInstance* targetInstance);
 #if ENABLE(SVG) && ENABLE(SVG_EXPERIMENTAL_FEATURES)
         void expandUseElementsInShadowTree(Node* element);
+        void expandSymbolElementsInShadowTree(Node* element);
 #endif
         void attachShadowTree();