Skip to content

Commit

Permalink
Remove the .bind(name, fn, thisObject) and promote jQuery.event.proxy…
Browse files Browse the repository at this point in the history
…() to jQuery.proxy() as alternative to handling scoping on callbacks. Fixes #5736.
  • Loading branch information
jeresig committed Dec 31, 2009
1 parent a00e63e commit 66975de
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 35 deletions.
60 changes: 30 additions & 30 deletions src/event.js
Expand Up @@ -35,7 +35,7 @@ jQuery.event = {
var fn = handler;

// Create unique handler function, wrapped around original handler
handler = this.proxy( fn );
handler = jQuery.proxy( fn );

// Store data in unique handler
handler.data = data;
Expand Down Expand Up @@ -405,24 +405,6 @@ jQuery.event = {
return event;
},

proxy: function( fn, proxy, thisObject ) {
if ( proxy !== undefined && !jQuery.isFunction( proxy ) ) {
thisObject = proxy;
proxy = undefined;
}

// FIXME: Should proxy be redefined to be applied with thisObject if defined?
proxy = proxy || function() {
return fn.apply( thisObject !== undefined ? thisObject : this, arguments );
};

// Set the guid of unique handler to the same of original handler, so it can be removed
proxy.guid = fn.guid = fn.guid || proxy.guid || this.guid++;

// So proxy can be declared as an argument
return proxy;
},

special: {
ready: {
// Make sure the ready event is setup
Expand Down Expand Up @@ -474,6 +456,24 @@ jQuery.event = {
}
};

jQuery.event.proxy = jQuery.proxy = function( fn, proxy, thisObject ) {

This comment has been minimized.

Copy link
@aheckmann

aheckmann Dec 31, 2009

Is there any reason to leave jQuery.event.proxy here? I don't see anywhere that uses it now.

if ( proxy !== undefined && !jQuery.isFunction( proxy ) ) {
thisObject = proxy;
proxy = undefined;
}

// FIXME: Should proxy be redefined to be applied with thisObject if defined?
proxy = proxy || function() {
return fn.apply( thisObject !== undefined ? thisObject : this, arguments );
};

// Set the guid of unique handler to the same of original handler, so it can be removed
proxy.guid = fn.guid = fn.guid || proxy.guid || jQuery.event.guid++;

// So proxy can be declared as an argument
return proxy;
};

jQuery.Event = function( src ) {
// Allow instantiation without the 'new' keyword
if ( !this.preventDefault ) {
Expand Down Expand Up @@ -759,7 +759,7 @@ if ( document.addEventListener ) {
}

jQuery.each(["bind", "one"], function( i, name ) {
jQuery.fn[ name ] = function( type, data, fn, thisObject ) {
jQuery.fn[ name ] = function( type, data, fn ) {
// Handle object literals
if ( typeof type === "object" ) {
for ( var key in type ) {
Expand All @@ -773,12 +773,13 @@ jQuery.each(["bind", "one"], function( i, name ) {
fn = data;
data = undefined;
}
fn = thisObject === undefined ? fn : jQuery.event.proxy( fn, thisObject );
var handler = name === "one" ? jQuery.event.proxy( fn, function( event ) {

var handler = name === "one" ? jQuery.proxy( fn, function( event ) {
jQuery( this ).unbind( event, handler );
return fn.apply( this, arguments );
}) : fn;
return type === "unload" ? this.one(type, data, handler, thisObject) : this.each(function() {

return type === "unload" ? this.one(type, data, handler) : this.each(function() {
jQuery.event.add( this, type, handler, data );
});
};
Expand Down Expand Up @@ -820,10 +821,10 @@ jQuery.fn.extend({

// link all the functions, so any of them can unbind this click handler
while ( i < args.length ) {
jQuery.event.proxy( fn, args[ i++ ] );
jQuery.proxy( fn, args[ i++ ] );
}

return this.click( jQuery.event.proxy( fn, function( event ) {
return this.click( jQuery.proxy( fn, function( event ) {
// Figure out which function to execute
var lastToggle = ( jQuery.data( this, "lastToggle" + fn.guid ) || 0 ) % i;
jQuery.data( this, "lastToggle" + fn.guid, lastToggle + 1 );
Expand All @@ -840,17 +841,16 @@ jQuery.fn.extend({
return this.mouseenter( fnOver ).mouseleave( fnOut || fnOver );
},

live: function( type, data, fn, thisObject ) {
live: function( type, data, fn ) {
if ( jQuery.isFunction( data ) ) {
if ( fn !== undefined ) {
thisObject = fn;
}
fn = data;
data = undefined;
}

jQuery( this.context ).bind( liveConvert( type, this.selector ), {
data: data, selector: this.selector, live: type
}, fn, thisObject );
}, fn );

return this;
},

Expand Down
10 changes: 5 additions & 5 deletions test/unit/event.js
Expand Up @@ -232,8 +232,8 @@ test("bind(), with different this object", function() {
};

jQuery("#firstp")
.bind("click", handler1, thisObject).click().unbind("click", handler1)
.bind("click", data, handler2, thisObject).click().unbind("click", handler2);
.bind("click", jQuery.proxy(handler1, thisObject)).click().unbind("click", handler1)
.bind("click", data, jQuery.proxy(handler2, thisObject)).click().unbind("click", handler2);

ok( !jQuery.data(jQuery("#firstp")[0], "events"), "Event handler unbound when using different this object and data." );
});
Expand Down Expand Up @@ -706,15 +706,15 @@ test(".live()/.die()", function() {
jQuery("#foo").trigger("click", true).die("click");

// Test binding with different this object
jQuery("#foo").live("click", function(e){ equals( this.foo, "bar", "live with event scope" ); }, { foo: "bar" });
jQuery("#foo").live("click", jQuery.proxy(function(e){ equals( this.foo, "bar", "live with event scope" ); }, { foo: "bar" }));
jQuery("#foo").trigger("click").die("click");

// Test binding with different this object, event data, and trigger data
jQuery("#foo").live("click", true, function(e, data){
jQuery("#foo").live("click", true, jQuery.proxy(function(e, data){
equals( e.data, true, "live with with different this object, event data, and trigger data" );
equals( this.foo, "bar", "live with with different this object, event data, and trigger data" );
equals( data, true, "live with with different this object, event data, and trigger data")
}, { foo: "bar" });
}, { foo: "bar" }));
jQuery("#foo").trigger("click", true).die("click");

// Verify that return false prevents default action
Expand Down

1 comment on commit 66975de

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I commented on jquery-dev I'm leaving it in so that we don't break compatibility with plugins.

Please sign in to comment.