Bug Tracker

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#935 closed bug (fixed)

Event.data is a global event object

Reported by: Nate Cavanaugh Owned by:
Priority: major Milestone: 1.1.3
Component: event Version: 1.1.2
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by brandon)

The behavior I was expecting was that binding data to an even via .bind(type, data, fn) would store the data relevant to that specific item, rather than a global event object.

A quick example:

$(['test1', 'test2', 'test3']).each(function(){ $('#'+this).bind('change', i, selectOnChange) })

Now, when selectOnChange gets executed, event.data will only contain the latest iterator, which leads me to think that event.data is being stored globally for all onchange events, rather than on a per instance basis.

Would it be possible to store this on a per instance basis?

Attachments (2)

jquery_bug935.patch (1.1 KB) - added by jesse 12 years ago.
Patch to fix Event data from being global
test_935.htm (862 bytes) - added by arrix 12 years ago.
unable to unbind() if data is specified for bind()

Download all attachments as: .zip

Change History (15)

comment:1 Changed 12 years ago by Arrix

Cause of the bug: The data is actually stored as a property of the handler function so the latest call to bind(type, data, fn) always overwrites previous data.

comment:2 Changed 12 years ago by brandon

Component: ajaxevent

comment:3 Changed 12 years ago by john

Milestone: 1.1.3
need: Test Case
Version: 1.1.2

comment:4 Changed 12 years ago by joern

need: Test CasePatch

Added test in [1588].

comment:5 Changed 12 years ago by dec

Any update here? This is a major bug causing me major pain. It practically means that all jQuery functions making use of event.data are useless unless you're passing static data (which seems pointless anyway).

Changed 12 years ago by jesse

Attachment: jquery_bug935.patch added

Patch to fix Event data from being global

comment:6 Changed 12 years ago by brandon

Resolution: fixed
Status: newclosed

Thanks Jesse for the patch!

Fixed in Rev. 1765.

comment:7 Changed 12 years ago by arrix

Resolution: fixed
Status: closedreopened

The fix introduces other problems. We are unable to unbind events bound with data specified. Because when data is given, guid will only be set on the closure, not the original function.

Changed 12 years ago by arrix

Attachment: test_935.htm added

unable to unbind() if data is specified for bind()

comment:8 Changed 12 years ago by arrix

Rev 1765 has changed the behavior of bind(). I remember, if we call bind() with the same handler function multiple times for the same element, the handler will only be executed once. Now the handler gets invoked multiple times if data is specified for bind().

comment:9 Changed 12 years ago by arrix

patch

Index: E:/zm/jquery/jquery/src/event/event.js
===================================================================
--- E:/zm/jquery/jquery/src/event/event.js	(revision 1771)
+++ E:/zm/jquery/jquery/src/event/event.js	(working copy)
@@ -32,8 +32,11 @@
 		}
 
 		// Make sure that the function being executed has a unique ID
-		if ( !handler.guid )
+		if ( !handler.guid ) {
 			handler.guid = this.guid++;
+			//don't forget to set guid for the original handler function
+			if (fn) fn.guid = handler.guid; 
+		}
 
 		// Init the element's event structure
 		if (!element.$events)
@@ -274,7 +277,7 @@
 	 */
 	bind: function( type, data, fn ) {
 		return this.each(function(){
-			jQuery.event.add( this, type, fn || data, data );
+			jQuery.event.add( this, type, fn || data, fn && data );
 		});
 	},
 	
@@ -309,7 +312,7 @@
 			jQuery.event.add( this, type, function(event) {
 				jQuery(this).unbind(event);
 				return (fn || data).apply( this, arguments);
-			}, data);
+			}, fn && data);
 		});
 	},
 

comment:10 Changed 12 years ago by brandon

Resolution: fixed
Status: reopenedclosed

Fixed in Rev. 1775.

comment:11 Changed 12 years ago by jesse

In my patch, I moved the "handler.guid = this.guid++;" code above the code that created wrapper handler function, then had "handler.guid = fn.guid;" which passed the guid on to the wrapper function. This didn't happen when the patch was applied, thus why the bug was introduced.

For cleanliness sake, I suggest either removing the line "handler.guid = fn.guid;", or better, just moving the block above and removing the "if (fn) fn.guid = handler.guid;", as this patch will do:

Index: event.js
===================================================================
--- event.js	(revision 1801)
+++ event.js	(working copy)
@@ -12,6 +12,10 @@
 		// around, causing it to be cloned in the process
 		if ( jQuery.browser.msie && element.setInterval != undefined )
 			element = window;
+
+		// Make sure that the function being executed has a unique ID
+		if ( !handler.guid )
+			handler.guid = this.guid++;
 		
 		// if data is passed, bind to handler 
 		if( data != undefined ) { 
@@ -31,13 +35,6 @@
 			handler.guid = fn.guid;
 		}
 
-		// Make sure that the function being executed has a unique ID
-		if ( !handler.guid ) {
-			handler.guid = this.guid++;
-			// Don't forget to set guid for the original handler function
-			if (fn) fn.guid = handler.guid;
-		}
-
 		// Init the element's event structure
 		if (!element.$events)
 			element.$events = {};

comment:12 Changed 12 years ago by jesse

Resolution: fixed
Status: closedreopened

comment:13 Changed 12 years ago by brandon

Description: modified (diff)
Resolution: fixed
Status: reopenedclosed

Good call ... thanks Jesse! Fixed in Rev [1962].

Note: See TracTickets for help on using tickets.