Effects: Resolve issues revealed by recent Callbacks fix

Notify full progress before resolving empty animations
Register animation callbacks before their ticker
Remove the right timer when immediately-done animations spawn more

Ref 9d822bc1c13dd3393b418210a99537c22c06f2c3
Fixes gh-3502
Fixes gh-3503
Closes gh-3496

diff --git a/src/effects.js b/src/effects.js
index 4823659c230d466ef2f416bc36ae8bf26bde890f..8fa2e56e6e611d464c8605c89617344846a56d17 100644 (file)
--- a/src/effects.js
+++ b/src/effects.js
@@ -315,12 +315,19 @@ function Animation( elem, properties, options ) {
 
                        deferred.notifyWith( elem, [ animation, percent, remaining ] );
 
+                       // If there's more to do, yield
                        if ( percent < 1 && length ) {
                                return remaining;
-                       } else {
-                               deferred.resolveWith( elem, [ animation ] );
-                               return false;
                        }
+
+                       // If this was an empty animation, synthesize a final progress notification
+                       if ( !length ) {
+                               deferred.notifyWith( elem, [ animation, 1, 0 ] );
+                       }
+
+                       // Resolve the animation and report its conclusion
+                       deferred.resolveWith( elem, [ animation ] );
+                       return false;
                },
                animation = deferred.promise( {
                        elem: elem,
@@ -385,6 +392,13 @@ function Animation( elem, properties, options ) {
                animation.opts.start.call( elem, animation );
        }
 
+       // Attach callbacks from options
+       animation
+               .progress( animation.opts.progress )
+               .done( animation.opts.done, animation.opts.complete )
+               .fail( animation.opts.fail )
+               .always( animation.opts.always );
+
        jQuery.fx.timer(
                jQuery.extend( tick, {
                        elem: elem,
@@ -393,11 +407,7 @@ function Animation( elem, properties, options ) {
                } )
        );
 
-       // attach callbacks from options
-       return animation.progress( animation.opts.progress )
-               .done( animation.opts.done, animation.opts.complete )
-               .fail( animation.opts.fail )
-               .always( animation.opts.always );
+       return animation;
 }
 
 jQuery.Animation = jQuery.extend( Animation, {
@@ -641,7 +651,7 @@ jQuery.fx.tick = function() {
        for ( ; i < timers.length; i++ ) {
                timer = timers[ i ];
 
-               // Checks the timer has not already been removed
+               // Run the timer and safely remove it when done (allowing for external removal)
                if ( !timer() && timers[ i ] === timer ) {
                        timers.splice( i--, 1 );
                }
@@ -654,11 +664,16 @@ jQuery.fx.tick = function() {
 };
 
 jQuery.fx.timer = function( timer ) {
-       jQuery.timers.push( timer );
+       var i = jQuery.timers.push( timer ) - 1,
+               timers = jQuery.timers;
+
        if ( timer() ) {
                jQuery.fx.start();
-       } else {
-               jQuery.timers.pop();
+
+       // If the timer finished immediately, safely remove it (allowing for external removal)
+       // Use a superfluous post-decrement for better compressibility w.r.t. jQuery.fx.tick above
+       } else if ( timers[ i ] === timer ) {
+               timers.splice( i--, 1 );
        }
 };
 

diff --git a/test/unit/effects.js b/test/unit/effects.js
index eafe4b116eea4fd65de1621101d45fef914f05a2..d64a639a18fafdeda6d2b4bbdfaf3f79bb1dd3b4 100644 (file)
--- a/test/unit/effects.js
+++ b/test/unit/effects.js
@@ -1256,17 +1256,18 @@ QUnit.test( "animate with CSS shorthand properties", function( assert ) {
 } );
 
 QUnit.test( "hide hidden elements, with animation (bug #7141)", function( assert ) {
-       assert.expect( 3 );
+       assert.expect( 4 );
 
-       var div = jQuery( "<div style='display:none'></div>" ).appendTo( "#qunit-fixture" );
-       assert.equal( div.css( "display" ), "none", "Element is hidden by default" );
-       div.hide( 1, function() {
-               assert.ok( !jQuery._data( div, "olddisplay" ), "olddisplay is undefined after hiding an already-hidden element" );
-               div.show( 1, function() {
-                       assert.equal( div.css( "display" ), "block", "Show a double-hidden element" );
+       var div = jQuery( "<div id='bug7141' style='display:none'/>" ).appendTo( "#qunit-fixture" );
+       assert.equal( div.css( "display" ), "none", "Element is initially hidden" );
+       div.hide( 10, function() {
+               assert.equal( div.css( "display" ), "none", "Element is hidden in .hide() callback" );
+               div.show( 11, function() {
+                       assert.equal( div.css( "display" ), "block", "Element is visible in .show() callback" );
                } );
        } );
-       this.clock.tick( 10 );
+       this.clock.tick( 50 );
+       assert.equal( div.css( "display" ), "block", "Element is visible after animations" );
 } );
 
 QUnit.test( "animate unit-less properties (#4966)", function( assert ) {