1259313 - Need a test for checking result of ParseEasing in GetKeyframeListFromPropertyIndexedKeyframe

Status: RESOLVED DUPLICATE of bug 1244591

(Core :: DOM: Animation, defect)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Bug 1253470 replaces a call to:

  AnimationUtils::ParseEasing(keyframes.mEasing, aTarget->OwnerDoc());

with a call to:

  TimingParams::ParseEasing(keyframes.mEasing, aTarget->OwnerDoc(), aRv);

here:

  https://hg.mozilla.org/mozilla-central/rev/a0c0ea60eaef#l2.39

But doesn't check the result, aRv. It probably should and we should probably add a test for that.

Comment 1

[Daisuke Akatsuka (:daisuke)]

Oh, thanks!

Comment 2

[Brian Birtles (:birtles, Feb 26~ parental leave)]

The code changes should now be fixed as part of bug 1245748 but we still need a test.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Now the ParseEasing is here:
https://dxr.mozilla.org/mozilla-central/rev/d62963756d9a9d19cbbb5d8f3dd3c7cfa8fdef88/dom/animation/KeyframeUtils.cpp#1005

Comment 4

[Daisuke Akatsuka (:daisuke)]

Ok, thanks!

Comment 5

[Daisuke Akatsuka (:daisuke)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c667879f1737

Comment 6

[Daisuke Akatsuka (:daisuke)]

Attachment: MozReview Request: Bug 1259313 - Need a test for checking result of ParseEasing in GetKeyframeListFromPropertyIndexedKeyframe. r?birtles

Review commit: https://reviewboard.mozilla.org/r/54452/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54452/

Comment 7

[Brian Birtles (:birtles, Feb 26~ parental leave)]

It looks like this was already added in bug 1244591. Specifically, here:

  https://hg.mozilla.org/mozilla-central/rev/d2dc79c8d77c#l4.27

Of the tests added in attachment 8755242 [details], however, I think we might still want to add at least the first one. (We might not want to second one due to upcoming changes in Web Animations.[1])

[1] https://github.com/w3c/web-animations/issues/148

Comment 8

[Daisuke Akatsuka (:daisuke)]

Thank, Brian!
Ok!

Comment 9

[Daisuke Akatsuka (:daisuke)]

Comment on attachment 8755242 [details]
MozReview Request: Bug 1259313 - Need a test for checking result of ParseEasing in GetKeyframeListFromPropertyIndexedKeyframe. r?birtles

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54452/diff/1-2/

Comment 10

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Sorry, I just checked and it turns out we have tests for this too!

  https://dxr.mozilla.org/mozilla-central/rev/16663eb3dcfa759f25b5e27b101bc79270c156f2/testing/web-platform/tests/web-animations/interfaces/KeyframeEffect/constructor.html#30

So I think we can mark this resolved duplicate of bug 1244591.