1396538 - Graph shape of some discrete animation properties in detail pane are not displayed correctly.

Status: RESOLVED FIXED

(DevTools :: Inspector: Animations, defect)

Description

[Daisuke Akatsuka (:daisuke)]

Attachment: background-repeat.png

As you can see in the attached screenshot, the graph shape of some properties that animate as discretely is not correct. The shape should be like stair.
For this test, I specified backgroundRepeat: ['round', 'space'] as keyframes.

The cause might be the computed value differ to specified keyframes value.

Comment 1

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1396538 - Part 1: Use computed value as the value scale.

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

Comment 2

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1396538 - Part 2: Add test for background-repeat.

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

Comment 3

[Daisuke Akatsuka (:daisuke)]

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

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8904200 [details]
Bug 1396538 - Part 1: Use computed value as the value scale.

https://reviewboard.mozilla.org/r/175976/#review180962

::: devtools/client/animationinspector/graph-helper.js:219
(Diff revision 1)
>     *                    from given style (e.g. center)
>     */
>    getDiscreteValueHelperFunction: function () {
>      const discreteValues = [];
>      this.keyframes.forEach(keyframe => {
> -      if (!discreteValues.includes(keyframe.value)) {
> +      // Set style once since the computed value may differ to specified keyframe value.

"may differ from specified": from instead of to.

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8904201 [details]
Bug 1396538 - Part 2: Add test for background-repeat.

https://reviewboard.mozilla.org/r/175978/#review180964

Comment 6

[Daisuke Akatsuka (:daisuke)]

Comment on attachment 8904200 [details]
Bug 1396538 - Part 1: Use computed value as the value scale.

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

Comment 7

[Daisuke Akatsuka (:daisuke)]

Comment on attachment 8904201 [details]
Bug 1396538 - Part 2: Add test for background-repeat.

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

Comment 8

[Daisuke Akatsuka (:daisuke)]

Thank you for quick review!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ad724d3f769
Part 1: Use computed value as the value scale. r=pbro
https://hg.mozilla.org/integration/autoland/rev/f21fdf4e07a9
Part 2: Add test for background-repeat. r=pbro

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/2ad724d3f769
https://hg.mozilla.org/mozilla-central/rev/f21fdf4e07a9