Closed Bug 1465077 Opened 2 years ago Closed 2 years ago

Negative elapsed time and scrubber position are displayed, if an animation is replayed

Categories

(DevTools :: Inspector: Animations, defect, P3)

defect

Tracking

(firefox-esr52 unaffected, firefox-esr60 unaffected, firefox60 unaffected, firefox61 disabled, firefox62 verified)

VERIFIED FIXED
Firefox 62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- disabled
firefox62 --- verified

People

(Reporter: gpalko, Assigned: daisuke)

References

(Blocks 2 open bugs)

Details

(Keywords: regression)

Attachments

(3 files)

Attached image resumed_seeker.PNG
[Environments:]
Windows 10, Windows 7, Ubuntu 16.04, OsX10.12
62.0a1 20180528220216

[Steps:]
1. Open Firefox and open a new tab with https://rawgit.com/dadaa/3b73f847427025b51ba1ab7333013d0c/raw/77f3f0bb884875a179c3407f73bf8a8dd54751c9/doc_simple_animation.html
2. Press F12.
3. Select Inspector and Animations panel.
4. Select "ball delayed" node in Inspector
5. Switch to 10x playback rate
6. Wait for the animation to be finished
7. On Animations panel click the Play/Resume button

[Actual Result:]
The animation is replayed, negative elapsed time is displayed, the scrubber starts according to the negative time(see attached screenshot)

[Expected Result:]
6. The animation should run again from 0.0s
The issue is not reproducing in the old animation inspector.
Keywords: regression
Assignee: nobody → dakatsuka
Blocks: 1399830
Priority: -- → P3
Comment on attachment 8981803 [details]
Bug 1465077 - Part 1: Introduce doSetCurrentTimes so as to not forget the createdTime.

https://reviewboard.mozilla.org/r/247856/#review254402

::: devtools/client/inspector/animation/animation.js:196
(Diff revision 1)
> +   *
> +   * @param {Number} currentTime
> +   * @param {Array} animations
> +   * @param {Object} timeScale
> +   */
> +  async callSetCurrentTimes(currentTime, animations, timeScale) {

`animations` and `timeScale` can be retrieved from `this.state`, right? So I think there is no need to pass them as arguments here.

Also, I'm not sure about the method name you've chosen here. What about `doSetCurrentTime` instead?
It's like, it's the low-level call while `setCurrentTime` is higher level API.
Attachment #8981803 - Flags: review?(pbrosset)
Comment on attachment 8981804 [details]
Bug 1465077 - Part 2: Add a test which checks whether the scrubber never be located at negative position.

https://reviewboard.mozilla.org/r/247858/#review254424
Attachment #8981804 - Flags: review?(pbrosset) → review+
Comment on attachment 8981803 [details]
Bug 1465077 - Part 1: Introduce doSetCurrentTimes so as to not forget the createdTime.

https://reviewboard.mozilla.org/r/247856/#review254402

> `animations` and `timeScale` can be retrieved from `this.state`, right? So I think there is no need to pass them as arguments here.
> 
> Also, I'm not sure about the method name you've chosen here. What about `doSetCurrentTime` instead?
> It's like, it's the low-level call while `setCurrentTime` is higher level API.

Thanks!

I thought that this function might be called with specific animations. Due to this reason, I made the animations to be the parameter. However, as you said, because there is no need actually in this case, retrieves from `this.state`.
Comment on attachment 8981803 [details]
Bug 1465077 - Part 1: Introduce doSetCurrentTimes so as to not forget the createdTime.

https://reviewboard.mozilla.org/r/247856/#review255056
Attachment #8981803 - Flags: review?(pbrosset) → review+
Pushed by dakatsuka@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f3de98e58e52
Part 1: Introduce doSetCurrentTimes so as to not forget the createdTime. r=pbro
https://hg.mozilla.org/integration/autoland/rev/8533951dca06
Part 2: Add a test which checks whether the scrubber never be located at negative position. r=pbro
https://hg.mozilla.org/mozilla-central/rev/f3de98e58e52
https://hg.mozilla.org/mozilla-central/rev/8533951dca06
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Verified as fixed on Nightly 62.0a1(20180611100116) on Win10 x64, Ubuntu 12.04 and MacOS 10.12
Animations are reloaded from 0.0s.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.