Bug Tracker

Ticket #2317 (closed bug: invalid)

Opened 6 years ago

Last modified 2 years ago

jQuery.event.trigger modifies data parameter

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

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"

Change History

comment:1 Changed 6 years ago by brandon

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
[]);
[]);

comment:2 Changed 6 years ago by davidserduke

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.

comment:3 Changed 6 years ago by paul

  • Owner set to 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!

comment:4 Changed 6 years ago by john

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.

comment:5 Changed 6 years ago by brandon

  • Status changed from new to closed
  • Resolution set to invalid

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.

Note: See TracTickets for help on using tickets.