Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#13353 closed bug (patchwelcome)

Trigger on click events fails to pass parameters (affects v1.9.0 and v2.0.0b1 only)

Reported by: Inaki Anduaga Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 1.9.0
Keywords: Cc:
Blocked by: Blocking: #13428

Description

Versions affected: v1.9.0 and v2.0.0b1 ONLY

Browsers affected: Firefox 18, Chrome v25, probably all others too OS: Windows 7 64

Steps:

1) Bind a click event with additional parameters in the handler to a simple input element

2) Trigger a click event on the input passing extra parameters

Expected Result: Parameters should be accesible inside the click binding.
Actual Result: Parameter values are undefined


Example: http://jsfiddle.net/UR6E8/1/
This should be a critical bug if it is indeed one and I'm not making some extremely basic mistake or there is some new change in Jquery 1.9 that somehow breaks the expected result.

Change History (12)

comment:1 Changed 7 years ago by Inaki Anduaga

Upon further review, it seems the bug affects only the 'click' event and applies only to checkbox input elements, so it's most likely related to the changes in http://jquery.com/upgrade-guide/1.9/#checkbox-radio-state-in-a-trigger-ed-click-event

comment:2 Changed 7 years ago by dmethvin

Excellent diagnosis! Yes, so it seems like if we want to fix this you can get your data but will have to live with the fact that the checkbox state would not be correct. Choose your poison.

comment:3 Changed 7 years ago by dmethvin

I should point out that if you just wanted to call your click handler(s) and pass the data but not have it call the DOM .click() method you could use .triggerHandler("click", data) as a workaround.

comment:4 Changed 7 years ago by gibson042

Resolution: patchwelcome
Status: newclosed

This is actually fixable, but at a whopping +124 bytes by my count. I'll leave the diff here for posterity (or possible pluginization), but note that it relies on undocumented functionality.

jquery:d79bf351...gibson042:13353

Not by a long shot do I see us landing this, though.

comment:5 in reply to:  4 Changed 7 years ago by Inaki Anduaga

Replying to gibson042:

This is actually fixable, but at a whopping +124 bytes by my count. I'll leave the diff here for posterity (or possible pluginization), but note that it relies on undocumented functionality.

jquery:d79bf351...gibson042:13353

Not by a long shot do I see us landing this, though.

I don't understand if you're being sarcastic or not (guessing not). What does the "patch welcome" status mean? Will it be fixed in the coming jquery update or do I have to modify my code to live with it? Thanks

comment:6 Changed 7 years ago by gibson042

My apologies. We value a small library, and in some cases balance that desire against failing on things like invalid inputs and edge cases. In this instance, the presence of a .triggerHandler workaround, the issues with our old implementation presenting the wrong checked state, and the massive size cost of the best identified fix combine to support a decision for excluding it. I marked it patchwelcome because we would land a small enough fix if one were discovered.

In the meantime, you can either use a workaround or (if you are willing to accept the size increase) import the code from my repository.

comment:7 in reply to:  6 Changed 7 years ago by Inaki Anduaga

Replying to gibson042:

My apologies. We value a small library, and in some cases balance that desire against failing on things like invalid inputs and edge cases. In this instance, the presence of a .triggerHandler workaround, the issues with our old implementation presenting the wrong checked state, and the massive size cost of the best identified fix combine to support a decision for excluding it. I marked it patchwelcome because we would land a small enough fix if one were discovered.

In the meantime, you can either use a workaround or (if you are willing to accept the size increase) import the code from my repository.

Thanks a lot for the explanation. I'm surprised there would be bugs left-over just for the sake of reducing the library size, hopefully the docs will be updated to reflect the issue in case the problem is never fixed.

comment:8 Changed 7 years ago by dmethvin

I'm surprised there would be bugs left-over just for the sake of reducing the library size

Adding more code is a sure-fire way to add more bugs.

comment:9 Changed 7 years ago by gibson042

Blocking: 13428 added

(In #13428) This is exactly analogous to #13353... I'm starting to think we should generalize and incorporate that solution after all.

comment:10 Changed 7 years ago by Richard Gibson

Ref #13353, gh-1183: Capture onlyHandlers in jQuery.Event.isTrigger.

Changeset: 3f05afbd8d9bbc75d30b68e720324d1ed984a315

comment:11 Changed 7 years ago by anonymous

The solution should be more generic, since it's not only click event, but also focus, blur, etc..

comment:12 Changed 7 years ago by dmethvin

#13769 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.