jQuery.event.trigger modifies data parameter

Reported by:	scottgonzalez	Owned by:	john
Priority:	major	Milestone:	1.2.4
Component:	event	Version:	1.2.3
Keywords:		Cc:
Blocked by:		Blocking:

Description

The object passed to jQuery.event.trigger as the data parameter is modified, but shouldn't be.

var e = $.event.fix({ type: "foo", target: document });
var data = [e, $(document)];
console.log(data); // has type "foo"
$(document).trigger('bar', data);
console.log(data); // has type "bar"

Attachments (0)

Change History (5)

Changed February 17, 2008 07:30AM UTC by brandon comment:1

Looks like we need to add a var in front of data to scope it locally when we clone it.

// Clone the incoming data, if any

data = jQuery.makeArray(data || []);

+ var data = jQuery.makeArray(data || []);

Changed February 17, 2008 06:34PM UTC by davidserduke comment:2

I think data is actually a parameter to the trigger function so already locally scoped. So the problem appears to come from two lines:

event = !data[0] || !data[0].preventDefault;

then

// Enforce the right trigger type
data[0].type = type;

In this case, the trigger function is checking to see if the first parameter of the data is an event. If not, it will create an event to shift to the front of the data parameter. If so, it will just leave it. The test case above is one of those cases where the coder is passing in an event as the first parameter but still wants an artificial event created.

I'm not sure the best way to fix this. A workaround is to switch the order of the data in the test case to

var data = [$(document), e];

So data[0] would no longer be an event. In this case the code would push a "bar" event at the front of the list which jQuery requires. Another way would be to put a "bar" event the front for jQuery like this

var data = [$.event.fix({ type: "bar", target: document }), e, $(document)];

Then the handler code wouldn't have to be changed.

I have no idea why it is necessary to "Enforce the right trigger type" and what caused that line (line 2 of the problem code above) to be added. Perhaps there is a case where the event can come in and not be the right type but still be appropriate? Another possibility (with unknown side effects) would be to put an additional test in the 'event' boolean like

event = !data[0] || ! ( data[0].preventDefault && data[0].type == type );

I think that would fix the test case.

Changed March 14, 2008 08:38PM UTC by paul comment:3

owner:	→ john

John, could you please review the last code line, and check if it fixes the issue? If yes, it'd be great if you could fix this during the Sprint. Thanks!

Changed March 15, 2008 10:32PM UTC by john comment:4

This is tricky. Really, the only way around this is to completely clone the event object on every trigger. I'm not completely sure how I feel about that, from a performance perspective.

Changed April 29, 2008 05:22PM UTC by brandon comment:5

resolution:	→ invalid
status:	new → closed

The whole event system is based around an event type. It is how it knows what handlers to fire. Since you triggered the actual event "bar", the event type _is_ "bar". If you wanted to trigger event "foo", trigger it instead.

This said if you are passing the event object to another handler/method via trigger and you need to include the original event type... simply add it to the event object as another property, add it as an argument to the handler, or reference the originalEvent property found in latest SVN (event.originalEvent.type). But just know that data[0] represents the event that is being triggered.

Side navigation

#2317 closed bug (invalid)