Bug 1062106 part 1 - Remove special handling of attributeType='XML'. r?longsonr, birtles
This patch removes handling of the 'attributeType' attribute so that we behave
as if attributeType is always 'auto'. This means that for CSS properties we
always animate them as CSS properties (i.e. we animate them as part of the
SMIL override stylesheet) rather than mapped attributes.
The one special case is width/height on an outer SVG. Previously we animated
this as a mapped attribute since Web compatibility requires that the
width/height on an outer SVG, when set explicitly, are mapped to style.
However, we can produce the same behavior by animating these as CSS properties
(as opposed to mapped attributes). There is no observable difference in results
returned by the SVG DOM APIs, only the level at which the result is added to
the cascade: the SMIL override stylesheet instead of the attribute animation
presentation hint level.
As part of this patch, we animate width/height on outer SVG elements as CSS
properties as opposed to mapped attributes both for consistency and also so
we can remove the animated mapped attribute code altogether.
MozReview-Commit-ID: Ll1LWWRQ66R
--- a/dom/smil/nsSMILAnimationController.cpp
+++ b/dom/smil/nsSMILAnimationController.cpp
@@ -685,46 +685,20 @@ nsSMILAnimationController::GetTargetIden
return false;
// animateTransform can only animate transforms, conversely transforms
// can only be animated by animateTransform
if (IsTransformAttribute(attributeNamespaceID, attributeName) !=
(aAnimElem->IsSVGElement(nsGkAtoms::animateTransform)))
return false;
- // Look up target (animated) attribute-type
- nsSMILTargetAttrType attributeType = aAnimElem->GetTargetAttributeType();
-
- // Check if an 'auto' attributeType refers to a CSS property or XML attribute.
- // Note that SMIL requires we search for CSS properties first. So if they
- // overlap, 'auto' = 'CSS'. (SMILANIM 3.1)
- bool isCSS = false;
- if (attributeType == eSMILTargetAttrType_auto) {
- if (attributeNamespaceID == kNameSpaceID_None) {
- // width/height are special as they may be attributes or for
- // outer-<svg> elements, mapped into style.
- if (attributeName == nsGkAtoms::width ||
- attributeName == nsGkAtoms::height) {
- isCSS = targetElem->GetNameSpaceID() != kNameSpaceID_SVG;
- } else {
- nsCSSPropertyID prop =
- nsCSSProps::LookupProperty(nsDependentAtomString(attributeName),
- CSSEnabledState::eForAllContent);
- isCSS = nsSMILCSSProperty::IsPropertyAnimatable(prop);
- }
- }
- } else {
- isCSS = (attributeType == eSMILTargetAttrType_CSS);
- }
-
// Construct the key
- aResult.mElement = targetElem;
- aResult.mAttributeName = attributeName;
+ aResult.mElement = targetElem;
+ aResult.mAttributeName = attributeName;
aResult.mAttributeNamespaceID = attributeNamespaceID;
- aResult.mIsCSS = isCSS;
return true;
}
void
nsSMILAnimationController::AddStyleUpdatesTo(RestyleTracker& aTracker)
{
MOZ_ASSERT(mMightHavePendingStyleUpdates,
@@ -734,23 +708,19 @@ nsSMILAnimationController::AddStyleUpdat
SVGAnimationElement* animElement = iter.Get()->GetKey();
nsSMILTargetIdentifier key;
if (!GetTargetIdentifierForAnimation(animElement, key)) {
// Something's wrong/missing about animation's target; skip this animation
continue;
}
- // mIsCSS true means that the rules are the ones returned from
- // Element::GetSMILOverrideStyleDeclaration (via nsSMILCSSProperty objects),
- // and mIsCSS false means the rules are nsSMILMappedAttribute objects
- // returned from nsSVGElement::GetAnimatedContentDeclarationBlock.
- nsRestyleHint rshint = key.mIsCSS ? eRestyle_StyleAttribute_Animations
- : eRestyle_SVGAttrAnimations;
- aTracker.AddPendingRestyle(key.mElement, rshint, nsChangeHint(0));
+ aTracker.AddPendingRestyle(key.mElement,
+ eRestyle_StyleAttribute_Animations,
+ nsChangeHint(0));
}
mMightHavePendingStyleUpdates = false;
}
//----------------------------------------------------------------------
// Add/remove child time containers
--- a/dom/smil/nsSMILCompositor.cpp
+++ b/dom/smil/nsSMILCompositor.cpp
@@ -20,18 +20,17 @@ nsSMILCompositor::KeyEquals(KeyTypePoint
/*static*/ PLDHashNumber
nsSMILCompositor::HashKey(KeyTypePointer aKey)
{
// Combine the 3 values into one numeric value, which will be hashed.
// NOTE: We right-shift one of the pointers by 2 to get some randomness in
// its 2 lowest-order bits. (Those shifted-off bits will always be 0 since
// our pointers will be word-aligned.)
return (NS_PTR_TO_UINT32(aKey->mElement.get()) >> 2) +
- NS_PTR_TO_UINT32(aKey->mAttributeName.get()) +
- (aKey->mIsCSS ? 1 : 0);
+ NS_PTR_TO_UINT32(aKey->mAttributeName.get());
}
// Cycle-collection support
void
nsSMILCompositor::Traverse(nsCycleCollectionTraversalCallback* aCallback)
{
if (!mKey.mElement)
return;
@@ -122,28 +121,36 @@ nsSMILCompositor::ClearAnimationEffects(
smilAttr->ClearAnimValue();
}
// Protected Helper Functions
// --------------------------
nsISMILAttr*
nsSMILCompositor::CreateSMILAttr()
{
- if (mKey.mIsCSS) {
- nsCSSPropertyID propId =
- nsCSSProps::LookupProperty(nsDependentAtomString(mKey.mAttributeName),
- CSSEnabledState::eForAllContent);
- if (nsSMILCSSProperty::IsPropertyAnimatable(propId)) {
- return new nsSMILCSSProperty(propId, mKey.mElement.get());
+ nsCSSPropertyID propID =
+ nsCSSProps::LookupProperty(nsDependentAtomString(mKey.mAttributeName),
+ CSSEnabledState::eForAllContent);
+ if (nsSMILCSSProperty::IsPropertyAnimatable(propID)) {
+ // If we are animating the 'width' or 'height' of an outer SVG
+ // element we should animate it as a CSS property, but for other elements
+ // (e.g. <rect>) we should animate it as a length attribute.
+ // The easiest way to test for an outer SVG element, is to see if it is an
+ // SVG-namespace element mapping its width/height attribute to style.
+ bool animateAsAttr = (mKey.mAttributeName == nsGkAtoms::width ||
+ mKey.mAttributeName == nsGkAtoms::height) &&
+ mKey.mElement->GetNameSpaceID() == kNameSpaceID_SVG &&
+ !mKey.mElement->IsAttributeMapped(mKey.mAttributeName);
+ if (!animateAsAttr) {
+ return new nsSMILCSSProperty(propID, mKey.mElement.get());
}
- } else {
- return mKey.mElement->GetAnimatedAttr(mKey.mAttributeNamespaceID,
- mKey.mAttributeName);
}
- return nullptr;
+
+ return mKey.mElement->GetAnimatedAttr(mKey.mAttributeNamespaceID,
+ mKey.mAttributeName);
}
uint32_t
nsSMILCompositor::GetFirstFuncToAffectSandwich()
{
// For performance reasons, we throttle most animations on elements in
// display:none subtrees. (We can't throttle animations that target the
// "display" property itself, though -- if we did, display:none elements
--- a/dom/smil/nsSMILTargetIdentifier.h
+++ b/dom/smil/nsSMILTargetIdentifier.h
@@ -7,44 +7,42 @@
#ifndef NS_SMILTARGETIDENTIFIER_H_
#define NS_SMILTARGETIDENTIFIER_H_
#include "mozilla/dom/Element.h"
/**
* Struct: nsSMILTargetIdentifier
*
- * Tuple of: { Animated Element, Attribute Name, Attribute Type (CSS vs. XML) }
+ * Tuple of: { Animated Element, Attribute Name }
*
* Used in nsSMILAnimationController as hash key for mapping an animation
* target to the nsSMILCompositor for that target.
*
* NOTE: Need a nsRefPtr for the element & attribute name, because
* nsSMILAnimationController retain its hash table for one sample into the
* future, and we need to make sure their target isn't deleted in that time.
*/
struct nsSMILTargetIdentifier
{
nsSMILTargetIdentifier()
: mElement(nullptr), mAttributeName(nullptr),
- mAttributeNamespaceID(kNameSpaceID_Unknown), mIsCSS(false) {}
+ mAttributeNamespaceID(kNameSpaceID_Unknown) {}
inline bool Equals(const nsSMILTargetIdentifier& aOther) const
{
return (aOther.mElement == mElement &&
aOther.mAttributeName == mAttributeName &&
- aOther.mAttributeNamespaceID == mAttributeNamespaceID &&
- aOther.mIsCSS == mIsCSS);
+ aOther.mAttributeNamespaceID == mAttributeNamespaceID);
}
RefPtr<mozilla::dom::Element> mElement;
- RefPtr<nsIAtom> mAttributeName;
- int32_t mAttributeNamespaceID;
- bool mIsCSS;
+ RefPtr<nsIAtom> mAttributeName;
+ int32_t mAttributeNamespaceID;
};
/**
* Class: nsSMILWeakTargetIdentifier
*
* Version of the above struct that uses non-owning pointers. These are kept
* private, to ensure that they aren't ever dereferenced (or used at all,
* outside of Equals()).
@@ -52,35 +50,32 @@ struct nsSMILTargetIdentifier
* This is solely for comparisons to determine if a target has changed
* from one sample to the next.
*/
class nsSMILWeakTargetIdentifier
{
public:
// Trivial constructor
nsSMILWeakTargetIdentifier()
- : mElement(nullptr), mAttributeName(nullptr), mIsCSS(false) {}
+ : mElement(nullptr), mAttributeName(nullptr) {}
// Allow us to update a weak identifier to match a given non-weak identifier
nsSMILWeakTargetIdentifier&
operator=(const nsSMILTargetIdentifier& aOther)
{
mElement = aOther.mElement;
mAttributeName = aOther.mAttributeName;
- mIsCSS = aOther.mIsCSS;
return *this;
}
// Allow for comparison vs. non-weak identifier
inline bool Equals(const nsSMILTargetIdentifier& aOther) const
{
return (aOther.mElement == mElement &&
- aOther.mAttributeName == mAttributeName &&
- aOther.mIsCSS == mIsCSS);
+ aOther.mAttributeName == mAttributeName);
}
private:
const nsIContent* mElement;
const nsIAtom* mAttributeName;
- bool mIsCSS;
};
#endif // NS_SMILTARGETIDENTIFIER_H_
--- a/dom/svg/nsSVGElement.cpp
+++ b/dom/svg/nsSVGElement.cpp
@@ -2618,34 +2618,16 @@ nsSVGElement::RecompileScriptEventListen
SetEventHandler(GetEventNameForAttr(attr), value, true);
}
}
nsISMILAttr*
nsSVGElement::GetAnimatedAttr(int32_t aNamespaceID, nsIAtom* aName)
{
if (aNamespaceID == kNameSpaceID_None) {
- // We check mapped-into-style attributes first so that animations
- // targeting width/height on outer-<svg> don't appear to be ignored
- // because we returned a nsISMILAttr for the corresponding
- // SVGAnimatedLength.
-
- // Mapped attributes:
- if (IsAttributeMapped(aName)) {
- nsCSSPropertyID prop =
- nsCSSProps::LookupProperty(nsDependentAtomString(aName),
- CSSEnabledState::eForAllContent);
- // Check IsPropertyAnimatable to avoid attributes that...
- // - map to explicitly unanimatable properties (e.g. 'direction')
- // - map to unsupported attributes (e.g. 'glyph-orientation-horizontal')
- if (nsSMILCSSProperty::IsPropertyAnimatable(prop)) {
- return new nsSMILMappedAttribute(prop, this);
- }
- }
-
// Transforms:
if (GetTransformListAttrName() == aName) {
// The transform attribute is being animated, so we must ensure that the
// SVGAnimatedTransformList is/has been allocated:
return GetAnimatedTransformList(DO_ALLOCATE)->ToSMILAttr(this);
}
// Motion (fake 'attribute' for animateMotion)
--- a/layout/reftests/svg/smil/anim-remove-6.svg
+++ b/layout/reftests/svg/smil/anim-remove-6.svg
@@ -1,24 +1,24 @@
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
onload="go()">
<!-- In this test, we change an animation element's "attributeType" attribute
- to "CSS", which invalidates a completed, frozen animation (since in SVG,
- there's no CSS property that matches the attributeName that we're
- animating, "x"). We verify that animation effects are removed from the
- previously-targeted attribute. -->
+ to refer to another namespace, which invalidates a completed, frozen animation
+ (since there is no animatable 'x' attribute in the xlink namespace on a rect)
+ We verify that animation effects are removed from the previously-targeted
+ attribute. -->
<script>
function go() {
// Seek animation before we start tweaking things, to make sure we've
// already started sampling it.
document.documentElement.setCurrentTime(2.0);
var anim = document.getElementById("anim");
- anim.setAttributeNS(null, "attributeType", "CSS");
+ anim.setAttributeNS(null, "attributeName", "xlink:x");
setTimeAndSnapshot(2.5, false);
}
</script>
<script xlink:href="smil-util.js" type="text/javascript"/>
<rect x="15" y="15" width="200" height="200" fill="blue">
<animate id="anim" attributeName="x"
begin="0s" dur="2s" by="100" fill="freeze"/>
</rect>
deleted file mode 100644
--- a/layout/reftests/svg/smil/mapped-attr-vs-css-prop-1.svg
+++ /dev/null
@@ -1,128 +0,0 @@
-<svg xmlns="http://www.w3.org/2000/svg"
- xmlns:xlink="http://www.w3.org/1999/xlink"
- class="reftest-wait"
- onload="document.documentElement.setCurrentTime(1);
- setTimeAndSnapshot(1, false)">
- <!-- XXXdholbert the above "document.documentElement.setCurrentTime" call
- shouldn't be necessary - it's a temporary workaround for Bug 552873 -->
- <script xlink:href="smil-util.js" type="text/javascript"/>
- <!-- This test consists of a 4x3 grid, containing various cases with
- animations that have attributeType="CSS" vs "XML", for the
- presentational ("mapped") attribute 'fill-opacity'.
-
- As one would expect, the "CSS" and "XML" versions are treated as
- separate animation targets. However, there are some interactions:
- - When they conflict, both the inline style & any animations with
- attributeType="CSS" will have higher priority than the XML attribute
- and animations with attributeType="XML".
- - However, as described in the "FIRST ROW" comment below, animations
- with attributeType="XML" can sometimes feed in to the base value
- used for animations with attributeType="CSS". -->
-
- <!-- GIANT GREEN BACKGROUND -->
- <!-- (We'll put red rects on top, and then animate them to be transparent -->
- <rect height="100%" width="100%" fill="lime" />
-
- <!-- FIRST ROW: Additive CSS and XML animations, with CSS or XML base values.
-
- When the base value is set using the inline style attribute, the
- attributeType="XML" animation should be ignored, because the XML
- attribute gets masked by the inline style.
-
- However, when the base value is set using the XML attribute (or more
- generally, when attributeType="XML" animations aren't masked by a value
- in the inline style or in a stylesheet), then the animations will
- effectively add together, because the (animated) XML attribute feeds
- into the computed style, which gets used as the base value for the CSS
- animation. -->
- <g>
- <!-- CSS base value + CSS animation + XML animation -->
- <rect x="0" width="50" height="50" fill="red" style="fill-opacity: 0.5">
- <animate attributeName="fill-opacity" attributeType="XML"
- by="1" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="CSS"
- by="-0.5" dur="1s" fill="freeze"/>
- </rect>
- <!-- CSS base value + XML animation + CSS animation -->
- <rect x="50" width="50" height="50" fill="red" style="fill-opacity: 0.5">
- <animate attributeName="fill-opacity" attributeType="CSS"
- by="-0.5" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="XML"
- by="1" dur="1s" fill="freeze"/>
- </rect>
- <!-- XML base value + CSS animation + XML animation -->
- <rect x="100" width="50" height="50" fill="red" fill-opacity="0.5">
- <animate attributeName="fill-opacity" attributeType="XML"
- by="-0.2" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="CSS"
- by="-0.3" dur="1s" fill="freeze"/>
- </rect>
- <!-- XML base value + XML animation + CSS animation -->
- <rect x="150" width="50" height="50" fill="red" fill-opacity="0.5">
- <animate attributeName="fill-opacity" attributeType="CSS"
- by="-0.2" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="XML"
- by="-0.3" dur="1s" fill="freeze"/>
- </rect>
- </g>
-
- <!-- SECOND ROW: Single animation, with CSS or XML attributeType & base value.
- In every case except for CSS-base-value + XML animation, the animation
- should take effect. -->
- <g transform="translate(0, 50)">
- <!-- CSS base value + CSS animation -->
- <rect x="0" width="50" height="50" fill="red" style="fill-opacity: 0.5">
- <animate attributeName="fill-opacity" attributeType="CSS"
- to="0" dur="1s" fill="freeze"/>
- </rect>
- <!-- CSS base value + XML animation -->
- <!-- (starting at fill-opacity 0, since anim shouldn't have any effect -->
- <rect x="50" width="50" height="50" fill="red" style="fill-opacity: 0">
- <animate attributeName="fill-opacity" attributeType="XML"
- to="0.5" dur="1s" fill="freeze"/>
- </rect>
- <!-- XML base value + CSS animation -->
- <rect x="100" width="50" height="50" fill="red" fill-opacity="0.5">
- <animate attributeName="fill-opacity" attributeType="CSS"
- to="0" dur="1s" fill="freeze"/>
- </rect>
- <!-- XML base value + XML animation -->
- <rect x="150" width="50" height="50" fill="red" fill-opacity="0.5">
- <animate attributeName="fill-opacity" attributeType="XML"
- to="0" dur="1s" fill="freeze"/>
- </rect>
- </g>
-
- <!-- THIRD ROW: Competing animations, with CSS or XML attributeType & base
- value. In each case, the attributeType="CSS" animation should win. -->
- <g transform="translate(0, 100)">
- <!-- CSS base value + CSS animation animation vs XML animation -->
- <rect x="0" width="50" height="50" fill="red" style="fill-opacity: 0.5">
- <animate attributeName="fill-opacity" attributeType="CSS"
- to="0" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="XML"
- to="1" dur="1s" fill="freeze"/>
- </rect>
- <!-- CSS base value + XML animation vs CSS animation -->
- <rect x="50" width="50" height="50" fill="red" style="fill-opacity: 0.5">
- <animate attributeName="fill-opacity" attributeType="XML"
- to="1" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="CSS"
- to="0" dur="1s" fill="freeze"/>
- </rect>
- <!-- XML base value + CSS animation vs XML animation -->
- <rect x="100" width="50" height="50" fill="red" fill-opacity="0.5">
- <animate attributeName="fill-opacity" attributeType="CSS"
- to="0" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="XML"
- to="1" dur="1s" fill="freeze"/>
- </rect>
- <!-- XML base value + XML animation vs CSS animation -->
- <rect x="150" width="50" height="50" fill="red" fill-opacity="0.5">
- <animate attributeName="fill-opacity" attributeType="XML"
- to="1" dur="1s" fill="freeze"/>
- <animate attributeName="fill-opacity" attributeType="CSS"
- to="0" dur="1s" fill="freeze"/>
- </rect>
- </g>
-</svg>
--- a/layout/reftests/svg/smil/reftest-stylo.list
+++ b/layout/reftests/svg/smil/reftest-stylo.list
@@ -249,19 +249,16 @@ fuzzy-if(cocoaWidget&&layersGPUAccelerat
== frozen-to-anim-1.svg frozen-to-anim-1.svg
== inactivate-with-active-unchanged-1.svg inactivate-with-active-unchanged-1.svg
== inactivate-with-active-unchanged-2.svg inactivate-with-active-unchanged-2.svg
== mapped-attr-long-url-1.svg mapped-attr-long-url-1.svg
== mapped-attr-long-url-2.svg mapped-attr-long-url-2.svg
-# interaction between xml mapped attributes and their css equivalents
-== mapped-attr-vs-css-prop-1.svg mapped-attr-vs-css-prop-1.svg
-
== min-1.svg min-1.svg
== smil-transitions-interaction-1a.svg smil-transitions-interaction-1a.svg
== smil-transitions-interaction-1b.svg smil-transitions-interaction-1b.svg
== smil-transitions-interaction-2a.svg smil-transitions-interaction-2a.svg
== smil-transitions-interaction-2b.svg smil-transitions-interaction-2b.svg
== smil-transitions-interaction-3a.svg smil-transitions-interaction-3a.svg
== smil-transitions-interaction-3b.svg smil-transitions-interaction-3b.svg
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -248,19 +248,16 @@ fuzzy-if(cocoaWidget&&layersGPUAccelerat
== frozen-to-anim-1.svg lime.svg
== inactivate-with-active-unchanged-1.svg anim-standard-ref.svg
== inactivate-with-active-unchanged-2.svg anim-standard-ref.svg
== mapped-attr-long-url-1.svg lime.svg
== mapped-attr-long-url-2.svg lime.svg
-# interaction between xml mapped attributes and their css equivalents
-== mapped-attr-vs-css-prop-1.svg lime.svg
-
== min-1.svg lime.svg
== smil-transitions-interaction-1a.svg lime.svg
== smil-transitions-interaction-1b.svg lime.svg
== smil-transitions-interaction-2a.svg lime.svg
== smil-transitions-interaction-2b.svg lime.svg
== smil-transitions-interaction-3a.svg lime.svg
== smil-transitions-interaction-3b.svg lime.svg
--- a/layout/style/test/test_restyles_in_smil_animation.html
+++ b/layout/style/test/test_restyles_in_smil_animation.html
@@ -41,17 +41,17 @@ function observeStyling(frameCount) {
docShell.recordProfileTimelineMarkers = true;
docShell.popProfileTimelineMarkers();
return new Promise(function(resolve) {
return waitForAnimationFrames(frameCount).then(function() {
var markers = docShell.popProfileTimelineMarkers();
docShell.recordProfileTimelineMarkers = false;
var stylingMarkers = markers.filter(function(marker, index) {
- return marker.restyleHint == "eRestyle_SVGAttrAnimations";
+ return marker.restyleHint == "eRestyle_StyleAttribute_Animations";
});
resolve(stylingMarkers);
});
});
}
function ensureElementRemoval(aElement) {
return new Promise(function(resolve) {
