Closed Bug 1248532 Opened 4 years ago Closed 4 years ago

steps-start does not produce correct value at the beginning of the interval

Categories

(Core :: DOM: Animation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 --- affected
firefox48 --- fixed

People

(Reporter: birtles, Assigned: daisuke)

References

Details

Attachments

(7 files, 2 obsolete files)

For a steps(start) timing function, we should step up *at* the x=0 point but instead we produce y=0 at that point. In the attached test case, there should be blue box visible but instead nothing is visible. In Chrome the blue box is visible.

There's a comment in the code which indicates we took advantage of the fact that the spec is unclear at this point to do what was most convenient for us.[1]

[1] https://dxr.mozilla.org/mozilla-central/rev/ea39d4a6232c278dd8d805608a07cf9f4cc4c76b/dom/animation/ComputedTimingFunction.cpp#40
I notice that Edge does the same as us here.
Blocks: 1216842
I think what we implement is the traditional way to define a step function, and I think the diagrams in the spec (which have no corresponding prose) seem wrong to me.
I also prefer current our implementation, it is contrastive.
For what it is worth, this is a patch to match steps(start) to the diagrams in the spec.
No longer blocks: 1216842
Hmm, it seems inconsistent to me that for a 2-step function at x=0.5 we're at the top of the second step but at x=0 we're not at the top of the first step.

Everywhere else in the timing model we're endpoint exclusive so I don't see why this should be an exception.

We do, however, need to make sure we are at progress zero when we fill backwards (not the top of the first step).
After writing a fix for bug 1223249 (using steps(end) with reverse instead of steps(start)), I have changed my mind.  The steps(start) in our current implementation can be easily replaced, but the steps(start) on the diagrams in the spec seems not to be easily replaced (need iterationStart?).  CSS transition does not have 'reverse', of course.
 
(In reply to Brian Birtles (:birtles) from comment #5)
> Hmm, it seems inconsistent to me that for a 2-step function at x=0.5 we're
> at the top of the second step but at x=0 we're not at the top of the first
> step.

I did not notice it.
Attached file Test during delay phase (obsolete) —
Attachment #8720048 - Attachment is obsolete: true
From my testing:

During delay:
Chrome: Output = Top of first step
Firefox, Edge, Safari: Output = Bottom of first step

At t=0:
Chrome: Output = Top of first step
Firefox, Edge, Safari: Output = Bottom of first step

According to the spec, during the delay phase we should actually produce the bottom of the first step.[1] I'm surprised Chrome doesn't seem to be doing this since I think they were the ones who asked for this to be added to the spec.

[1] http://w3c.github.io/web-animations/#step-timing-function (although this spec text has a clearly buggy sentence, "when a step timing function is applied to an animation effect or applied to an animation effect associated with an animation effect")
This patch does not contain any test cases for cubic-bezier yet, but I believe it is worthwhile.
The previous patch had a change against ComputedTimingFunction.
Attachment #8720121 - Attachment is obsolete: true
Assignee: nobody → daisuke
https://reviewboard.mozilla.org/r/42625/#review39115

::: layout/style/test/file_animations_effect_timing_duration.html:52
(Diff revision 1)
> +  // Setter of timing option should set up the changes to animations for the
> +  // next layer transaction but it won't schedule a paint immediately so we need
> +  // to tick the refresh driver before we can wait on the next paint.
> +  advance_clock(0);
>  
>    yield waitForPaints();

IMO, we should call advance_clock(0) inside waitForPaints if we are under test control.
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42623/diff/1-2/
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42625/diff/1-2/
Now, the new patch failed to compile on Linux.
So, please wait to review.
You should undef True and False in ComputedTimingFunction.h
Thank you for your advice, Hiro!
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42623/diff/2-3/
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42625/diff/2-3/
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

https://reviewboard.mozilla.org/r/42623/#review39393

This looks good but I don't think it handles steps-end correctly.

::: dom/animation/ComputedTimingFunction.h:23
(Diff revision 3)
> +  // StepTimingBeforeFlag is used in step timing function.
> +  // http://w3c.github.io/web-animations/#step-timing-function
> +  enum class StepTimingBeforeFlag {
> +    No,
> +    Yes
> +  };

How about calling this just "BeforeFlag" and making the values "Set" and "Unset"?

::: dom/animation/ComputedTimingFunction.cpp:105
(Diff revision 3)
> -    // up to zero, so we can't do that.  And it's not clear the spec
> -    // really meant it.
> -    return 1.0 - StepEnd(mSteps, 1.0 - aPortion);
>    }
>    MOZ_ASSERT(mType == nsTimingFunction::Type::StepEnd, "bad type");
>    return StepEnd(mSteps, aPortion);

I think we need to use the before flag when evaluating StepEnd too such as when you have iterationStart = 0.5.

Maybe once we do that we can combine the implementation of StepStart and StepEnd somehow.
Attachment #8735172 - Flags: review?(bbirtles)
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

https://reviewboard.mozilla.org/r/42625/#review39397

This is good but we still lack the following tests:

* With iterationStart = 0.5 and steps(2, start) -- i.e. the second step lines up with the beginning
* The above but in reverse
* With iterationStart = 0.75 and steps(4, start) -- i.e. the fourth step lines up with the beginning
* With iterationStart = 0.5 and steps(2, end)
* With direction alternate and alternate-reverse
* With the step timing function applied to a keyframe
* With the step timing function applied to both the effect and a keyframe

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:446
(Diff revision 3)
> +                              { width: '100px' } ],
> +                            { delay: 1000,
> +                              duration: 1000,
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });
> +  anim.pause();

I think pausing isn't actually necessary here.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:448
(Diff revision 3)
> +                              duration: 1000,
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });
> +  anim.pause();
> +  anim.currentTime = 0;
> +  assert_equals(getComputedStyle(target).width, '0px');

I think it would be better to test the getComputedTiming().progress value instead.

(That also makes it easier to differentiate between the width being 0px due to the backwards fill as opposed to 0px because that's the specified style.)

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:449
(Diff revision 3)
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });
> +  anim.pause();
> +  anim.currentTime = 0;
> +  assert_equals(getComputedStyle(target).width, '0px');
> +  anim.currentTime = 1000;

Should we test 999 as well?

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:451
(Diff revision 3)
> +  anim.pause();
> +  anim.currentTime = 0;
> +  assert_equals(getComputedStyle(target).width, '0px');
> +  anim.currentTime = 1000;
> +  assert_equals(getComputedStyle(target).width, '50px');
> +  anim.currentTime = 1500;

Should we test 1499?

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:457
(Diff revision 3)
> +test(function(t) {
> +  var target = createDiv(t);
> +  var anim = target.animate([ { opacity: 0 },
> +                              { opacity: 1 } ],
> +                            { delay: 1000,
> +                              duration: 1000,
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });
> +  anim.pause();
> +  anim.currentTime = 0;
> +  assert_equals(getComputedStyle(target).opacity, '0');
> +  anim.currentTime = 1000;
> +  assert_equals(getComputedStyle(target).opacity, '0.5');
> +  anim.currentTime = 1500;
> +  assert_equals(getComputedStyle(target).opacity, '1');
> +  anim.currentTime = 2000;
> +  assert_equals(getComputedStyle(target).opacity, '1');
> +}, 'Test bounds point of step-start easing with compositor');

I don't think this test actually adds anything. getComputedStyle() doesn't query the compositor.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:490
(Diff revision 3)
> +  anim.pause();
> +  anim.currentTime = 0;
> +  assert_equals(getComputedStyle(target).width, '100px');
> +  anim.currentTime = 1001;
> +  assert_equals(getComputedStyle(target).width, '100px');
> +  anim.currentTime = 1501;

Add a test for 1500?

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:492
(Diff revision 3)
> +  assert_equals(getComputedStyle(target).width, '100px');
> +  anim.currentTime = 1001;
> +  assert_equals(getComputedStyle(target).width, '100px');
> +  anim.currentTime = 1501;
> +  assert_equals(getComputedStyle(target).width, '50px');
> +  anim.currentTime = 2000;

Add a test for 2500 (just in case an implementation special-cases the boundary condition)

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:495
(Diff revision 3)
> +
> +

Nit: We only need one blank line here.
Attachment #8735173 - Flags: review?(bbirtles)
Review commit: https://reviewboard.mozilla.org/r/43181/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43181/
Attachment #8735173 - Attachment description: MozReview Request: Bug 1248532 - Part 2: Add tests r?birtles → MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r?birtles
Attachment #8736235 - Flags: review?(bbirtles)
Attachment #8735172 - Flags: review?(bbirtles)
Attachment #8735173 - Flags: review?(bbirtles)
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42623/diff/3-4/
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42625/diff/3-4/
Now, try server is closing. I'll push to try server later.
(In reply to Brian Birtles (:birtles) from comment #26)
> Comment on attachment 8735173 [details]
> MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r?birtles
> 
> https://reviewboard.mozilla.org/r/42625/#review39397
> 
> This is good but we still lack the following tests:
> 
> * With iterationStart = 0.5 and steps(2, end)

I tested this condition.
It looks work well without any modifying for step-end function.
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

https://reviewboard.mozilla.org/r/42623/#review39973

As discussed, in the test case added in part 3, the progress should be zero during the delay. So I think we need to update StepEnd. Cancelling this review for the time being.

::: dom/animation/ComputedTimingFunction.h:19
(Diff revision 4)
>  
>  class ComputedTimingFunction
>  {
>  public:
> +  // BeforeFlag is used in step timing function.
> +  // http://w3c.github.io/web-animations/#step-timing-function

Nit: https

Also, I think we can link to:
https://w3c.github.io/web-animations/#before-flag
Attachment #8735172 - Flags: review?(bbirtles)
Attachment #8735173 - Flags: review?(bbirtles) → review+
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

https://reviewboard.mozilla.org/r/42625/#review39977

This looks good. r=me with comments addressed, particularly extracting out extra functions to make these tests easier to read.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:514
(Diff revisions 3 - 4)
> +  anim.currentTime = 0;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 999;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 1000;
> +  assert_equals(anim.effect.getComputedTiming().progress, 1);
> +  anim.currentTime = 1499;
> +  assert_equals(anim.effect.getComputedTiming().progress, 1);
> +  anim.currentTime = 1500;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 2000;

We have this pattern a lot in this file. Can we extract a function that takes an array like:

  [ 0,    0.5 ],
  [ 999,  0.5 ],
  [ 1000, 1   ],
  [ 1499, 1   ],
  ...
  
And then steps through the array and calls currentTime and assert_equals?

When we call assert_equals we can add a message like:

  'Progress at ' + time + 'ms'

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:561
(Diff revisions 3 - 4)
> +
> +test(function(t) {
> +  var target = createDiv(t);
> +  var anim = target.animate([ { width: '0px' },
> +                              { width: '100px' } ],
> +                            { duration: 1000,

Oh, I think what I wanted to test here was the delay phase. i.e. it should be 0.75 during the delay.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:570
(Diff revisions 3 - 4)
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.25);
> +  anim.currentTime = 499;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.25);
> +  anim.currentTime = 500;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 749;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 750;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.75);
> +  anim.currentTime = 999;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.75);
> +  anim.currentTime = 1000;
> +  assert_equals(anim.effect.getComputedTiming().progress, 1);

We probably don't need this part.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:591
(Diff revisions 3 - 4)
> +                            { duration: 1000,
> +                              iterations: 2,
> +                              direction: 'alternate',
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });

Sorry, I wasn't clear. I think what I want to test is something like:

  delay: 1000,
  iterations: 2,
  iterationStart: 1.5,
  direction: 'alternate',
  fill: 'both',
  easing: 'steps(2, start)'
  
Then just check that we do the right thing during the backwards and forwards fill.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:618
(Diff revisions 3 - 4)
> +                            { duration: 1000,
> +                              iterations: 2,
> +                              direction: 'alternate-reverse',
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });

Here too, I think I was thinking more about:

  { delay: 1000,
    duration: 1000,
    iterations: 2,
    iterationStart: 0.5,
    direction: 'alternate-reverse',
    fill: 'both',
    easing: 'steps(2, start)' }

And checking we do the right thing in the forwards and backwards fill.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:650
(Diff revisions 3 - 4)
> +  assert_equals(getComputedStyle(target).width, '0px');
> +  assert_equals(anim.effect.getComputedTiming().progress, 0);
> +  anim.currentTime = 999;
> +  assert_equals(getComputedStyle(target).width, '0px');
> +  assert_equals(anim.effect.getComputedTiming().progress, 0);
> +  anim.currentTime = 1000;
> +  assert_equals(getComputedStyle(target).width, '50px');
> +  assert_equals(anim.effect.getComputedTiming().progress, 0);
> +  anim.currentTime = 1499;
> +  assert_equals(getComputedStyle(target).width, '50px');
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.499);
> +  anim.currentTime = 1500;
> +  assert_equals(getComputedStyle(target).width, '100px');
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 2000;
> +  assert_equals(getComputedStyle(target).width, '100px');
> +  assert_equals(anim.effect.getComputedTiming().progress, 1);
> +  anim.currentTime = 2500;

This is good, but I guess we can just extract another function that takes an array with:

  [ 0,    '0px'  ],
  [ 999,  '0px'  ],
  [ 1000, '50px' ],
  ...
  
?

(I'm not sure we need to test the progress in this case)

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:674
(Diff revisions 3 - 4)
> +  var anim = target.animate([ { width: '0px', easing: 'linear' },
> +                              { width: '100px'} ],
> +                            { delay: 1000,
> +                              duration: 1000,
> +                              fill: 'both',
> +                              easing: 'steps(2, start)' });

I'm not sure using easing: 'linear' really helps here (it's the default). Let's just drop this test.
Comment on attachment 8736235 [details]
MozReview Request: Bug 1248532 - Part 3: add a test for step-end with iterationStart. r=birtles

https://reviewboard.mozilla.org/r/43181/#review39981

r=me with the following change made and the test converted to use whatever convenience function we create in part 2.

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:712
(Diff revision 1)
> +  anim.currentTime = 0;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);
> +  anim.currentTime = 999;
> +  assert_equals(anim.effect.getComputedTiming().progress, 0.5);

As discussed, I think progress should be 0 here.
Attachment #8736235 - Flags: review?(bbirtles)
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42623/diff/4-5/
Attachment #8735173 - Attachment description: MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r?birtles → MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles
Attachment #8736235 - Attachment description: MozReview Request: Bug 1248532 - Part 3: add a test for step-end with iterationStart. r?birtles → MozReview Request: Bug 1248532 - Part 3: add a test for step-end with iterationStart. r=birtles
Attachment #8735172 - Flags: review?(bbirtles)
Attachment #8736235 - Flags: review?(bbirtles)
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42625/diff/4-5/
Comment on attachment 8736235 [details]
MozReview Request: Bug 1248532 - Part 3: add a test for step-end with iterationStart. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43181/diff/1-2/
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

https://reviewboard.mozilla.org/r/42623/#review40203

r=me with the simplifications to the StepTiming function I proposed (assuming they work--or something similar to that)

::: dom/animation/ComputedTimingFunction.cpp:36
(Diff revision 5)
> +  if (aPortion == 1.0) {
> +    return 1.0;
> +  }
> +
> +  bool isBeforeTransaction =
> +    aBeforeFlag == ComputedTimingFunction::BeforeFlag::Set &&
> +    (aPortion == 0 || fmod(aSteps * aPortion, 1) == 0); // is transition point ?
> +
> +  uint32_t step = uint32_t(aPortion * aSteps) +
> +    (aType == nsTimingFunction::Type::StepStart ? 1 : 0);
> +
> +  if (isBeforeTransaction && step > 0) {
> +    return double(step - 1) / double(aSteps);
> +  }
>    return double(step) / double(aSteps);

I wonder if we can make this whole function simpler and easier to read and debug by calculating 'step' first like so:

  // Calculate current step using step-end behavior
  uint32_t step = uint32_t(aPortion * aSteps); // floor

  // step-start is one step ahead
  if (aType == nsTimingFunction::Type::StepStart) {
    step++;
  }

  // If the "before flag" is set and we are at a transition point,
  // drop back a step (but only if we are not already at the zero point--
  // we do this clamping here since |step| is an unsigned integer)
  if (step != 0 &&
      aBeforeFlag == ComputedTimingFunction::BeforeFlag::Set &&
      fmod(aPortion * aSteps, 1) == 0) {
    step--;
  }

  // Don't step up or down if we are already at the limit
  step = std::min(step, 1);

  // Convert to a progress value
  return double(step) / double(aSteps);


Does that work?

::: dom/animation/ComputedTimingFunction.cpp:40
(Diff revision 5)
> +
> +  if (aPortion == 1.0) {
> +    return 1.0;
> +  }
> +
> +  bool isBeforeTransaction =

isBeforeStep?

::: dom/animation/ComputedTimingFunction.cpp:42
(Diff revision 5)
> +    return 1.0;
> +  }
> +
> +  bool isBeforeTransaction =
> +    aBeforeFlag == ComputedTimingFunction::BeforeFlag::Set &&
> +    (aPortion == 0 || fmod(aSteps * aPortion, 1) == 0); // is transition point ?

I think fmod(0, 1) == 0?

So we could just drop the aPortion == 0 test?

Also, a very minor nit, but later we have (aPortion * aSteps) and here we have (aSteps * aPortion). This could we be a little easier to read if we used the same order in both places, i.e.

  bool isBeforeStep =
    aBeforeFlag == ComputedTiming::BeforeFlag::Set &&
    fmod(aPortion * aSteps) == 0;

::: dom/animation/ComputedTimingFunction.cpp:55
(Diff revision 5)
>    return double(step) / double(aSteps);
>  }
>  
>  double
> -ComputedTimingFunction::GetValue(double aPortion) const
> +ComputedTimingFunction::GetValue(double aPortion,
> +  ComputedTimingFunction::BeforeFlag aBeforeFlag) const

Nit: If we are going to split the parameters over multiple lines, we should line them up.

So normally you'd write:

  double
  ComputedTimingFunction::GetValue(double aPortion,
                                  ComputedTimingFunction::BeforeFlag aBeforeFlag) const

But since that doesn't fit within 80 chars, in this case you'd do:

  double
  ComputedTimingFunction::GetValue(
      double aPortion,
      ComputedTimingFunction::BeforeFlag aBeforeFlag) const

(i.e. recently I notice a lot of people use a 4-space indent for the parameters in this case)

::: dom/animation/KeyframeEffect.cpp:397
(Diff revision 5)
> +    result.mBeforeFlag = ComputedTimingFunction::BeforeFlag::Set;
> +  }
> +
>    if (aTiming.mFunction) {
>      result.mProgress.SetValue(
> -      aTiming.mFunction->GetValue(result.mProgress.Value()));
> +      aTiming.mFunction->GetValue(result.mProgress.Value(), result.mBeforeFlag));

Nit: I think this might be just over 80 characters?
Attachment #8735172 - Flags: review?(bbirtles) → review+
Comment on attachment 8736235 [details]
MozReview Request: Bug 1248532 - Part 3: add a test for step-end with iterationStart. r=birtles

https://reviewboard.mozilla.org/r/43181/#review40219

Looks great. Thanks!

::: testing/web-platform/tests/web-animations/keyframe-effect/effect-easing.html:676
(Diff revision 2)
> +                  { currentTime: 2500, progress: 0.5 }
> +                ]
> +  },
> +  {
> +    description: 'Test bounds point of step-end easing ' +
> +                 'with iterationStart of not transition point and delay',

'with iterationStart not at a transition point'

(I don't think you need to mention the delay--it's just there to help us test the initial transition point)
Attachment #8736235 - Flags: review?(bbirtles) → review+
Comment on attachment 8735172 [details]
MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42623/diff/5-6/
Attachment #8735172 - Attachment description: MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r?birtles → MozReview Request: Bug 1248532 - Part 1: steps-start does not produce correct value at the beginning of the interval. r=birtles
Comment on attachment 8735173 [details]
MozReview Request: Bug 1248532 - Part 2: Add tests for step-start. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42625/diff/5-6/
Comment on attachment 8736235 [details]
MozReview Request: Bug 1248532 - Part 3: add a test for step-end with iterationStart. r=birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43181/diff/2-3/
Keywords: checkin-needed
Duplicate of this bug: 1201769
Depends on: 1270031
Depends on: 1275475
You need to log in before you can comment on or make changes to this bug.