Bug 520239, patch 5: Enable SVG/SMIL animation of shorthand properties, and remove unneeded nsISMILCSSValueType interface. r=roc

content/smil/nsISMILCSSValueType.h

@@ -1,118 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is the Mozilla SMIL module.
- *
- * The Initial Developer of the Original Code is the Mozilla Corporation.
- * Portions created by the Initial Developer are Copyright (C) 2009
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- *   Daniel Holbert <dholbert@mozilla.com>
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either of the GNU General Public License Version 2 or later (the "GPL"),
- * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
-
-/* interface for accessing CSS values stored in nsSMILValue objects */
-
-#ifndef NS_ISMILCSSVALUETYPE_H_
-#define NS_ISMILCSSVALUETYPE_H_
-
-#include "nsISMILType.h"
-#include "nsCSSProperty.h"
-#include "nscore.h" // For NS_OVERRIDE
-
-class nsPresContext;
-class nsIContent;
-class nsAString;
-
-//////////////////////////////////////////////////////////////////////////////
-// nsISMILCSSValueType: Customized version of the nsISMILType interface, with
-// some additional methods for parsing & extracting CSS values, so that the
-// details of the value-storage representation can be abstracted away.
-
-class nsISMILCSSValueType : public nsISMILType
-{
-public:
-  // Methods inherited from nsISMILType
-  // ----------------------------------
-  NS_OVERRIDE virtual nsresult Init(nsSMILValue& aValue) const = 0;
-  NS_OVERRIDE virtual void     Destroy(nsSMILValue& aValue) const = 0;
-  NS_OVERRIDE virtual nsresult Assign(nsSMILValue& aDest,
-                                      const nsSMILValue& aSrc) const = 0;
-  NS_OVERRIDE virtual nsresult Add(nsSMILValue& aDest,
-                                   const nsSMILValue& aValueToAdd,
-                                   PRUint32 aCount) const = 0;
-  NS_OVERRIDE virtual nsresult ComputeDistance(const nsSMILValue& aFrom,
-                                               const nsSMILValue& aTo,
-                                               double& aDistance) const = 0;
-  NS_OVERRIDE virtual nsresult Interpolate(const nsSMILValue& aStartVal,
-                                           const nsSMILValue& aEndVal,
-                                           double aUnitDistance,
-                                           nsSMILValue& aResult) const = 0;
-
-  /*
-   * Virtual destructor: nothing to do here, but subclasses
-   * may need it.
-   */
-  virtual ~nsISMILCSSValueType() {};
-
-  // Methods introduced in this interface
-  // ------------------------------------
-  // These are helper methods used by nsSMILCSSProperty - these let us keep
-  // our data representation private.
-
-  /**
-   * Sets up the given nsSMILValue to represent the given string value.  The
-   * string is interpreted as a value for the given property on the given
-   * element.
-   *
-   * Note: aValue is expected to be freshly initialized (i.e. it should have
-   * been passed into the "Init()" method of some nsISMILCSSValueType subclass)
-   *
-   * @param       aPropID         The property for which we're parsing a value.
-   * @param       aTargetElement  The target element to whom the property/value
-   *                              setting applies.
-   * @param       aString         The string to be parsed as a CSS value.
-   * @param [out] aValue          The nsSMILValue to be populated.
-   * @return                      PR_TRUE on success, PR_FALSE on failure.
-   */
-  virtual PRBool ValueFromString(nsCSSProperty aPropID,
-                                 nsIContent* aTargetElement,
-                                 const nsAString& aString,
-                                 nsSMILValue& aValue) const = 0;
-
-  /**
-   * Creates a string representation of the given nsSMILValue.
-   *
-   * @param       aValue   The nsSMILValue to be converted into a string.
-   * @param [out] aString  The string to be populated with the given value.
-   * @return               PR_TRUE on success, PR_FALSE on failure.
-   */
-  virtual PRBool ValueToString(const nsSMILValue& aValue,
-                               nsAString& aString) const = 0;
-};
-
-#endif //  NS_ISMILCSSVALUETYPE_H_

content/smil/nsSMILCSSProperty.cpp

@@ -38,7 +38,6 @@
 /* representation of a SMIL-animatable CSS property on an element */
 
 #include "nsSMILCSSProperty.h"
-#include "nsISMILCSSValueType.h"
 #include "nsSMILCSSValueType.h"
 #include "nsSMILValue.h"
 #include "nsCSSDeclaration.h"
@@ -47,20 +46,6 @@
 #include "nsIContent.h"
 #include "nsPIDOMWindow.h"
 
-// Helper Functions
-static nsISMILCSSValueType*
-GetSMILTypeForProperty(nsCSSProperty aPropID)
-{
-  if (!nsSMILCSSProperty::IsPropertyAnimatable(aPropID)) {
-    NS_NOTREACHED("Attempting to animate an un-animatable property");
-    return nsnull;
-  }
-  if (aPropID < eCSSProperty_COUNT_no_shorthands) {
-    return &nsSMILCSSValueType::sSingleton;
-  }
-  return nsnull; // XXXdholbert Return shorthand type here, when we add it
-}
-
 static PRBool
 GetCSSComputedValue(nsIContent* aElem,
                     nsCSSProperty aPropID,
@@ -125,13 +110,12 @@ nsSMILCSSProperty::GetBaseValue() const
   nsSMILValue baseValue;
   if (didGetComputedVal) {
     // (4) Create the nsSMILValue from the computed style value
-    nsISMILCSSValueType* smilType = GetSMILTypeForProperty(mPropID);
+    nsSMILCSSValueType::sSingleton.Init(baseValue);
-    NS_ABORT_IF_FALSE(smilType, "animating an unsupported type");
+    if (!nsCSSProps::IsShorthand(mPropID) &&
-    
+        !nsSMILCSSValueType::sSingleton.ValueFromString(mPropID, mElement,
-    smilType->Init(baseValue);
+                                                        computedStyleVal,
-    if (!smilType->ValueFromString(mPropID, mElement,
+                                                        baseValue)) {
-                                   computedStyleVal, baseValue)) {
+      nsSMILCSSValueType::sSingleton.Destroy(baseValue);
-      smilType->Destroy(baseValue);
       NS_ABORT_IF_FALSE(baseValue.IsNull(),
                         "Destroy should leave us with null-typed value");
     }
@@ -145,11 +129,12 @@ nsSMILCSSProperty::ValueFromString(const nsAString& aStr,
                                    nsSMILValue& aValue) const
 {
   NS_ENSURE_TRUE(IsPropertyAnimatable(mPropID), NS_ERROR_FAILURE);
-  nsISMILCSSValueType* smilType = GetSMILTypeForProperty(mPropID);
+  nsSMILCSSValueType::sSingleton.Init(aValue);
-  smilType->Init(aValue);
+  PRBool success =
-  PRBool success = smilType->ValueFromString(mPropID, mElement, aStr, aValue);
+    nsSMILCSSValueType::sSingleton.ValueFromString(mPropID, mElement,
+                                                   aStr, aValue);
   if (!success) {
-    smilType->Destroy(aValue);
+    nsSMILCSSValueType::sSingleton.Destroy(aValue);
   }
   return success ? NS_OK : NS_ERROR_FAILURE;
 }
@@ -161,9 +146,8 @@ nsSMILCSSProperty::SetAnimValue(const nsSMILValue& aValue)
 
   nsresult rv = NS_OK;
   nsAutoString valStr;
-  nsISMILCSSValueType* smilType = GetSMILTypeForProperty(mPropID);
 
-  if (smilType->ValueToString(aValue, valStr)) {
+  if (nsSMILCSSValueType::sSingleton.ValueToString(aValue, valStr)) {
     // Apply the style to the target element
     nsCOMPtr<nsIDOMCSSStyleDeclaration> overrideStyle;
     mElement->GetSMILOverrideStyle(getter_AddRefs(overrideStyle));
@@ -215,8 +199,7 @@ nsSMILCSSProperty::IsPropertyAnimatable(nsCSSProperty aPropID)
     case eCSSProperty_font:
     case eCSSProperty_marker:
     case eCSSProperty_overflow:
-      // XXXdholbert Shorthand types not yet supported
+      return PR_TRUE;
-      return PR_FALSE;
 
     // PROPERTIES OF TYPE eCSSType_Rect
     case eCSSProperty_clip:

content/smil/nsSMILCSSValueType.cpp

@@ -307,9 +307,6 @@ nsSMILCSSValueType::ValueFromString(nsCSSProperty aPropID,
 {
   NS_ABORT_IF_FALSE(aValue.mType == &nsSMILCSSValueType::sSingleton,
                     "Passed-in value is wrong type");
-  NS_ABORT_IF_FALSE(aPropID < eCSSProperty_COUNT_no_shorthands,
-                    "nsSMILCSSValueType shouldn't be used with "
-                    "shorthand properties");
   NS_ABORT_IF_FALSE(!aValue.mU.mPtr, "expecting barely-initialized outparam");
 
   nsPresContext* presContext = GetPresContextForElement(aTargetElement);

content/smil/nsSMILCSSValueType.h

@@ -40,16 +40,18 @@
 #ifndef NS_SMILCSSVALUETYPE_H_
 #define NS_SMILCSSVALUETYPE_H_
 
-#include "nsISMILCSSValueType.h"
+#include "nsISMILType.h"
+#include "nsCSSProperty.h"
 #include "nscore.h" // For NS_OVERRIDE
 
+class nsPresContext;
 class nsIContent;
+class nsAString;
 
 /*
- * nsSMILCSSValueType: Represents a SMIL-animated simple (non-shorthand) CSS
+ * nsSMILCSSValueType: Represents a SMIL-animated CSS value.
- * value.
  */
-class nsSMILCSSValueType : public nsISMILCSSValueType
+class nsSMILCSSValueType : public nsISMILType
 {
 public:
   // nsISMILValueType Methods
@@ -69,15 +71,38 @@ public:
                                            double aUnitDistance,
                                            nsSMILValue& aResult) const;
 
-  // nsISMILCSSValueType Methods
+  /**
-  // ---------------------------
+   * Sets up the given nsSMILValue to represent the given string value.  The
-  NS_OVERRIDE virtual PRBool ValueFromString(nsCSSProperty aPropID,
+   * string is interpreted as a value for the given property on the given
-                                             nsIContent* aTargetElement,
+   * element.
-                                             const nsAString& aString,
+   *
-                                             nsSMILValue& aValue) const;
+   * Note: aValue is expected to be freshly initialized (i.e. it should already
+   * have been passed into nsSMILCSSValueType::Init(), and it should not have
+   * been set up further via e.g. Assign() or another ValueFromString() call.)
+   *
+   * @param       aPropID         The property for which we're parsing a value.
+   * @param       aTargetElement  The target element to whom the property/value
+   *                              setting applies.
+   * @param       aString         The string to be parsed as a CSS value.
+   * @param [out] aValue          The nsSMILValue to be populated.
+   * @return                      PR_TRUE on success, PR_FALSE on failure.
+   */
+  PRBool ValueFromString(nsCSSProperty aPropID, nsIContent* aTargetElement,
+                         const nsAString& aString, nsSMILValue& aValue) const;
 
-  NS_OVERRIDE virtual PRBool ValueToString(const nsSMILValue& aValue,
+  /**
-                                           nsAString& aString) const;
+   * Creates a string representation of the given nsSMILValue.
+   *
+   * Note: aValue is expected to be of this type (that is, it's expected to
+   * have been initialized by nsSMILCSSValueType::sSingleton).  If aValue is a
+   * freshly-initialized value, this method will succeed, though the resulting
+   * string will be empty.
+   *
+   * @param       aValue   The nsSMILValue to be converted into a string.
+   * @param [out] aString  The string to be populated with the given value.
+   * @return               PR_TRUE on success, PR_FALSE on failure.
+   */
+  PRBool ValueToString(const nsSMILValue& aValue, nsAString& aString) const;
 
   // Singleton for nsSMILValue objects to hold onto.
   static nsSMILCSSValueType sSingleton;

content/smil/test/db_smilCSSFromBy.js

@@ -91,6 +91,10 @@ var gFromByBundles =
                                                _fromByTestLists.paint)),
   // Check that 'by' animations involving URIs have no effect
   new TestcaseBundle(gPropList.filter,         _fromByTestLists.URIsAndNone),
+  new TestcaseBundle(gPropList.font, [
+    new AnimTestcaseFromBy("10px serif",
+                           "normal normal 400 100px / 10px monospace"),
+  ]),
   new TestcaseBundle(gPropList.font_size,      _fromByTestLists.lengthPx),
   new TestcaseBundle(gPropList.font_size_adjust, [
     // These testcases implicitly have no effect, because font-size-adjust is
@@ -100,6 +104,14 @@ var gFromByBundles =
     new AnimTestcaseFromBy("0.1", "none")
   ]),
   new TestcaseBundle(gPropList.lighting_color, _fromByTestLists.color),
+  new TestcaseBundle(gPropList.marker,         _fromByTestLists.URIsAndNone),
+  new TestcaseBundle(gPropList.marker_end,     _fromByTestLists.URIsAndNone),
+  new TestcaseBundle(gPropList.marker_mid,     _fromByTestLists.URIsAndNone),
+  new TestcaseBundle(gPropList.marker_start,   _fromByTestLists.URIsAndNone),
+  new TestcaseBundle(gPropList.overflow, [
+    new AnimTestcaseFromBy("inherit", "auto"),
+    new AnimTestcaseFromBy("scroll", "hidden")
+  ]),
   new TestcaseBundle(gPropList.opacity,        _fromByTestLists.opacity),
   new TestcaseBundle(gPropList.stroke_miterlimit, [
     new AnimTestcaseFromBy("1", "1", { midComp: "1.5", toComp: "2" }),

content/smil/test/db_smilCSSFromTo.js

@@ -248,7 +248,7 @@ var gFromToBundles = [
                           toComp: "normal normal 400 100px / 10px monospace"}),
     new AnimTestcaseFromTo("oblique normal 200 30px / 10px cursive",
                            "normal small-caps 800 40px / 10px serif"),
-  ], "need support for 'font' shorthand"),
+  ]),
   new TestcaseBundle(gPropList.font_family, [
     new AnimTestcaseFromTo("serif", "sans-serif"),
     new AnimTestcaseFromTo("cursive", "monospace"),
@@ -328,8 +328,7 @@ var gFromToBundles = [
   new TestcaseBundle(gPropList.lighting_color,
                      [].concat(_fromToTestLists.color,
                                _fromToTestLists.colorFromInheritWhite)),
-  new TestcaseBundle(gPropList.marker, _fromToTestLists.URIsAndNone,
+  new TestcaseBundle(gPropList.marker, _fromToTestLists.URIsAndNone),
-                     "need support for 'marker' shorthand property"),
   new TestcaseBundle(gPropList.marker_end, _fromToTestLists.URIsAndNone),
   new TestcaseBundle(gPropList.marker_mid, _fromToTestLists.URIsAndNone),
   new TestcaseBundle(gPropList.marker_start, _fromToTestLists.URIsAndNone),
@@ -337,8 +336,9 @@ var gFromToBundles = [
   new TestcaseBundle(gPropList.opacity, _fromToTestLists.opacity),
   new TestcaseBundle(gPropList.overflow, [
     new AnimTestcaseFromTo("auto", "visible"),
+    new AnimTestcaseFromTo("inherit", "visible", { fromComp: "hidden" }),
     new AnimTestcaseFromTo("scroll", "auto"),
-  ], "need support for 'overflow' shorthand"),
+  ]),
   new TestcaseBundle(gPropList.pointer_events, [
     new AnimTestcaseFromTo("visibleFill", "stroke",
                            { fromComp: "visiblefill" }),

content/smil/test/test_smilCSSFromBy.xhtml

@@ -18,6 +18,7 @@
        more predictable. (otherwise, line-height varies depending on platform)
     -->
   <text x="20" y="20" style="line-height: 10px !important">testing 123</text>
+  <line/>
   <marker/>
   <filter><feDiffuseLighting/></filter>
 </svg>

layout/reftests/svg/smil/style/reftest.list

@@ -66,8 +66,7 @@ fails == anim-css-fillopacity-3-clamp-big.svg     anim-css-fillopacity-3-ref.svg
 == anim-css-fillopacity-3-clamp-small.svg   anim-css-fillopacity-3-ref.svg
 
 # 'font' shorthand property
-# XXXdholbert We don't yet support animating shorthand properties (bug 520239)
+== anim-css-font-1.svg  anim-css-font-1-ref.svg
-fails == anim-css-font-1.svg  anim-css-font-1-ref.svg
 
 # 'font-size' property, from/by/to with pixel values only
 == anim-css-fontsize-1-from-by-px-px.svg    anim-css-fontsize-1-ref.svg