Closed Bug 978948 Opened 7 years ago Closed 6 years ago
Timeout as animation generators in the Canvas Debugger
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 :)
(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.
Shelving a very wip patch.
Component: Developer Tools → Developer Tools: Canvas Debugger
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*"
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.
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 shimthat 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  http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/
Attachment #8567210 - Flags: review?(vporof)
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
You need to log in before you can comment on or make changes to this bug.