Skip to main content

Bug Tracker

Side navigation

#13353 closed bug (patchwelcome)

Opened January 31, 2013 01:29AM UTC

Closed January 31, 2013 05:26PM UTC

Last modified April 15, 2013 05:14PM UTC

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:
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.

Attachments (0)
Change History (12)

Changed January 31, 2013 01:46AM UTC by Inaki Anduaga comment:1

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

Changed January 31, 2013 02:12AM UTC by dmethvin comment:2

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.

Changed January 31, 2013 02:18AM UTC by dmethvin comment:3

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.

Changed January 31, 2013 05:26PM UTC by gibson042 comment:4

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.

Changed January 31, 2013 07:49PM UTC by Inaki Anduaga comment:5

Replying to [comment:4 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

Changed January 31, 2013 09:50PM UTC by gibson042 comment:6

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.

Changed January 31, 2013 11:43PM UTC by Inaki Anduaga comment:7

Replying to [comment:6 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.

Changed January 31, 2013 11:46PM UTC by dmethvin comment:8

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.

Changed February 10, 2013 09:38PM UTC by gibson042 comment:9

blocking: → 13428

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

Changed February 28, 2013 10:03PM UTC by Richard Gibson comment:10

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

Changeset: 3f05afbd8d9bbc75d30b68e720324d1ed984a315

Changed April 10, 2013 12:22PM UTC by anonymous comment:11

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

Changed April 15, 2013 05:14PM UTC by dmethvin comment:12

#13769 is a duplicate of this ticket.