Skip to content
Permalink
Browse files

Regression: makes sure that all instances of a callback are removed. …

…Unit test added.
  • Loading branch information
jaubourg committed Apr 25, 2012
1 parent 97210d4 commit 245f5a244e024cb99ac84be86f86b88654a909c9
Showing with 17 additions and 2 deletions.
  1. +2 −2 src/callbacks.js
  2. +15 −0 test/unit/callbacks.js
@@ -119,8 +119,8 @@ jQuery.Callbacks = function( options ) {
// Remove a callback from the list
remove: function() {
if ( list ) {
jQuery.each( arguments, function( index, arg ) {
if ( ( index = jQuery.inArray( arg, list ) ) > -1 ) {
jQuery.each( arguments, function( _, arg, index ) {

This comment has been minimized.

Copy link
@gibson042

gibson042 Apr 25, 2012

Member

Did you try this with var index; instead of having it as a fake parameter? Our initial variable declarations are compressed very well by gzip.

This comment has been minimized.

Copy link
@jaubourg

jaubourg Apr 25, 2012

Author Member

42dbc86

thanks for your expert eye.

while( ( index = jQuery.inArray( arg, list, index || 0 ) ) > -1 ) {
list.splice( index, 1 );
// Handle firing indexes
if ( firing ) {
@@ -235,3 +235,18 @@ test( "jQuery.Callbacks.fireWith - arguments are copied", function() {
strictEqual( hello, "hello", "arguments are copied internally" );
});
});

test( "jQuery.Callbacks.remove - should remove all instances", function() {

expect( 1 );

var cb = jQuery.Callbacks();

function fn() {
ok( false, "function wasn't removed" );
}

cb.add( fn, fn, function() {
ok( true, "end of test" );
}).remove( fn ).fire();
});

0 comments on commit 245f5a2

Please sign in to comment.
You can’t perform that action at this time.