Bug 1390364 - Don't allow interpolating 'fill:none' with 'fill:none'; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 16 Aug 2017 14:51:31 +0900
changeset 647273 d6d4256433f1d319dfa5de5c8456084ceeea68b7
parent 647200 4e93516e92e58d166ad37b8544c3230024afb587
child 726446 5c8db21c5bf925a710e2543b1ba10bca1b73bfaa
push id74344
push userbmo:bbirtles@mozilla.com
push dateWed, 16 Aug 2017 05:52:02 +0000
reviewershiro
bugs1390364
milestone57.0a1
Bug 1390364 - Don't allow interpolating 'fill:none' with 'fill:none'; r?hiro In SMIL we don't expect the 'none' value of the 'fill' property to be additive and hence the following animation should have no effect: <rect width="100" height="100" y="100" fill="blue"> <animate attributeName="fill" dur="3s" from="red" by="none"/> </rect> Although SMIL doesn't make this entirely clear, [1] says that "by animation" and "from-by animation" may only be used "with attributes that support addition (e.g. most numeric attributes)" and [2] says that <paint>s are "only additive if each value can be converted to an RGB color". As a result, the animation above should have no effect. By extrapolation, animating from 'none' by 'none' should also have no effect: <rect width="100" height="100" y="100" fill="blue"> <animate attributeName="fill" dur="3s" from="none" by="none"/> </rect> However, in Servo's interpolation of <paint>s we special case the interpolation and addition of 'none' such that if both values are 'none' it is allowed. We should disallow this in order to produce the expected behavior and in order to match Gecko's behavior. [1] https://www.w3.org/TR/smil-animation/#AnimFuncValues [2] https://www.w3.org/TR/SVG11/animate.html#AnimationAttributesAndProperties MozReview-Commit-ID: 2w2eH3Xdzhv
servo/components/style/properties/helpers/animated_properties.mako.rs
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -2473,17 +2473,16 @@ impl Animatable for IntermediateSVGPaint
     #[inline]
     fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64) -> Result<Self, ()> {
         match (self, other) {
             (&SVGPaintKind::Color(ref self_color), &SVGPaintKind::Color(ref other_color)) => {
                 Ok(SVGPaintKind::Color(self_color.add_weighted(other_color, self_portion, other_portion)?))
             }
             // FIXME context values should be interpolable with colors
             // Gecko doesn't implement this behavior either.
-            (&SVGPaintKind::None, &SVGPaintKind::None) => Ok(SVGPaintKind::None),
             (&SVGPaintKind::ContextFill, &SVGPaintKind::ContextFill) => Ok(SVGPaintKind::ContextFill),
             (&SVGPaintKind::ContextStroke, &SVGPaintKind::ContextStroke) => Ok(SVGPaintKind::ContextStroke),
             _ => Err(())
         }
     }
 }
 
 impl ComputeSquaredDistance for IntermediateSVGPaintKind {