"Too much recursion" with jQuery 3, Firefox, and animate's complete callback. #3434
Assignees
Labels
Comments
|
Yeah, this kinda, sorta by design – #1578 (comment) /cc @dmethvin, @scottgonzalez |
|
One option to try is to use page visibility api to swap rAF with setTimeout. |
markelog
added a commit
to markelog/jquery
that referenced
this issue
Mar 6, 2017
rAF logic was introduced almost three years ago relative to this commit, as a primary method for scheduling animation (see jquerygh-1578 pull). With it there was two substantial changes - one was explicitly mentioned and the other was not. First, if browser window was hidden aka `document.hidden === true` it would immediately execute all scheduled animation without waiting for time pass i.e. tick time become `0` instead of 13 ms of a default value. Which created possibility for circular executions in case if `complete` method executed the same animation (see jquerygh-3434 issue). And the second one - since then there was two ways of scheduling animation: with `setInterval` and `requestAnimationFrame`, but there was a difference in their execution. In case of `setInterval` it waited default `jQuery.fx.interval` value before actually starting the new tick, not counting the first step which wasn't set to be executed through tick method (aka `jQuery.fx.tick`). Whereas `requestAnimationFrame` first scheduled the call and executed the `step` method right after that, counting the first call of `jQuery.fx.timer`, `tick` was happening twice in one frame. But since tests explicitly disabled rAF method i.e. `requestAnimationFrame = null` and checking only `setInterval` logic, since it's impossible to do it otherwise - we missed that change. Faulty logic also was presented with `cancelAnimationFrame`, which couldn't clear any timers since `raf` scheduler didn't define new `timerId` value. Because that change was so subtle, apparently no user noticed it proving that both `cancelAnimationFrame` and `clearInterval` code paths are redundant. Since `cancelAnimationFrame` didn't work properly and rAF is and was a primary used code path, plus the same approach is used in other popular animation libs. Therefore those code paths were removed. These changes also replace two different functions which schedule the animation with one, which checks what type of logic should be used and executes it appropriatley, but for secondary path it now uses `setTimeout` making it more consistent with rAF path. Since ticks are happening globally we also don't require to listen `visibilitychange` event. It also changes the way how first call is scheduled so execution of animation will not happen twice in one frame. No new tests were not introduced, since now `setTimeout` logic should be equivalent to the rAF one, but one test was changed since now we actually execute animation at the first tick. Fixes jquerygh-3434
markelog
added a commit
to markelog/jquery
that referenced
this issue
Mar 6, 2017
rAF logic was introduced almost three years ago relative to this commit, as a primary method for scheduling animation (see jquerygh-1578 pull). With it there was two substantial changes - one was explicitly mentioned and the other was not. First, if browser window was hidden aka `document.hidden === true` it would immediately execute all scheduled animation without waiting for time pass i.e. tick time become `0` instead of 13 ms of a default value. Which created possibility for circular executions in case if `complete` method executed the same animation (see jquerygh-3434 issue). And the second one - since then there was two ways of scheduling animation: with `setInterval` and `requestAnimationFrame`, but there was a difference in their execution. In case of `setInterval` it waited default `jQuery.fx.interval` value before actually starting the new tick, not counting the first step which wasn't set to be executed through tick method (aka `jQuery.fx.tick`). Whereas `requestAnimationFrame` first scheduled the call and executed the `step` method right after that, counting the first call of `jQuery.fx.timer`, `tick` was happening twice in one frame. But since tests explicitly disabled rAF method i.e. `requestAnimationFrame = null` and checking only `setInterval` logic, since it's impossible to do it otherwise - we missed that change. Faulty logic also was presented with `cancelAnimationFrame`, which couldn't clear any timers since `raf` scheduler didn't define new `timerId` value. Because that change was so subtle, apparently no user noticed it proving that both `cancelAnimationFrame` and `clearInterval` code paths are redundant. Since `cancelAnimationFrame` didn't work properly and rAF is and was a primary used code path, plus the same approach is used in other popular animation libs. Therefore those code paths were removed. These changes also replace two different functions which schedule the animation with one, which checks what type of logic should be used and executes it appropriatley, but for secondary path it now uses `setTimeout` making it more consistent with rAF path. Since ticks are happening globally we also don't require to listen `visibilitychange` event. It also changes the way how first call is scheduled so execution of animation will not happen twice in one frame. No new tests were not introduced, since now `setTimeout` logic should be equivalent to the rAF one, but one test was changed since now we actually execute animation at the first tick. Fixes jquerygh-3434
markelog
added a commit
to markelog/jquery
that referenced
this issue
Mar 6, 2017
rAF logic was introduced almost three years ago relative to this commit, as a primary method for scheduling animation (see jquerygh-1578 pull). With it there was two substantial changes - one was explicitly mentioned and the other was not. First, if browser window was hidden aka `document.hidden === true` it would immediately execute all scheduled animation without waiting for time pass i.e. tick time become `0` instead of 13 ms of a default value. Which created possibility for circular executions in case if `complete` method executed the same animation (see jquerygh-3434 issue). And the second one - since then there was two ways of scheduling animation: with `setInterval` and `requestAnimationFrame`, but there was a difference in their execution. In case of `setInterval` it waited default `jQuery.fx.interval` value before actually starting the new tick, not counting the first step which wasn't set to be executed through tick method (aka `jQuery.fx.tick`). Whereas `requestAnimationFrame` first scheduled the call and executed the `step` method right after that, counting the first call of `jQuery.fx.timer`, `tick` was happening twice in one frame. But since tests explicitly disabled rAF method i.e. `requestAnimationFrame = null` and checking only `setInterval` logic, since it's impossible to do it otherwise - we missed that change. Faulty logic also was presented with `cancelAnimationFrame`, which couldn't clear any timers since `raf` scheduler didn't define new `timerId` value. Because that change was so subtle, apparently no user noticed it proving that both `cancelAnimationFrame` and `clearInterval` code paths are redundant. Since `cancelAnimationFrame` didn't work properly and rAF is and was a primary used code path, plus the same approach is used in other popular animation libs. Therefore those code paths were removed. These changes also replace two different functions which schedule the animation with one, which checks what type of logic should be used and executes it appropriatley, but for secondary path it now uses `setTimeout` making it more consistent with rAF path. Since ticks are happening globally we also don't require to listen `visibilitychange` event. It also changes the way how first call is scheduled so execution of animation will not happen twice in one frame. No new tests were not introduced, since now `setTimeout` logic should be equivalent to the rAF one, but one test was changed since now we actually execute animation at the first tick. Fixes jquerygh-3434
4 tasks
markelog
added a commit
to markelog/jquery
that referenced
this issue
Mar 6, 2017
rAF logic was introduced almost three years ago relative to this commit, as a primary method for scheduling animation (see jquerygh-1578 pull). With it there was two substantial changes - one was explicitly mentioned and the other was not. First, if browser window was hidden aka `document.hidden === true` it would immediately execute all scheduled animation without waiting for time pass i.e. tick time become `0` instead of 13 ms of a default value. Which created possibility for circular executions in case if `complete` method executed the same animation (see jquerygh-3434 issue). And the second one - since then there was two ways of scheduling animation: with `setInterval` and `requestAnimationFrame`, but there was a difference in their execution. In case of `setInterval` it waited default `jQuery.fx.interval` value before actually starting the new tick, not counting the first step which wasn't set to be executed through tick method (aka `jQuery.fx.tick`). Whereas `requestAnimationFrame` first scheduled the call and executed the `step` method right after that, counting the first call of `jQuery.fx.timer`, `tick` was happening twice in one frame. But since tests explicitly disabled rAF method i.e. `requestAnimationFrame = null` and checking only `setInterval` logic, since it's impossible to do it otherwise - we missed that change. Faulty logic also was presented with `cancelAnimationFrame`, which couldn't clear any timers since `raf` scheduler didn't define new `timerId` value. Because that change was so subtle, apparently no user noticed it proving that both `cancelAnimationFrame` and `clearInterval` code paths are redundant. Since `cancelAnimationFrame` didn't work properly and rAF is and was a primary used code path, plus the same approach is used in other popular animation libs. Therefore those code paths were removed. These changes also replace two different functions which schedule the animation with one, which checks what type of logic should be used and executes it appropriatley, but for secondary path it now uses `setTimeout` making it more consistent with rAF path. Since ticks are happening globally we also don't require to listen `visibilitychange` event. It also changes the way how first call is scheduled so execution of animation will not happen twice in one frame. No new tests were not introduced, since now `setTimeout` logic should be equivalent to the rAF one, but one test was changed since now we actually execute animation at the first tick. Fixes jquerygh-3434
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Versions:
Thing: I have a loading bar that animates from left-to-right, then resets. Note that it uses the complete callback (from animate) to fire itself again.
Repro:
Note that it works if I wrap the "reset" call in a setTimeout, as in:
complete: function() { setTimeout(reset, 0) }Speculation: Probably something to do with the requestAnimationFrame changes in jQuery 3? (If you leave the window focused, the recursion error does not appear).
Does NOT happen in Chrome/IE. Also, doesn't happen with jQuery 2.1.3.
The text was updated successfully, but these errors were encountered: