Skip to main content

Bug Tracker

Side navigation

#935 closed bug (fixed)

Opened February 08, 2007 10:31PM UTC

Closed May 23, 2007 01:52PM UTC

Last modified June 21, 2007 04:03AM UTC

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

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 April 17, 2007 01:28PM UTC.

    Patch to fix Event data from being global

  • test_935.htm (0.8 KB) - added by arrix April 24, 2007 05:23AM UTC.

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

Change History (13)

Changed February 09, 2007 05:06AM UTC by Arrix comment:1

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.

Changed February 10, 2007 09:37PM UTC by brandon comment:2

component: ajaxevent

Changed March 24, 2007 06:08PM UTC by john comment:3

milestone: → 1.1.3
need: → Test Case
version: → 1.1.2

Changed March 25, 2007 12:08PM UTC by joern comment:4

need: Test CasePatch

Added test in [1588].

Changed April 17, 2007 11:36AM UTC by dec comment:5

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 April 22, 2007 03:18AM UTC by brandon comment:6

resolution: → fixed
status: newclosed

Thanks Jesse for the patch!

Fixed in Rev. 1765.

Changed April 24, 2007 05:21AM UTC by arrix comment:7

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 April 24, 2007 05:37AM UTC by arrix comment:8

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().

Changed April 24, 2007 05:39AM UTC by arrix comment:9

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);
 		});
 	},
 

Changed April 24, 2007 10:35PM UTC by brandon comment:10

resolution: → fixed
status: reopenedclosed

Fixed in Rev. 1775.

Changed April 27, 2007 09:44AM UTC by jesse comment:11

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 = {};

Changed May 23, 2007 10:23AM UTC by jesse comment:12

resolution: fixed
status: closedreopened

Changed May 23, 2007 01:52PM UTC by brandon comment:13

description: 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?\ \ 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? \ \
resolution: → fixed
status: reopenedclosed

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