Add unprefixed cancelAnimationFrame

RESOLVED FIXED in Firefox 23

Status

()

Core
Layout
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evilpie, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug, {dev-doc-complete, regression})

Trunk
mozilla24
dev-doc-complete, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22 unaffected, firefox23+ fixed, firefox24+ fixed)

Details

(Whiteboard: [parity-IE][parity-webkit])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
requestAnimationFrame was unprefixed in Bug 704063.
Huh, only unprefixed requestAnimationFrame has been added without adding cancalAnimationFrame!?
You're in for a lot of broken sites if you don't backport this change to Firefox 23...
status-firefox23: --- → affected
tracking-firefox23: --- → ?
Boris, do you agree this needs to go in the same release as unprefixing requestAnimationFrame?  (And do you want to do it, or should we find someone else?)
Flags: needinfo?(bzbarsky)
tracking until we get an answer to comment 3 - will untrack if this doesn't have to ship at the same time as requestAnimationFrame.
status-firefox22: --- → unaffected
status-firefox24: --- → affected
tracking-firefox23: ? → +
tracking-firefox24: --- → +
Just to add one reason: the following polyfill from Paul Irish's blog breaks when only requestAnimationFrame is unprefixed:
http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/
You can find this polyfill in many places all over the internet, like:
http://my.opera.com/emoller/blog/2011/12/20/requestanimationframe-for-smart-er-animating
https://gist.github.com/paulirish/1579671

This is a really bad idea to unprefix only one of them.
This was just an oversight, yeah.  We need to add an unprefixed cancelAnimationFrame.  Luckily, this should be trivial to implement: both the prefixed and unprefixed cancel should have the same behavior.
Blocks: 704063
No longer depends on: 704063
Flags: needinfo?(bzbarsky)
Keywords: regression
(Assignee)

Comment 7

4 years ago
Created attachment 754791 [details] [diff] [review]
Patch v1

Will need a uuid rev for trunk and probably a new interface for backports
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
Attachment #754791 - Flags: review?(bzbarsky)
(Assignee)

Comment 8

4 years ago
Created attachment 754795 [details] [diff] [review]
Patch v1.1
Attachment #754791 - Attachment is obsolete: true
Attachment #754791 - Flags: review?(bzbarsky)
Attachment #754795 - Flags: review?(bzbarsky)
Comment on attachment 754795 [details] [diff] [review]
Patch v1.1

r=me with that uuid rev.
Attachment #754795 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 10

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1c63a4904d1
(Assignee)

Comment 11

4 years ago
Created attachment 754977 [details] [diff] [review]
Patch for aurora
Attachment #754977 - Flags: review?(bzbarsky)
(Assignee)

Comment 12

4 years ago
(In reply to :Ms2ger from comment #11)
> Created attachment 754977 [details] [diff] [review]
> Patch for aurora

https://tbpl.mozilla.org/?tree=Try&rev=5427e53156ff
Comment on attachment 754977 [details] [diff] [review]
Patch for aurora

r=me
Attachment #754977 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/e1c63a4904d1
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox24: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
(Assignee)

Comment 15

4 years ago
Comment on attachment 754977 [details] [diff] [review]
Patch for aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 704063
User impact if declined: sites may break
Testing completed (on m-c, etc.): a simpler patch that does change uuids is on m-c. This one passes try.
Risk to taking this patch (and alternatives if risky): pretty low risk. Alternative is unsupporting unprefixed requestAnimationFrame
String or IDL/UUID changes made by this patch: none
Attachment #754977 - Flags: approval-mozilla-aurora?
(In reply to Michał Gołębiowski from comment #5)
> Just to add one reason: the following polyfill from Paul Irish's blog breaks
> when only requestAnimationFrame is unprefixed:
> http://www.paulirish.com/2011/requestanimationframe-for-smart-animating/
> You can find this polyfill in many places all over the internet, like:
> http://my.opera.com/emoller/blog/2011/12/20/requestanimationframe-for-smart-
> er-animating
> https://gist.github.com/paulirish/1579671

Actually those polyfills will work because we didn't remove prefixed mozRequestAnimationFrame, mozCancelAnimationFrame and mozCancelRequestAnimationFrame.
But those polyfills will not use unprefixed functions if prefixed one is present. It is bad because the prefixed version doesn't support high-resolution timers. In short, the polyfills are broken anyway.
(In reply to Masatoshi Kimura [:emk] from comment #16)
> Actually those polyfills will work because we didn't remove prefixed
> mozRequestAnimationFrame, mozCancelAnimationFrame and
> mozCancelRequestAnimationFrame.
I'm sorry but you're wrong. Specifically, this polyfill:
https://gist.github.com/paulirish/1579671
would cause Firefox to use unprefixed requestAnimationFrame (the first `for` loop would exit immediately) but cancelAnimationFrame would be defined as a function clearing a timeout that wasn't even set (since it assumes requestAnimationFrame is implemented using timeouts, too).

> But those polyfills will not use unprefixed functions if prefixed one is
> present. It is bad because the prefixed version doesn't support
> high-resolution timers. In short, the polyfills are broken anyway.
You're wrong about that, too. Read the code again.
Do we really need to worry about changing UUIDs on Aurora?  I thought we didn't promise stability until beta.  But maybe I'm misremembering.
> Do we really need to worry about changing UUIDs on Aurora?

We may not have to, but it's easier to play it safe.
(In reply to Michał Gołębiowski from comment #17)
> (the first `for` loop would exit immediately)

Ah, I overlooked |&& !window.requestAnimationFrame| part, sorry.
Comment on attachment 754977 [details] [diff] [review]
Patch for aurora

Dbaron: we don't 'worry' about UUIDs on Aurora, but still good to know when there are changes in uplifted patches.
Attachment #754977 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/4e643d5826ac
status-firefox23: affected → fixed
https://developer.mozilla.org/en-US/docs/Web/API/window.cancelAnimationFrame
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/23
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23
Keywords: dev-doc-needed → dev-doc-complete
(In reply to lsblakk@mozilla.com [:lsblakk] from comment #21)
> Comment on attachment 754977 [details] [diff] [review]
> Patch for aurora
> 
> Dbaron: we don't 'worry' about UUIDs on Aurora, but still good to know when
> there are changes in uplifted patches.

Should we land the other patch on aurora, then, since it doesn't have a (small) codesize, runtime memory use, and performance hit?
You need to log in before you can comment on or make changes to this bug.