Support setTimeout as animation generators in the Canvas Debugger

RESOLVED FIXED in Firefox 39

Status

RESOLVED FIXED
5 years ago
3 days ago

People

(Reporter: vporof, Assigned: jsantell)

Tracking

unspecified
Firefox 39
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
We should do something about these ancient ways of animating things. At the very least, displaying a huge red flashing banner PLEASE DON'T USE SETINTERVAL FOR ANIMATION should do the trick :)
(Reporter)

Updated

5 years ago
Depends on: 917226
(In reply to Victor Porof [:vporof][:vp] from comment #0)
> We should do something about these ancient ways of animating things. At the
> very least, displaying a huge red flashing banner PLEASE DON'T USE
> SETINTERVAL FOR ANIMATION should do the trick :)

doesn't have to be flashing. A red notification would work though.
(Reporter)

Comment 2

5 years ago
Posted patch wip (obsolete) — Splinter Review
Shelving a very wip patch.
(Reporter)

Updated

5 years ago
Component: Developer Tools → Developer Tools: Canvas Debugger
See Also: → bug 985488
(Reporter)

Updated

5 years ago
Duplicate of this bug: 1040883
(Reporter)

Updated

5 years ago
Attachment #8384861 - Attachment is obsolete: true
Do we have any intention of supporting this, especially once we make clear that it's recording a rAF frame (bug 985488)? Can also provide an even more explicit explanation of "Don't use setTimeout/setInterval *link*"
Flags: needinfo?(vporof)
(Reporter)

Comment 5

4 years ago
I'd like us to support it. There's a lot of demos on the internet that *STILL* use setInterval. However, we should make it absolutely clear that it's a Bad Thing and the developers should feel Bad.
Flags: needinfo?(vporof)
Gonna take a look at this.
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Summary: Support setInterval and setTimeout as animation generators in the Canvas Debugger → Support setTimeout as animation generators in the Canvas Debugger
As per discussion in IRC, changing this to only support setTimeout at this time, as it'll be more common than setInterval due to the popular rAF shim[0]that uses it, and similarities it shares to rAF in function signature.

This'll need to be rebased hard with the other canvas debugger patches. A few things are built out, as if the rAF/setTimeout paths are different, but those can be merged all into one ANIMATION_GENERATOR array, as their implementations are identical, just a matter of taste splitting them up in the actor.

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

[0] http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/
Attachment #8567210 - Flags: review?(vporof)
(Reporter)

Comment 8

4 years ago
Comment on attachment 8567210 [details] [diff] [review]
978948-canvas-settimeout.patch

Review of attachment 8567210 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/server/actors/canvas.js
@@ +335,5 @@
>        return;
>      }
> +    // Handle animations generated using setTimeout. While using
> +    // those timers is considered extremely poor practice, they're still widely
> +    // used on the web, especially for old demos; it's nice to support them as well.

Update the comment for recordAnimationFrame too
Attachment #8567210 - Flags: review?(vporof) → review+
Updated. Waiting for bug 985488 to land for rebasing
Attachment #8567210 - Attachment is obsolete: true
Attachment #8567530 - Flags: review+
Blocks: 1135403
Depends on: 985488
No longer depends on: 985488
Blocks: 985488
Rebased
Attachment #8567530 - Attachment is obsolete: true
Attachment #8567688 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/c2c0f42ecb00
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c2c0f42ecb00
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 39

Updated

9 months ago
Product: Firefox → DevTools

Updated

3 days ago
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.