- Revision
- 109679
- Author
- [email protected]
- Date
- 2012-03-04 11:07:52 -0800 (Sun, 04 Mar 2012)
Log Message
Introduce SMIL overrideStyle, to make SVG stop mutating CSS styles directly
https://bugs.webkit.org/show_bug.cgi?id=79790
Reviewed by Dirk Schulze.
Source/WebCore:
Restore SMIL animation behavior back to pre-r109342.
1) Always animate presentation attributes as CSS properties, regardless of the
attributeTypes value. Matches Opera/FF, and makes the attribute optional again
as specified in both SMIL & SVG.
Extend existing svg/animations/attributesTypes.html to verify this.
2) Switch setInstancesUpdatesBlocked calls to the right locations again, to
avoid chromium assertions (and/or extra work being done). Fixing that reveals another
problem: in the instance updating code path, we always called setTargetAttributeAnimatedCSSValue
even for XML animations.
* svg/SVGAnimationElement.cpp:
(WebCore::SVGAnimationElement::shouldApplyAnimation): Restore old logic.
(WebCore::SVGAnimationElement::setTargetAttributeAnimatedValue): Fix typo, move setInstancesUpdatesBlocked calls.
LayoutTests:
Extended attributesTypes.html test case to check that presentation attributes are
always animated as CSS properties, regardless of their attributeTypes value.
* platform/chromium/test_expectations.txt: Remove animate-elem-32-t.svg suppression.
* svg/animations/attributeTypes-expected.txt: Updated expectations.
* svg/animations/resources/attributeTypes.svg: Fix comment.
* svg/animations/script-tests/attributeTypes.js: Extend test.
(sample1):
(sample2):
(sample3):
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (109678 => 109679)
--- trunk/LayoutTests/ChangeLog 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/LayoutTests/ChangeLog 2012-03-04 19:07:52 UTC (rev 109679)
@@ -1,3 +1,21 @@
+2012-03-04 Nikolas Zimmermann <[email protected]>
+
+ Introduce SMIL overrideStyle, to make SVG stop mutating CSS styles directly
+ https://bugs.webkit.org/show_bug.cgi?id=79790
+
+ Reviewed by Dirk Schulze.
+
+ Extended attributesTypes.html test case to check that presentation attributes are
+ always animated as CSS properties, regardless of their attributeTypes value.
+
+ * platform/chromium/test_expectations.txt: Remove animate-elem-32-t.svg suppression.
+ * svg/animations/attributeTypes-expected.txt: Updated expectations.
+ * svg/animations/resources/attributeTypes.svg: Fix comment.
+ * svg/animations/script-tests/attributeTypes.js: Extend test.
+ (sample1):
+ (sample2):
+ (sample3):
+
2012-03-03 Raymond Toy <[email protected]>
DelayNode has a fixed one second max delay time
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (109678 => 109679)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2012-03-04 19:07:52 UTC (rev 109679)
@@ -4196,8 +4196,6 @@
// EPOGER: To be rebaselined after skia rev. 3283 lands in webkit
BUGCR116252 LINUX : platform/chromium-linux/fast/text/chromium-linux-fontconfig-renderstyle.html = IMAGE+TEXT
-BUGWK80053 DEBUG : svg/W3C-SVG-1.1/animate-elem-32-t.svg = CRASH PASS
-
BUGWK80058 WIN RELEASE : fast/workers/stress-js-execution.html = CRASH PASS
BUGWK80067 DEBUG : media/track/track-cues-pause-on-exit.html = PASS TEXT
Modified: trunk/LayoutTests/svg/animations/attributeTypes-expected.txt (109678 => 109679)
--- trunk/LayoutTests/svg/animations/attributeTypes-expected.txt 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/LayoutTests/svg/animations/attributeTypes-expected.txt 2012-03-04 19:07:52 UTC (rev 109679)
@@ -12,32 +12,40 @@
PASS getComputedStyle(rect2).fill is "#008000"
PASS rect3.width.animVal.value is 100
PASS getComputedStyle(rect3).fill is "#ff0000"
+PASS rect3.getAttribute('fill') is "red"
PASS rect4.width.animVal.value is 100
PASS getComputedStyle(rect4).fill is "#ff0000"
+PASS rect4.getAttribute('fill') is "red"
PASS rect1.width.animVal.value is 55
PASS getComputedStyle(rect1).fill is "#008000"
PASS rect2.width.animVal.value is 100
PASS getComputedStyle(rect2).fill is "#008000"
PASS rect3.width.animVal.value is 100
PASS getComputedStyle(rect3).fill is "#804000"
+PASS rect3.getAttribute('fill') is "red"
PASS rect4.width.animVal.value is 100
PASS getComputedStyle(rect4).fill is "#804000"
+PASS rect4.getAttribute('fill') is "red"
PASS rect1.width.animVal.value is 100
PASS getComputedStyle(rect1).fill is "#008000"
PASS rect2.width.animVal.value is 100
PASS getComputedStyle(rect2).fill is "#008000"
PASS rect3.width.animVal.value is 100
PASS getComputedStyle(rect3).fill is "#008000"
+PASS rect3.getAttribute('fill') is "red"
PASS rect4.width.animVal.value is 100
PASS getComputedStyle(rect4).fill is "#008000"
+PASS rect4.getAttribute('fill') is "red"
PASS rect1.width.animVal.value is 100
PASS getComputedStyle(rect1).fill is "#008000"
PASS rect2.width.animVal.value is 100
PASS getComputedStyle(rect2).fill is "#008000"
PASS rect3.width.animVal.value is 100
PASS getComputedStyle(rect3).fill is "#008000"
+PASS rect3.getAttribute('fill') is "red"
PASS rect4.width.animVal.value is 100
PASS getComputedStyle(rect4).fill is "#008000"
+PASS rect4.getAttribute('fill') is "red"
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/svg/animations/resources/attributeTypes.svg (109678 => 109679)
--- trunk/LayoutTests/svg/animations/resources/attributeTypes.svg 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/LayoutTests/svg/animations/resources/attributeTypes.svg 2012-03-04 19:07:52 UTC (rev 109679)
@@ -17,7 +17,7 @@
<animate id="an3" attributeType="auto" attributeName="fill" fill="freeze" from="red" to="green" begin="0s" dur="4s"/>
</rect>
-<!-- 'fill' is a presentation attribute, mapped to CSS, attributeType is set to "CSS": this animation runs. -->
+<!-- 'fill' is a presentation attribute, mapped to CSS, attributeType is set to "XML": this animation runs. -->
<rect x="150" y="150" width="100" height="100" fill="red">
<animate id="an4" attributeType="XML" attributeName="fill" fill="freeze" from="red" to="green" begin="0s" dur="4s"/>
</rect>
Modified: trunk/LayoutTests/svg/animations/script-tests/attributeTypes.js (109678 => 109679)
--- trunk/LayoutTests/svg/animations/script-tests/attributeTypes.js 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/LayoutTests/svg/animations/script-tests/attributeTypes.js 2012-03-04 19:07:52 UTC (rev 109679)
@@ -14,10 +14,12 @@
shouldBe("rect3.width.animVal.value", "100");
//shouldBe("rect3.width.baseVal.value", "100");
shouldBeEqualToString("getComputedStyle(rect3).fill", "#ff0000");
+ shouldBeEqualToString("rect3.getAttribute('fill')", "red");
shouldBe("rect4.width.animVal.value", "100");
//shouldBe("rect4.width.baseVal.value", "100");
shouldBeEqualToString("getComputedStyle(rect4).fill", "#ff0000");
+ shouldBeEqualToString("rect4.getAttribute('fill')", "red");
}
function sample2() {
@@ -32,10 +34,12 @@
shouldBe("rect3.width.animVal.value", "100");
//shouldBe("rect3.width.baseVal.value", "100");
shouldBeEqualToString("getComputedStyle(rect3).fill", "#804000");
+ shouldBeEqualToString("rect3.getAttribute('fill')", "red");
shouldBe("rect4.width.animVal.value", "100");
//shouldBe("rect4.width.baseVal.value", "100");
shouldBeEqualToString("getComputedStyle(rect4).fill", "#804000");
+ shouldBeEqualToString("rect4.getAttribute('fill')", "red");
}
function sample3() {
@@ -50,10 +54,12 @@
shouldBe("rect3.width.animVal.value", "100");
//shouldBe("rect3.width.baseVal.value", "100");
shouldBeEqualToString("getComputedStyle(rect3).fill", "#008000");
+ shouldBeEqualToString("rect3.getAttribute('fill')", "red");
shouldBe("rect4.width.animVal.value", "100");
//shouldBe("rect4.width.baseVal.value", "100");
shouldBeEqualToString("getComputedStyle(rect4).fill", "#008000");
+ shouldBeEqualToString("rect4.getAttribute('fill')", "red");
}
function executeTest() {
Modified: trunk/Source/WebCore/ChangeLog (109678 => 109679)
--- trunk/Source/WebCore/ChangeLog 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/Source/WebCore/ChangeLog 2012-03-04 19:07:52 UTC (rev 109679)
@@ -1,3 +1,26 @@
+2012-03-04 Nikolas Zimmermann <[email protected]>
+
+ Introduce SMIL overrideStyle, to make SVG stop mutating CSS styles directly
+ https://bugs.webkit.org/show_bug.cgi?id=79790
+
+ Reviewed by Dirk Schulze.
+
+ Restore SMIL animation behavior back to pre-r109342.
+ 1) Always animate presentation attributes as CSS properties, regardless of the
+ attributeTypes value. Matches Opera/FF, and makes the attribute optional again
+ as specified in both SMIL & SVG.
+
+ Extend existing svg/animations/attributesTypes.html to verify this.
+
+ 2) Switch setInstancesUpdatesBlocked calls to the right locations again, to
+ avoid chromium assertions (and/or extra work being done). Fixing that reveals another
+ problem: in the instance updating code path, we always called setTargetAttributeAnimatedCSSValue
+ even for XML animations.
+
+ * svg/SVGAnimationElement.cpp:
+ (WebCore::SVGAnimationElement::shouldApplyAnimation): Restore old logic.
+ (WebCore::SVGAnimationElement::setTargetAttributeAnimatedValue): Fix typo, move setInstancesUpdatesBlocked calls.
+
2012-03-04 Jonathan Dong <[email protected]>
[BlackBerry] upstream MediaPlayerPrivateBlackBerry.[cpp|h]
Modified: trunk/Source/WebCore/svg/SVGAnimationElement.cpp (109678 => 109679)
--- trunk/Source/WebCore/svg/SVGAnimationElement.cpp 2012-03-04 18:53:51 UTC (rev 109678)
+++ trunk/Source/WebCore/svg/SVGAnimationElement.cpp 2012-03-04 19:07:52 UTC (rev 109679)
@@ -357,13 +357,16 @@
if (shouldApply == DontApplyAnimation)
return;
+ if (targetElement->isStyled())
+ static_cast<SVGStyledElement*>(targetElement)->setInstanceUpdatesBlocked(true);
+
if (shouldApply == ApplyCSSAnimation)
setTargetAttributeAnimatedCSSValue(targetElement, attributeName, value);
else
setTargetAttributeAnimatedXMLValue(targetElement, attributeName, value);
if (targetElement->isStyled())
- static_cast<SVGStyledElement*>(targetElement)->setInstanceUpdatesBlocked(true);
+ static_cast<SVGStyledElement*>(targetElement)->setInstanceUpdatesBlocked(false);
// If the target element has instances, update them as well, w/o requiring the <use> tree to be rebuilt.
const HashSet<SVGElementInstance*>& instances = targetElement->instancesForElement();
@@ -376,11 +379,8 @@
if (shouldApply == ApplyCSSAnimation)
setTargetAttributeAnimatedCSSValue(shadowTreeElement, attributeName, value);
else
- setTargetAttributeAnimatedCSSValue(shadowTreeElement, attributeName, value);
+ setTargetAttributeAnimatedXMLValue(shadowTreeElement, attributeName, value);
}
-
- if (targetElement->isStyled())
- static_cast<SVGStyledElement*>(targetElement)->setInstanceUpdatesBlocked(false);
}
void SVGAnimationElement::resetAnimationState(const String& baseValue)
@@ -403,24 +403,15 @@
if (!hasValidAttributeType() || !targetElement || attributeName == anyQName())
return DontApplyAnimation;
- AttributeType attributeType = this->attributeType();
- switch (attributeType) {
- case AttributeTypeAuto:
- // For attributeType="auto", try CSS first. If that fails, try XML.
- case AttributeTypeCSS:
- // If attributeName is a known animatable SVG CSS property, apply the animation.
- if (isTargetAttributeCSSProperty(targetElement, attributeName))
- return ApplyCSSAnimation;
- // If attributeName is unknown and ttributeType is not 'auto', don't apply the animation.
- if (attributeType == AttributeTypeCSS)
- return DontApplyAnimation;
- // For attributeType="auto", try XML animation now.
- case AttributeTypeXML:
- return ApplyXMLAnimation;
- };
+ // Always animate CSS properties, using the ApplyCSSAnimation code path, regardless of the attributeType value.
+ if (isTargetAttributeCSSProperty(targetElement, attributeName))
+ return ApplyCSSAnimation;
- ASSERT_NOT_REACHED();
- return DontApplyAnimation;
+ // If attributeType="CSS" and attributeName doesn't point to a CSS property, ignore the animation.
+ if (attributeType() == AttributeTypeCSS)
+ return DontApplyAnimation;
+
+ return ApplyXMLAnimation;
}
void SVGAnimationElement::calculateKeyTimesForCalcModePaced()
