Bug 1398038 - Implement extended property-indexed keyframe syntax; r?hiro, r?bz draft
authorBrian Birtles <birtles@gmail.com>
Wed, 18 Oct 2017 16:12:21 +0900
changeset 683765 b50524555e5b095287d52c0b8e87e14f4813554d
parent 683764 8c27b68144c9e1e65ce9cace87eddabe3d7f540d
child 736720 ff642fdc30f0aa357383fdcf9be4c6cbe2427a22
push id85456
push userbmo:bbirtles@mozilla.com
push dateFri, 20 Oct 2017 06:31:55 +0000
reviewershiro, bz
bugs1398038
milestone58.0a1
Bug 1398038 - Implement extended property-indexed keyframe syntax; r?hiro, r?bz This implements the changes specified in these three spec changesets: https://github.com/w3c/web-animations/commit/8efd180bd95af477b0a6a1939282a79fbf1d7bc2 https://github.com/w3c/web-animations/commit/f43ecdfbe5f35d243fcde01b372bf7de01a15110 https://github.com/w3c/web-animations/commit/a4f1ad1a60c0d68035fc3922b96922eea69ef6c8 MozReview-Commit-ID: KFhgZ5ip6BA
dom/animation/KeyframeUtils.cpp
dom/webidl/BaseKeyframeTypes.webidl
testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini
testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini
testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini
--- a/dom/animation/KeyframeUtils.cpp
+++ b/dom/animation/KeyframeUtils.cpp
@@ -22,17 +22,17 @@
 #include "nsContentUtils.h" // For GetContextForContent
 #include "nsCSSParser.h"
 #include "nsCSSPropertyIDSet.h"
 #include "nsCSSProps.h"
 #include "nsCSSPseudoElements.h" // For CSSPseudoElementType
 #include "nsIScriptError.h"
 #include "nsStyleContext.h"
 #include "nsTArray.h"
-#include <algorithm> // For std::stable_sort
+#include <algorithm> // For std::stable_sort, std::min
 
 namespace mozilla {
 
 // ------------------------------------------------------------------
 //
 // Internal data types
 //
 // ------------------------------------------------------------------
@@ -1387,40 +1387,26 @@ GetKeyframeListFromPropertyIndexedKeyfra
   // get its explicit dictionary members.
   dom::binding_detail::FastBasePropertyIndexedKeyframe keyframeDict;
   if (!keyframeDict.Init(aCx, aValue, "BasePropertyIndexedKeyframe argument",
                          false)) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
-  Maybe<dom::CompositeOperation> composite;
-  if (keyframeDict.mComposite.WasPassed()) {
-    composite.emplace(keyframeDict.mComposite.Value());
-  }
-
   // Get all the property--value-list pairs off the object.
   JS::Rooted<JSObject*> object(aCx, &aValue.toObject());
   nsTArray<PropertyValuesPair> propertyValuesPairs;
   if (!GetPropertyValuesPairs(aCx, object, ListAllowance::eAllow,
                               aDocument->GetStyleBackendType(),
                               propertyValuesPairs)) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
-  // Parse the easing property. We need to do this after reading off the
-  // property values since this is conceptually a separate step that happens
-  // after the WebIDL-like processing.
-  Maybe<ComputedTimingFunction> easing =
-    TimingParams::ParseEasing(keyframeDict.mEasing, aDocument, aRv);
-  if (aRv.Failed()) {
-    return;
-  }
-
   // Create a set of keyframes for each property.
   nsCSSParser parser(aDocument->CSSLoader());
   nsClassHashtable<nsFloatHashKey, Keyframe> processedKeyframes;
   for (const PropertyValuesPair& pair : propertyValuesPairs) {
     size_t count = pair.mValues.Length();
     if (count == 0) {
       // No animation values for this property.
       continue;
@@ -1439,18 +1425,16 @@ GetKeyframeListFromPropertyIndexedKeyfra
     size_t i = 0;
 
     for (const nsString& stringValue : pair.mValues) {
       // For single-valued lists, the single value should be added to a
       // keyframe with offset 1.
       double offset = n ? i++ / double(n) : 1;
       Keyframe* keyframe = processedKeyframes.LookupOrAdd(offset);
       if (keyframe->mPropertyValues.IsEmpty()) {
-        keyframe->mTimingFunction = easing;
-        keyframe->mComposite = composite;
         keyframe->mComputedOffset = offset;
       }
 
       Maybe<PropertyValuePair> valuePair =
         MakePropertyValuePair(pair.mProperty, stringValue, parser, aDocument);
       if (!valuePair) {
         continue;
       }
@@ -1459,16 +1443,134 @@ GetKeyframeListFromPropertyIndexedKeyfra
   }
 
   aResult.SetCapacity(processedKeyframes.Count());
   for (auto iter = processedKeyframes.Iter(); !iter.Done(); iter.Next()) {
     aResult.AppendElement(Move(*iter.UserData()));
   }
 
   aResult.Sort(ComputedOffsetComparator());
+
+  // Fill in any specified offsets
+  //
+  // This corresponds to step 5, "Otherwise," branch, substeps 5-6 of
+  // https://w3c.github.io/web-animations/#processing-a-keyframes-argument
+  const FallibleTArray<Nullable<double>>* offsets = nullptr;
+  AutoTArray<Nullable<double>, 1> singleOffset;
+  auto& offset = keyframeDict.mOffset;
+  if (offset.IsDouble()) {
+    singleOffset.AppendElement(offset.GetAsDouble());
+    // dom::Sequence is a fallible but AutoTArray is infallible and we need to
+    // point to one or the other. Fortunately, fallible and infallible array
+    // types can be implicitly converted provided they are const.
+    const FallibleTArray<Nullable<double>>& asFallibleArray = singleOffset;
+    offsets = &asFallibleArray;
+  } else if (offset.IsDoubleOrNullSequence()) {
+    offsets = &offset.GetAsDoubleOrNullSequence();
+  }
+  // If offset.IsNull() is true, then we want to leave the mOffset member of
+  // each keyframe with its initialized value of null. By leaving |offsets|
+  // as nullptr here, we skip updating mOffset below.
+
+  size_t offsetsToFill =
+    offsets ? std::min(offsets->Length(), aResult.Length()) : 0;
+  for (size_t i = 0; i < offsetsToFill; i++) {
+    if (!offsets->ElementAt(i).IsNull()) {
+      aResult[i].mOffset.emplace(offsets->ElementAt(i).Value());
+    }
+  }
+
+  // Check that the keyframes are loosely sorted and that any specified offsets
+  // are between 0.0 and 1.0 inclusive.
+  //
+  // This corresponds to steps 6-7 of
+  // https://w3c.github.io/web-animations/#processing-a-keyframes-argument
+  //
+  // In the spec, TypeErrors arising from invalid offsets and easings are thrown
+  // at the end of the procedure since it assumes we initially store easing
+  // values as strings and then later parse them.
+  //
+  // However, we will parse easing members immediately when we process them
+  // below. In order to maintain the relative order in which TypeErrors are
+  // thrown according to the spec, namely exceptions arising from invalid
+  // offsets are thrown before exceptions arising from invalid easings, we check
+  // the offsets here.
+  if (!HasValidOffsets(aResult)) {
+    aRv.ThrowTypeError<dom::MSG_INVALID_KEYFRAME_OFFSETS>();
+    aResult.Clear();
+    return;
+  }
+
+  // Fill in any easings.
+  //
+  // This corresponds to step 5, "Otherwise," branch, substeps 7-11 of
+  // https://w3c.github.io/web-animations/#processing-a-keyframes-argument
+  FallibleTArray<Maybe<ComputedTimingFunction>> easings;
+  auto parseAndAppendEasing = [&](const nsString& easingString,
+                                  ErrorResult& aRv) {
+    auto easing = TimingParams::ParseEasing(easingString, aDocument, aRv);
+    if (!aRv.Failed() && !easings.AppendElement(Move(easing), fallible)) {
+      aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
+    }
+  };
+
+  auto& easing = keyframeDict.mEasing;
+  if (easing.IsString()) {
+    parseAndAppendEasing(easing.GetAsString(), aRv);
+    if (aRv.Failed()) {
+      aResult.Clear();
+      return;
+    }
+  } else {
+    for (const nsString& easingString : easing.GetAsStringSequence()) {
+      parseAndAppendEasing(easingString, aRv);
+      if (aRv.Failed()) {
+        aResult.Clear();
+        return;
+      }
+    }
+  }
+
+  // If |easings| is empty, then we are supposed to fill it in with the value
+  // "linear" and then repeat the list as necessary.
+  //
+  // However, for Keyframe.mTimingFunction we represent "linear" as a None
+  // value. Since we have not assigned 'mTimingFunction' for any of the
+  // keyframes in |aResult| they will already have their initial None value
+  // (i.e. linear). As a result, if |easings| is empty, we don't need to do
+  // anything.
+  if (!easings.IsEmpty()) {
+    for (size_t i = 0; i < aResult.Length(); i++) {
+      aResult[i].mTimingFunction = easings[i % easings.Length()];
+    }
+  }
+
+  // Fill in any composite operations.
+  //
+  // This corresponds to step 5, "Otherwise," branch, substep 12 of
+  // https://w3c.github.io/web-animations/#processing-a-keyframes-argument
+  const FallibleTArray<dom::CompositeOperation>* compositeOps;
+  AutoTArray<dom::CompositeOperation, 1> singleCompositeOp;
+  auto& composite = keyframeDict.mComposite;
+  if (composite.IsCompositeOperation()) {
+    singleCompositeOp.AppendElement(composite.GetAsCompositeOperation());
+    const FallibleTArray<dom::CompositeOperation>& asFallibleArray =
+      singleCompositeOp;
+    compositeOps = &asFallibleArray;
+  } else {
+    compositeOps = &composite.GetAsCompositeOperationSequence();
+  }
+
+  // Fill in and repeat as needed.
+  if (!compositeOps->IsEmpty()) {
+    for (size_t i = 0; i < aResult.Length(); i++) {
+      aResult[i].mComposite.emplace(
+        compositeOps->ElementAt(i % compositeOps->Length()));
+    }
+  }
 }
 
 /**
  * Returns true if the supplied set of keyframes has keyframe values for
  * any property for which it does not also supply a value for the 0% and 100%
  * offsets. In this case we are supposed to synthesize an additive zero value
  * but since we don't support additive animation yet we can't support this
  * case. We try to detect that here so we can throw an exception. The check is
--- a/dom/webidl/BaseKeyframeTypes.webidl
+++ b/dom/webidl/BaseKeyframeTypes.webidl
@@ -15,18 +15,19 @@
 
 enum CompositeOperation { "replace", "add", "accumulate" };
 
 // The following dictionary types are not referred to by other .webidl files,
 // but we use it for manual JS->IDL and IDL->JS conversions in
 // KeyframeEffectReadOnly's implementation.
 
 dictionary BasePropertyIndexedKeyframe {
-  DOMString easing = "linear";
-  CompositeOperation composite;
+  (double? or sequence<double?>) offset = [];
+  (DOMString or sequence<DOMString>) easing = [];
+  (CompositeOperation or sequence<CompositeOperation>) composite = [];
 };
 
 dictionary BaseKeyframe {
   double? offset = null;
   DOMString easing = "linear";
   CompositeOperation composite;
 
   // Non-standard extensions
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/interfaces/Animatable/animate.html.ini
+++ /dev/null
@@ -1,40 +0,0 @@
-[animate.html]
-  type: testharness
-  [Element.animate() accepts a property-indexed keyframe with a single offset]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets that is too short]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets that is too long]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets with an embedded null value]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets with a trailing null value]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets with leading and trailing null values]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets with adjacent null values]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of easings]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of easings that is too short]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an array of easings that is too long]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with an empty array of easings]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with a composite array]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with a composite array that is too short]
-    expected: FAIL
-  [Element.animate() accepts a property-indexed keyframe with a composite array that is too long]
-    expected: FAIL
-  [Element.animate() does not accept property-indexed keyframes not loosely sorted by offset]
-    expected: FAIL
-  [Element.animate() does not accept property-indexed keyframes not loosely sorted by offset even though not all offsets are specified]
-    expected: FAIL
-  [Element.animate() does not accept property-indexed keyframes with offsets out of range]
-    expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/constructor.html.ini
+++ /dev/null
@@ -1,54 +0,0 @@
-[constructor.html]
-  type: testharness
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with a single offset]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is too short]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is too long]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with an embedded null value]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with a trailing null value]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with leading and trailing null values]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets with adjacent null values]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of easings]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of easings roundtrips]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of easings that is too short]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of easings that is too short roundtrips]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an array of easings that is too long]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with an array of easings that is too long roundtrips]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with an empty array of easings]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with an empty array of easings roundtrips]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with a composite array]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with a composite array roundtrips]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with a composite array that is too short]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with a composite array that is too short roundtrips]
-    expected: FAIL
-  [a KeyframeEffectReadOnly can be constructed with a property-indexed keyframe with a composite array that is too long]
-    expected: FAIL
-  [a KeyframeEffectReadOnly constructed with a property-indexed keyframe with a composite array that is too long roundtrips]
-    expected: FAIL
-  [KeyframeEffectReadOnly constructor throws with property-indexed keyframes not loosely sorted by offset]
-    expected: FAIL
-  [KeyframeEffectReadOnly constructor throws with property-indexed keyframes not loosely sorted by offset even though not all offsets are specified]
-    expected: FAIL
-  [KeyframeEffectReadOnly constructor throws with property-indexed keyframes with offsets out of range]
-    expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/setKeyframes.html.ini
+++ /dev/null
@@ -1,40 +0,0 @@
-[setKeyframes.html]
-  type: testharness
-  [Keyframes can be replaced with a property-indexed keyframe with a single offset]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is too short]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is too long]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets with an embedded null value]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets with a trailing null value]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets with leading and trailing null values]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets with adjacent null values]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of offsets that is not strictly ascending in the unused part of the array]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of easings]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of easings that is too short]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an array of easings that is too long]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with an empty array of easings]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with a composite array]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with a composite array that is too short]
-    expected: FAIL
-  [Keyframes can be replaced with a property-indexed keyframe with a composite array that is too long]
-    expected: FAIL
-  [KeyframeEffect constructor throws with property-indexed keyframes not loosely sorted by offset]
-    expected: FAIL
-  [KeyframeEffect constructor throws with property-indexed keyframes not loosely sorted by offset even though not all offsets are specified]
-    expected: FAIL
-  [KeyframeEffect constructor throws with property-indexed keyframes with offsets out of range]
-    expected: FAIL