<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[179069] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/179069">179069</a></dd>
<dt>Author</dt> <dd>darin@apple.com</dd>
<dt>Date</dt> <dd>2015-01-24 11:55:58 -0800 (Sat, 24 Jan 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Move height/width implementation for use element from RenderSVGViewportContainer to SVGUseElement
https://bugs.webkit.org/show_bug.cgi?id=140826
Reviewed by Anders Carlsson.
Source/WebCore:
Tests: svg/animations/use-animate-width-and-height.html
svg/custom/use-attribute-invalidations.html
svg/custom/use-dynamic-attribute-setting.html
This is an adaptation of work Rob Buis did in Blink:
http://src.chromium.org/viewvc/blink?view=revision&revision=173258
The goal here is to reduce use of SVGElementInstance since we are going to
remove it. The tests Rob added to Blink (which I believe I improved a bit here)
meant we had to fix quite a few bugs in the implementation of the width/height
logic rather than just moving it. Even so, this could use even more test coverage
since there is separate logic for <symbol> and <svg>, three different code paths
(animation/attribute setting, initial creation, and one other), and also
distinct issues for attributes not set at all, attributes set to values that
can't be parsed, and attributes set with different units.
* rendering/svg/RenderSVGViewportContainer.cpp:
(WebCore::RenderSVGViewportContainer::calcViewport): Removed the old logic.
* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::SVGSVGElement): Use ASCIILiteral to more efficiently
create strings from ASCII literals here.
(WebCore::SVGSVGElement::parseAttribute): Default to 100%, not 0, when the
width or height property are either not set or not successfully parsed. Without
this change, one of the SVG tests starts failing.
* svg/SVGUseElement.cpp:
(WebCore::updateWidthAndHeight): Added. The tricky part here is that we have
to copy width and height attributes only if they were successfully parsed, and
also we need to copy the current animating values, not the original attribute
strings. Kind of messy, but I wanted to adapt Rob's solution for the time being,
rather than inventing something new.
(WebCore::SVGUseElement::svgAttributeChanged): Call updateWidthAndHeight.
This is used both when actual attribute changes occur and also when animation
changes the current value.
(WebCore::SVGUseElement::buildShadowAndInstanceTree): Call updateWidthAndHeight.
This is used when the shadow elements are first created.
(WebCore::SVGUseElement::expandUseElementsInShadowTree): Call updateWidthAndHeight.
This was in Rob's patch, but I am not sure we have sufficient test coverage.
LayoutTests:
* platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.png: Old test
result was expecting failure. New one expects success.
* platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.txt: Ditto.
* svg/animations/use-animate-width-and-height-expected.txt: Added.
* svg/animations/use-animate-width-and-height.html: Added.
* svg/custom/use-attribute-invalidations-expected.html: Added.
* svg/custom/use-attribute-invalidations.html: Added.
* svg/custom/use-dynamic-attribute-setting-expected.html: Added.
* svg/custom/use-dynamic-attribute-setting.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformmacsvgcustomrelativesizedshadowtreecontentwithsymbolexpectedpng">trunk/LayoutTests/platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.png</a></li>
<li><a href="#trunkLayoutTestsplatformmacsvgcustomrelativesizedshadowtreecontentwithsymbolexpectedtxt">trunk/LayoutTests/platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.txt</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingsvgRenderSVGViewportContainercpp">trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp</a></li>
<li><a href="#trunkSourceWebCoresvgSVGSVGElementcpp">trunk/Source/WebCore/svg/SVGSVGElement.cpp</a></li>
<li><a href="#trunkSourceWebCoresvgSVGUseElementcpp">trunk/Source/WebCore/svg/SVGUseElement.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestssvganimationsuseanimatewidthandheightexpectedtxt">trunk/LayoutTests/svg/animations/use-animate-width-and-height-expected.txt</a></li>
<li><a href="#trunkLayoutTestssvganimationsuseanimatewidthandheighthtml">trunk/LayoutTests/svg/animations/use-animate-width-and-height.html</a></li>
<li><a href="#trunkLayoutTestssvgcustomuseattributeinvalidationsexpectedhtml">trunk/LayoutTests/svg/custom/use-attribute-invalidations-expected.html</a></li>
<li><a href="#trunkLayoutTestssvgcustomuseattributeinvalidationshtml">trunk/LayoutTests/svg/custom/use-attribute-invalidations.html</a></li>
<li><a href="#trunkLayoutTestssvgcustomusedynamicattributesettingexpectedhtml">trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting-expected.html</a></li>
<li><a href="#trunkLayoutTestssvgcustomusedynamicattributesettinghtml">trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (179068 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-01-24 19:14:44 UTC (rev 179068)
+++ trunk/LayoutTests/ChangeLog 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2015-01-24 Darin Adler <darin@apple.com>
+
+ Move height/width implementation for use element from RenderSVGViewportContainer to SVGUseElement
+ https://bugs.webkit.org/show_bug.cgi?id=140826
+
+ Reviewed by Anders Carlsson.
+
+ * platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.png: Old test
+ result was expecting failure. New one expects success.
+ * platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.txt: Ditto.
+ * svg/animations/use-animate-width-and-height-expected.txt: Added.
+ * svg/animations/use-animate-width-and-height.html: Added.
+ * svg/custom/use-attribute-invalidations-expected.html: Added.
+ * svg/custom/use-attribute-invalidations.html: Added.
+ * svg/custom/use-dynamic-attribute-setting-expected.html: Added.
+ * svg/custom/use-dynamic-attribute-setting.html: Added.
+
</ins><span class="cx"> 2015-01-23 Brent Fulgham <bfulgham@apple.com>
</span><span class="cx">
</span><span class="cx"> [Win] Unreviewed gardening. Add Windows baseline for
</span></span></pre></div>
<a id="trunkLayoutTestsplatformmacsvgcustomrelativesizedshadowtreecontentwithsymbolexpectedpng"></a>
<div class="binary"><h4>Modified: trunk/LayoutTests/platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.png</h4>
<pre class="diff"><span>
<span class="cx">(Binary files differ)
</span></span></pre></div>
<a id="trunkLayoutTestsplatformmacsvgcustomrelativesizedshadowtreecontentwithsymbolexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.txt (179068 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.txt 2015-01-24 19:14:44 UTC (rev 179068)
+++ trunk/LayoutTests/platform/mac/svg/custom/relative-sized-shadow-tree-content-with-symbol-expected.txt 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -8,13 +8,13 @@
</span><span class="cx"> text run at (0,0) width 744: "The svg area contained in the div element (red box), should fill out the whole area (two green rectangles, first: (0,0)-"
</span><span class="cx"> text run at (0,18) width 686: "(50%,50%), second: (50%,50%)-(100%,100%)), especially after resizing the content box to a different size"
</span><span class="cx"> RenderBlock {div} at (0,52) size 402x402 [border: (1px solid #FF0000)]
</span><del>- RenderSVGRoot {svg} at (9,69) size 400x400
</del><ins>+ RenderSVGRoot {svg} at (9,69) size 200x200
</ins><span class="cx"> RenderSVGHiddenContainer {defs} at (0,0) size 0x0
</span><span class="cx"> RenderSVGHiddenContainer {symbol} at (0,0) size 0x0
</span><span class="cx"> RenderSVGRect {rect} at (9,69) size 0x0 [fill={[type=SOLID] [color=#008000]}] [x=0.00] [y=0.00] [width=0.00] [height=0.00]
</span><span class="cx"> RenderSVGRect {rect} at (9,69) size 100x100 [fill={[type=SOLID] [color=#008000]}] [x=0.00] [y=0.00] [width=100.00] [height=100.00]
</span><del>- RenderSVGContainer {use} at (9,69) size 400x400
- RenderSVGViewportContainer {svg} at (9,69) size 400x400
- RenderSVGRect {rect} at (209,269) size 200x200 [fill={[type=SOLID] [color=#008000]}] [x=100.00] [y=100.00] [width=100.00] [height=100.00]
- RenderSVGRect {rect} at (9,69) size 200x200 [fill={[type=SOLID] [color=#008000]}] [x=0.00] [y=0.00] [width=100.00] [height=100.00]
</del><ins>+ RenderSVGContainer {use} at (9,69) size 200x200
+ RenderSVGViewportContainer {svg} at (9,69) size 200x200
+ RenderSVGRect {rect} at (109,169) size 100x100 [fill={[type=SOLID] [color=#008000]}] [x=100.00] [y=100.00] [width=100.00] [height=100.00]
+ RenderSVGRect {rect} at (9,69) size 100x100 [fill={[type=SOLID] [color=#008000]}] [x=0.00] [y=0.00] [width=100.00] [height=100.00]
</ins><span class="cx"> RenderText {#text} at (0,0) size 0x0
</span></span></pre></div>
<a id="trunkLayoutTestssvganimationsuseanimatewidthandheightexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/animations/use-animate-width-and-height-expected.txt (0 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/animations/use-animate-width-and-height-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/animations/use-animate-width-and-height-expected.txt 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -0,0 +1,49 @@
</span><ins>+Test animation of use element width/height
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS use.width.animVal.value is 100
+PASS use.width.baseVal.value is 100
+PASS use.height.animVal.value is 100
+PASS use.height.baseVal.value is 100
+PASS use.getAttribute('width') is '100'
+PASS use.getAttribute('height') is '100'
+PASS shadowRoot.firstChild.width.animVal.value is 100
+PASS shadowRoot.firstChild.height.animVal.value is 100
+PASS use.width.animVal.value is 105
+PASS use.width.baseVal.value is 100
+PASS use.height.animVal.value is 105
+PASS use.height.baseVal.value is 100
+FAIL use.getAttribute('width') should be 105. Was 100.
+FAIL use.getAttribute('height') should be 105. Was 100.
+PASS shadowRoot.firstChild.width.animVal.value is 105
+PASS shadowRoot.firstChild.height.animVal.value is 105
+PASS use.width.animVal.value is 115
+PASS use.width.baseVal.value is 100
+PASS use.height.animVal.value is 115
+PASS use.height.baseVal.value is 100
+FAIL use.getAttribute('width') should be 115. Was 100.
+FAIL use.getAttribute('height') should be 115. Was 100.
+PASS shadowRoot.firstChild.width.animVal.value is 115
+PASS shadowRoot.firstChild.height.animVal.value is 115
+PASS use.width.animVal.value is 125
+PASS use.width.baseVal.value is 100
+PASS use.height.animVal.value is 125
+PASS use.height.baseVal.value is 100
+FAIL use.getAttribute('width') should be 125. Was 100.
+FAIL use.getAttribute('height') should be 125. Was 100.
+PASS shadowRoot.firstChild.width.animVal.value is 125
+PASS shadowRoot.firstChild.height.animVal.value is 125
+PASS use.width.animVal.value is 135
+PASS use.width.baseVal.value is 100
+PASS use.height.animVal.value is 135
+PASS use.height.baseVal.value is 100
+FAIL use.getAttribute('width') should be 135. Was 100.
+FAIL use.getAttribute('height') should be 135. Was 100.
+PASS shadowRoot.firstChild.width.animVal.value is 135
+PASS shadowRoot.firstChild.height.animVal.value is 135
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins><span class="cx">Property changes on: trunk/LayoutTests/svg/animations/use-animate-width-and-height-expected.txt
</span><span class="cx">___________________________________________________________________
</span></span></pre></div>
<a id="svneolstyle"></a>
<div class="addfile"><h4>Added: svn:eol-style</h4></div>
<a id="trunkLayoutTestssvganimationsuseanimatewidthandheighthtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/animations/use-animate-width-and-height.html (0 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/animations/use-animate-width-and-height.html (rev 0)
+++ trunk/LayoutTests/svg/animations/use-animate-width-and-height.html 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -0,0 +1,143 @@
</span><ins>+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+<script src="../dynamic-updates/resources/SVGTestCase.js"></script>
+<script src="resources/SVGAnimationTestCase.js"></script>
+</head>
+<body onload="runSMILTest()">
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+description("Test animation of use element width/height");
+createSVGTestCase();
+
+// Set up test document.
+
+var symbol = createSVGElement("symbol");
+symbol.setAttribute("id", "symbol");
+
+var rect = createSVGElement("rect");
+rect.setAttribute("id", "rect");
+rect.setAttribute("x", "10");
+rect.setAttribute("y", "10");
+rect.setAttribute("width", "100%");
+rect.setAttribute("height", "100%");
+rect.setAttribute("fill", "green");
+rect.setAttribute("onclick", "executeTest()");
+symbol.appendChild(rect);
+rootSVGElement.appendChild(symbol);
+
+var use = createSVGElement("use");
+use.setAttribute("id", "use");
+use.setAttributeNS('http://www.w3.org/1999/xlink', 'xlink:href', '#symbol');
+use.setAttribute("x", "0");
+use.setAttribute("y", "0");
+use.setAttribute("width", "100");
+use.setAttribute("height", "100");
+rootSVGElement.appendChild(use);
+
+var animate = createSVGElement("animate");
+animate.setAttribute("id", "animate");
+animate.setAttribute("attributeName", "width");
+animate.setAttribute("attributeType", "XML");
+animate.setAttribute("begin", "1s");
+animate.setAttribute("dur", "10s");
+animate.setAttribute("from", "100");
+animate.setAttribute("to", "200");
+use.appendChild(animate);
+
+var animate2 = createSVGElement("animate");
+animate2.setAttribute("id", "animate2");
+animate2.setAttribute("attributeName", "height");
+animate2.setAttribute("attributeType", "XML");
+animate2.setAttribute("begin", "1s");
+animate2.setAttribute("dur", "10s");
+animate2.setAttribute("from", "100");
+animate2.setAttribute("to", "200");
+use.appendChild(animate2);
+
+var shadowRoot = internals.shadowRoot(use);
+
+function sample1()
+{
+ // Check initial/end conditions
+ shouldBe("use.width.animVal.value", "100");
+ shouldBe("use.width.baseVal.value", "100");
+ shouldBe("use.height.animVal.value", "100");
+ shouldBe("use.height.baseVal.value", "100");
+ shouldBe("use.getAttribute('width')", "'100'");
+ shouldBe("use.getAttribute('height')", "'100'");
+ shouldBe("shadowRoot.firstChild.width.animVal.value", "100");
+ shouldBe("shadowRoot.firstChild.height.animVal.value", "100");
+}
+
+function sample2()
+{
+ shouldBe("use.width.animVal.value", "105");
+ shouldBe("use.width.baseVal.value", "100");
+ shouldBe("use.height.animVal.value", "105");
+ shouldBe("use.height.baseVal.value", "100");
+ shouldBe("use.getAttribute('width')", "'105'");
+ shouldBe("use.getAttribute('height')", "'105'");
+ shouldBe("shadowRoot.firstChild.width.animVal.value", "105");
+ shouldBe("shadowRoot.firstChild.height.animVal.value", "105");
+}
+
+function sample3()
+{
+ shouldBe("use.width.animVal.value", "115");
+ shouldBe("use.width.baseVal.value", "100");
+ shouldBe("use.height.animVal.value", "115");
+ shouldBe("use.height.baseVal.value", "100");
+ shouldBe("use.getAttribute('width')", "'115'");
+ shouldBe("use.getAttribute('height')", "'115'");
+ shouldBe("shadowRoot.firstChild.width.animVal.value", "115");
+ shouldBe("shadowRoot.firstChild.height.animVal.value", "115");
+}
+
+function sample4() {
+ shouldBe("use.width.animVal.value", "125");
+ shouldBe("use.width.baseVal.value", "100");
+ shouldBe("use.height.animVal.value", "125");
+ shouldBe("use.height.baseVal.value", "100");
+ shouldBe("use.getAttribute('width')", "'125'");
+ shouldBe("use.getAttribute('height')", "'125'");
+ shouldBe("shadowRoot.firstChild.width.animVal.value", "125");
+ shouldBe("shadowRoot.firstChild.height.animVal.value", "125");
+}
+
+function sample5()
+{
+ shouldBe("use.width.animVal.value", "135");
+ shouldBe("use.width.baseVal.value", "100");
+ shouldBe("use.height.animVal.value", "135");
+ shouldBe("use.height.baseVal.value", "100");
+ shouldBe("use.getAttribute('width')", "'135'");
+ shouldBe("use.getAttribute('height')", "'135'");
+ shouldBe("shadowRoot.firstChild.width.animVal.value", "135");
+ shouldBe("shadowRoot.firstChild.height.animVal.value", "135");
+}
+
+function executeTest()
+{
+ const expectedValues = [
+ // [animationId, time, sampleCallback]
+ ["animate2", 0.0, sample1],
+ ["animate2", 0.5, sample2],
+ ["animate2", 1.5, sample3],
+ ["animate2", 2.5, sample4],
+ ["animate2", 3.5, sample5]
+ ];
+
+ runAnimationTest(expectedValues);
+}
+
+window.clickX = 50;
+window.clickY = 50;
+
+var successfullyParsed = true;
+</script>
+</body>
+</html>
</ins><span class="cx">Property changes on: trunk/LayoutTests/svg/animations/use-animate-width-and-height.html
</span><span class="cx">___________________________________________________________________
</span></span></pre></div>
<a id="svnmimetype"></a>
<div class="addfile"><h4>Added: svn:mime-type</h4></div>
<a id="svneolstyle"></a>
<div class="addfile"><h4>Added: svn:eol-style</h4></div>
<a id="trunkLayoutTestssvgcustomuseattributeinvalidationsexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/use-attribute-invalidations-expected.html (0 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/custom/use-attribute-invalidations-expected.html (rev 0)
+++ trunk/LayoutTests/svg/custom/use-attribute-invalidations-expected.html 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -0,0 +1,17 @@
</span><ins>+<!DOCTYPE HTML>
+<html>
+<body>
+<p>This test passes if there are three 100x100px green squares in a diagonal line below:</p>
+<svg width="300" height="300">
+ <svg width="100" height="100">
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+ <svg x="100" y="100" width="100" height="100">
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+ <svg x="200" y="200" width="100" height="100">
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+</svg>
+</body>
+</html>
</ins><span class="cx">Property changes on: trunk/LayoutTests/svg/custom/use-attribute-invalidations-expected.html
</span><span class="cx">___________________________________________________________________
</span></span></pre></div>
<a id="svnmimetype"></a>
<div class="addfile"><h4>Added: svn:mime-type</h4></div>
<a id="svneolstyle"></a>
<div class="addfile"><h4>Added: svn:eol-style</h4></div>
<a id="trunkLayoutTestssvgcustomuseattributeinvalidationshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/use-attribute-invalidations.html (0 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/custom/use-attribute-invalidations.html (rev 0)
+++ trunk/LayoutTests/svg/custom/use-attribute-invalidations.html 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -0,0 +1,27 @@
</span><ins>+<!DOCTYPE HTML>
+<html>
+<body>
+<p>This test passes if there are three 100x100px green squares in a diagonal line below:</p>
+<svg width="300" height="300">
+ <defs>
+ <symbol id="symbol" width="100">
+ <rect width="100%" height="100%" fill="green"/>
+ </symbol>
+ </defs>
+ <svg x="0" y="0" width="100" height="100">
+ <use id="use1" xlink:href="#symbol"/>
+ </svg>
+ <use id="use2" xlink:href="#symbol" x="100" y="100" width="10" height="100"/>
+ <use id="use3" xlink:href="#symbol" x="200" y="200" width="100" height="10"/>
+</svg>
+<script>
+ // Force layout since the intention is to test behavior of changes *after* layout has occurred.
+ // FIXME: Find a clean reliable way to do this; Blink tests were using requestAnimationFrame,
+ // which seems like it would be racy and wrong, but perhaps they have some rationale for that.
+ document.body.offsetTop;
+
+ document.getElementById('use2').removeAttribute('width');
+ document.getElementById('use3').setAttribute('height', '100');
+</script>
+</body>
+</html>
</ins><span class="cx">Property changes on: trunk/LayoutTests/svg/custom/use-attribute-invalidations.html
</span><span class="cx">___________________________________________________________________
</span></span></pre></div>
<a id="svnmimetype"></a>
<div class="addfile"><h4>Added: svn:mime-type</h4></div>
<a id="svneolstyle"></a>
<div class="addfile"><h4>Added: svn:eol-style</h4></div>
<a id="trunkLayoutTestssvgcustomusedynamicattributesettingexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting-expected.html (0 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting-expected.html (rev 0)
+++ trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting-expected.html 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -0,0 +1,14 @@
</span><ins>+<!DOCTYPE HTML>
+<html>
+<body>
+<p>This test passes if there are two 100x100px green squares in a diagonal line below:</p>
+<svg width="300" height="300">
+ <svg width="100" height="100">
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+ <svg x="100" y="100" width="100" height="100">
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+</svg>
+</body>
+</html>
</ins><span class="cx">Property changes on: trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting-expected.html
</span><span class="cx">___________________________________________________________________
</span></span></pre></div>
<a id="svnmimetype"></a>
<div class="addfile"><h4>Added: svn:mime-type</h4></div>
<a id="svneolstyle"></a>
<div class="addfile"><h4>Added: svn:eol-style</h4></div>
<a id="trunkLayoutTestssvgcustomusedynamicattributesettinghtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting.html (0 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting.html (rev 0)
+++ trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting.html 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -0,0 +1,24 @@
</span><ins>+<!DOCTYPE HTML>
+<html>
+<body>
+<p>This test passes if there are two 100x100px green squares in a diagonal line below:</p>
+<svg width="300" height="300">
+ <defs>
+ <svg id="svg" width="10" height="10">
+ <rect width="100%" height="100%" fill="green"/>
+ </svg>
+ </defs>
+ <use id="use1" xlink:href="#svg" x="0" y="0" width="100" height="100"/>
+ <use id="use2" xlink:href="#svg" x="100" y="100" width="10" height="10"/>
+</svg>
+<script>
+ // Force layout since the intention is to test behavior of changes *after* layout has occurred.
+ // FIXME: Find a clean reliable way to do this; Blink tests were using requestAnimationFrame,
+ // which seems like it would be racy and wrong, but perhaps they have some rationale for that.
+ document.body.offsetTop;
+
+ document.getElementById('use2').setAttribute('width', '100');
+ document.getElementById('use2').setAttribute('height', '100');
+</script>
+</body>
+</html>
</ins><span class="cx">Property changes on: trunk/LayoutTests/svg/custom/use-dynamic-attribute-setting.html
</span><span class="cx">___________________________________________________________________
</span></span></pre></div>
<a id="svnmimetype"></a>
<div class="addfile"><h4>Added: svn:mime-type</h4></div>
<a id="svneolstyle"></a>
<div class="addfile"><h4>Added: svn:eol-style</h4></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (179068 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-01-24 19:14:44 UTC (rev 179068)
+++ trunk/Source/WebCore/ChangeLog 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -1,3 +1,51 @@
</span><ins>+2015-01-24 Darin Adler <darin@apple.com>
+
+ Move height/width implementation for use element from RenderSVGViewportContainer to SVGUseElement
+ https://bugs.webkit.org/show_bug.cgi?id=140826
+
+ Reviewed by Anders Carlsson.
+
+ Tests: svg/animations/use-animate-width-and-height.html
+ svg/custom/use-attribute-invalidations.html
+ svg/custom/use-dynamic-attribute-setting.html
+
+ This is an adaptation of work Rob Buis did in Blink:
+
+ http://src.chromium.org/viewvc/blink?view=revision&revision=173258
+
+ The goal here is to reduce use of SVGElementInstance since we are going to
+ remove it. The tests Rob added to Blink (which I believe I improved a bit here)
+ meant we had to fix quite a few bugs in the implementation of the width/height
+ logic rather than just moving it. Even so, this could use even more test coverage
+ since there is separate logic for <symbol> and <svg>, three different code paths
+ (animation/attribute setting, initial creation, and one other), and also
+ distinct issues for attributes not set at all, attributes set to values that
+ can't be parsed, and attributes set with different units.
+
+ * rendering/svg/RenderSVGViewportContainer.cpp:
+ (WebCore::RenderSVGViewportContainer::calcViewport): Removed the old logic.
+
+ * svg/SVGSVGElement.cpp:
+ (WebCore::SVGSVGElement::SVGSVGElement): Use ASCIILiteral to more efficiently
+ create strings from ASCII literals here.
+ (WebCore::SVGSVGElement::parseAttribute): Default to 100%, not 0, when the
+ width or height property are either not set or not successfully parsed. Without
+ this change, one of the SVG tests starts failing.
+
+ * svg/SVGUseElement.cpp:
+ (WebCore::updateWidthAndHeight): Added. The tricky part here is that we have
+ to copy width and height attributes only if they were successfully parsed, and
+ also we need to copy the current animating values, not the original attribute
+ strings. Kind of messy, but I wanted to adapt Rob's solution for the time being,
+ rather than inventing something new.
+ (WebCore::SVGUseElement::svgAttributeChanged): Call updateWidthAndHeight.
+ This is used both when actual attribute changes occur and also when animation
+ changes the current value.
+ (WebCore::SVGUseElement::buildShadowAndInstanceTree): Call updateWidthAndHeight.
+ This is used when the shadow elements are first created.
+ (WebCore::SVGUseElement::expandUseElementsInShadowTree): Call updateWidthAndHeight.
+ This was in Rob's patch, but I am not sure we have sufficient test coverage.
+
</ins><span class="cx"> 2015-01-24 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> Provide implementation for WTF::DefaultHash<bool>
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingsvgRenderSVGViewportContainercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp (179068 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp 2015-01-24 19:14:44 UTC (rev 179068)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGViewportContainer.cpp 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -58,62 +58,17 @@
</span><span class="cx">
</span><span class="cx"> void RenderSVGViewportContainer::calcViewport()
</span><span class="cx"> {
</span><del>- SVGSVGElement& svg = svgSVGElement();
- FloatRect oldViewport = m_viewport;
</del><ins>+ SVGSVGElement& element = svgSVGElement();
+ SVGLengthContext lengthContext(&element);
+ FloatRect newViewport(element.x().value(lengthContext), element.y().value(lengthContext), element.width().value(lengthContext), element.height().value(lengthContext));
</ins><span class="cx">
</span><del>- SVGLengthContext lengthContext(&svg);
- m_viewport = FloatRect(svg.x().value(lengthContext), svg.y().value(lengthContext), svg.width().value(lengthContext), svg.height().value(lengthContext));
</del><ins>+ if (m_viewport == newViewport)
+ return;
</ins><span class="cx">
</span><del>- SVGElement* correspondingElement = svg.correspondingElement();
- if (correspondingElement && svg.isInShadowTree()) {
- const HashSet<SVGElementInstance*>& instances = correspondingElement->instancesForElement();
- ASSERT(!instances.isEmpty());
</del><ins>+ m_viewport = newViewport;
</ins><span class="cx">
</span><del>- SVGUseElement* useElement = 0;
- const HashSet<SVGElementInstance*>::const_iterator end = instances.end();
- for (HashSet<SVGElementInstance*>::const_iterator it = instances.begin(); it != end; ++it) {
- const SVGElementInstance* instance = (*it);
- ASSERT(instance->correspondingElement()->hasTagName(SVGNames::svgTag) || instance->correspondingElement()->hasTagName(SVGNames::symbolTag));
- if (instance->shadowTreeElement() == &svg) {
- ASSERT(correspondingElement == instance->correspondingElement());
- useElement = instance->directUseElement();
- if (!useElement)
- useElement = instance->correspondingUseElement();
- break;
- }
- }
-
- ASSERT(useElement);
- bool isSymbolElement = correspondingElement->hasTagName(SVGNames::symbolTag);
-
- // Spec (<use> on <symbol>): 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.
-
- // Spec (<use> on <svg>): If attributes width and/or height are provided on the 'use' element, then these
- // values will override the corresponding attributes on the 'svg' in the generated tree.
-
- SVGLengthContext lengthContext(&svg);
- if (useElement->hasAttribute(SVGNames::widthAttr))
- m_viewport.setWidth(useElement->width().value(lengthContext));
- else if (isSymbolElement && svg.hasAttribute(SVGNames::widthAttr)) {
- SVGLength containerWidth(LengthModeWidth, "100%");
- m_viewport.setWidth(containerWidth.value(lengthContext));
- }
-
- if (useElement->hasAttribute(SVGNames::heightAttr))
- m_viewport.setHeight(useElement->height().value(lengthContext));
- else if (isSymbolElement && svg.hasAttribute(SVGNames::heightAttr)) {
- SVGLength containerHeight(LengthModeHeight, "100%");
- m_viewport.setHeight(containerHeight.value(lengthContext));
- }
- }
-
- if (oldViewport != m_viewport) {
- setNeedsBoundariesUpdate();
- setNeedsTransformUpdate();
- }
</del><ins>+ setNeedsBoundariesUpdate();
+ setNeedsTransformUpdate();
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool RenderSVGViewportContainer::calculateLocalTransform()
</span></span></pre></div>
<a id="trunkSourceWebCoresvgSVGSVGElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (179068 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/svg/SVGSVGElement.cpp 2015-01-24 19:14:44 UTC (rev 179068)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -84,8 +84,8 @@
</span><span class="cx"> : SVGGraphicsElement(tagName, document)
</span><span class="cx"> , m_x(LengthModeWidth)
</span><span class="cx"> , m_y(LengthModeHeight)
</span><del>- , m_width(LengthModeWidth, "100%")
- , m_height(LengthModeHeight, "100%")
</del><ins>+ , m_width(LengthModeWidth, ASCIILiteral("100%"))
+ , m_height(LengthModeHeight, ASCIILiteral("100%"))
</ins><span class="cx"> , m_useCurrentView(false)
</span><span class="cx"> , m_zoomAndPan(SVGZoomAndPanMagnify)
</span><span class="cx"> , m_timeContainer(SMILTimeContainer::create(this))
</span><span class="lines">@@ -257,11 +257,23 @@
</span><span class="cx"> setXBaseValue(SVGLength::construct(LengthModeWidth, value, parseError));
</span><span class="cx"> else if (name == SVGNames::yAttr)
</span><span class="cx"> setYBaseValue(SVGLength::construct(LengthModeHeight, value, parseError));
</span><del>- else if (name == SVGNames::widthAttr)
- setWidthBaseValue(SVGLength::construct(LengthModeWidth, value, parseError, ForbidNegativeLengths));
- else if (name == SVGNames::heightAttr)
- setHeightBaseValue(SVGLength::construct(LengthModeHeight, value, parseError, ForbidNegativeLengths));
- else if (SVGLangSpace::parseAttribute(name, value)
</del><ins>+ else if (name == SVGNames::widthAttr) {
+ SVGLength length = SVGLength::construct(LengthModeWidth, value, parseError, ForbidNegativeLengths);
+ if (parseError != NoError || value.isEmpty()) {
+ // FIXME: This is definitely the correct behavior for a missing/removed attribute.
+ // Not sure it's correct for the empty string or for something that can't be parsed.
+ length = SVGLength(LengthModeWidth, ASCIILiteral("100%"));
+ }
+ setWidthBaseValue(length);
+ } else if (name == SVGNames::heightAttr) {
+ SVGLength length = SVGLength::construct(LengthModeHeight, value, parseError, ForbidNegativeLengths);
+ if (parseError != NoError || value.isEmpty()) {
+ // FIXME: This is definitely the correct behavior for a removed attribute.
+ // Not sure it's correct for the empty string or for something that can't be parsed.
+ length = SVGLength(LengthModeHeight, ASCIILiteral("100%"));
+ }
+ setHeightBaseValue(length);
+ } else if (SVGLangSpace::parseAttribute(name, value)
</ins><span class="cx"> || SVGExternalResourcesRequired::parseAttribute(name, value)
</span><span class="cx"> || SVGFitToViewBox::parseAttribute(this, name, value)
</span><span class="cx"> || SVGZoomAndPan::parseAttribute(this, name, value)) {
</span></span></pre></div>
<a id="trunkSourceWebCoresvgSVGUseElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (179068 => 179069)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2015-01-24 19:14:44 UTC (rev 179068)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2015-01-24 19:55:58 UTC (rev 179069)
</span><span class="lines">@@ -213,6 +213,26 @@
</span><span class="cx"> return 0;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static void updateWidthAndHeight(SVGElement& shadowElement, const SVGUseElement& useElement, const SVGElement& originalElement)
+{
+ // FIXME: The check for valueInSpecifiedUnits being non-zero below is a workaround for the fact
+ // that we currently have no good way to tell whether a particular animatable attribute is a value
+ // indicating it was unspecified, or specified but could not be parsed. Would be nice to fix that some day.
+ if (is<SVGSymbolElement>(shadowElement)) {
+ // Spec (<use> on <symbol>): 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.
+ shadowElement.setAttribute(SVGNames::widthAttr, (useElement.widthIsValid() && useElement.width().valueInSpecifiedUnits()) ? AtomicString(useElement.width().valueAsString()) : "100%");
+ shadowElement.setAttribute(SVGNames::heightAttr, (useElement.heightIsValid() && useElement.height().valueInSpecifiedUnits()) ? AtomicString(useElement.height().valueAsString()) : "100%");
+ } else if (is<SVGSVGElement>(shadowElement)) {
+ // Spec (<use> on <svg>): If attributes width and/or height are provided on the 'use' element, then these
+ // values will override the corresponding attributes on the 'svg' in the generated tree.
+ shadowElement.setAttribute(SVGNames::widthAttr, (useElement.widthIsValid() && useElement.width().valueInSpecifiedUnits()) ? AtomicString(useElement.width().valueAsString()) : originalElement.getAttribute(SVGNames::widthAttr));
+ shadowElement.setAttribute(SVGNames::heightAttr, (useElement.heightIsValid() && useElement.height().valueInSpecifiedUnits()) ? AtomicString(useElement.height().valueAsString()) : originalElement.getAttribute(SVGNames::heightAttr));
+ }
+}
+
</ins><span class="cx"> void SVGUseElement::svgAttributeChanged(const QualifiedName& attrName)
</span><span class="cx"> {
</span><span class="cx"> if (!isSupportedAttribute(attrName)) {
</span><span class="lines">@@ -222,13 +242,16 @@
</span><span class="cx">
</span><span class="cx"> SVGElementInstance::InvalidationGuard invalidationGuard(this);
</span><span class="cx">
</span><del>- auto renderer = this->renderer();
- if (attrName == SVGNames::xAttr
- || attrName == SVGNames::yAttr
- || attrName == SVGNames::widthAttr
- || attrName == SVGNames::heightAttr) {
</del><ins>+ if (attrName == SVGNames::xAttr || attrName == SVGNames::yAttr || attrName == SVGNames::widthAttr || attrName == SVGNames::heightAttr) {
</ins><span class="cx"> updateRelativeLengthsInformation();
</span><del>- if (renderer)
</del><ins>+ if (m_targetElementInstance) {
+ // FIXME: It's unnecessarily inefficient to do this work any time we change "x" or "y".
+ // FIXME: It's unnecessarily inefficient to update both width and height each time either is changed.
+ ASSERT(m_targetElementInstance->shadowTreeElement());
+ ASSERT(m_targetElementInstance->correspondingElement());
+ updateWidthAndHeight(*m_targetElementInstance->shadowTreeElement(), *this, *m_targetElementInstance->correspondingElement());
+ }
+ if (auto* renderer = this->renderer())
</ins><span class="cx"> RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="lines">@@ -429,6 +452,10 @@
</span><span class="cx"> // shadow tree elements <-> instances in the instance tree.
</span><span class="cx"> associateInstancesWithShadowTreeElements(shadowTreeRootElement->firstChild(), m_targetElementInstance.get());
</span><span class="cx">
</span><ins>+ ASSERT(m_targetElementInstance->shadowTreeElement());
+ ASSERT(m_targetElementInstance->correspondingElement() == target);
+ updateWidthAndHeight(*m_targetElementInstance->shadowTreeElement(), *this, *target);
+
</ins><span class="cx"> // If no shadow tree element is present, this means that the reference root
</span><span class="cx"> // element was removed, as it is disallowed (ie. <use> on <foreignObject>)
</span><span class="cx"> // Do NOT leave an inconsistent instance tree around, instead destruct it.
</span><span class="lines">@@ -651,7 +678,7 @@
</span><span class="cx">
</span><span class="cx"> if (target && !isDisallowedElement(*target)) {
</span><span class="cx"> RefPtr<Element> newChild = target->cloneElementWithChildren(document());
</span><del>- ASSERT(newChild->isSVGElement());
</del><ins>+ updateWidthAndHeight(downcast<SVGElement>(*newChild), use, *target);
</ins><span class="cx"> cloneParent->appendChild(newChild.release());
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>