Side navigation
#7930 closed bug (fixed)
Opened January 09, 2011 06:11PM UTC
Closed September 08, 2011 06:37PM UTC
Last modified November 22, 2011 12:29AM UTC
Methods on custom objects are being called when an event with the same name is triggered on it
| Reported by: | timmolendijk | Owned by: | dmethvin |
|---|---|---|---|
| Priority: | low | Milestone: | 1.next |
| Component: | event | Version: | 1.4.4 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
I don't understand why this happens:
var customObject = { hello: function() { alert("Hello!"); } };
$(customObject).trigger('hello'); // An alert will pop up!
$(customObject).triggerHandler('hello'); // No alert will pop up.
Why is
customObject.hellobeing called when an
helloevent is triggered on
customObject? I wasn't expecting it and I cannot find any hint towards it in the docs. My guess is that it's an unintended side-effect of the new (explicit) support for using
jQuery.fn.triggeron custom objects. Am I right?
Attachments (0)
Change History (8)
Changed January 10, 2011 03:27AM UTC by comment:1
| owner: | → dmethvin |
|---|---|
| status: | new → assigned |
Changed January 10, 2011 02:54PM UTC by comment:2
| _comment0: | I already assessed this one yesterday but didn't come around to add my findings. Sorry Dave. I basically figured out the same thing as you (code branch in trigger() is only meant for DOM element nodes to use the browsers native event method if applicable) \ \ This also is straightforward to fix imo. We could just change [https://github.com/jquery/jquery/blob/c5c1f18adb4dfae5366eafada12ed02fe1b7064d/src/event.js#L388 this line] to \ {{{ \ !isClick && target && jQuery.acceptData( target ) ) { \ }}} \ This is better readable, uses acceptData instead repeating the very same thing in the conditional. It also has the nice side effect of fixing a potential other bug: that this branch gets run even when target is e.g. undefined. \ \ So when you tackle this I guess you could do some refactoring and save us a few bytes, as there are at least two other places where a similar pattern (along `elem && elem.nodeName && jQuery.noData[elem.nodeName.toLowerCase()]`) is used which (I guess) can be switch to using acceptData() too. (Maybe even incorporate the check if elem is defined/undefined into the acceptData too?) \ \ Replying to [comment:1 dmethvin]: \ > Sounds like we should put a check into jQuery.event.trigger to see if the element is a DOM element, before someone decides this is a feature. \ This "faulty" undocumented behavior is in jQuery since 1.1 (!!!!) as you can see with this [http://jsfiddle.net/jitter/DEkqZ/ live test case]. So by now there surely are some people relying on this hidden "gem" but this should get fixed anyway. → 1294671478228393 |
|---|---|
| _comment1: | I already assessed this one yesterday but didn't come around to add my findings. Sorry Dave. I basically figured out the same thing as you (code branch in trigger() is only meant for DOM element nodes to use the browsers native event method if applicable) \ \ This also is straightforward to fix imo. It should be fine to just change [https://github.com/jquery/jquery/blob/c5c1f18adb4dfae5366eafada12ed02fe1b7064d/src/event.js#L388 this line] to \ {{{ \ !isClick && target && jQuery.acceptData( target ) ) { \ }}} \ This is better readable, uses acceptData instead repeating the very same thing in the conditional. It also has the nice side effect of fixing a potential other bug: that this branch gets run even when target is e.g. undefined. \ \ So when you tackle this I guess you could do some refactoring and save us a few bytes, as there are at least two other places where a similar pattern (along `elem && elem.nodeName && jQuery.noData[elem.nodeName.toLowerCase()]`) is used which (I guess) can be switch to using acceptData() too. (Maybe even incorporate the check if elem is defined/undefined into the acceptData too?) \ \ Replying to [comment:1 dmethvin]: \ > Sounds like we should put a check into jQuery.event.trigger to see if the element is a DOM element, before someone decides this is a feature. \ This "faulty" undocumented behavior is in jQuery since 1.1 (!!!!) as you can see with this [http://jsfiddle.net/jitter/DEkqZ/ live test case]. So by now there surely are some people relying on this hidden "gem" but this should get fixed anyway. → 1294671578886296 |
| component: | unfiled → event |
| priority: | undecided → low |
I already assessed this one yesterday but didn't come around to add my findings. Sorry Dave. I basically figured out the same thing as you (code branch in trigger() is only meant for DOM element nodes to use the browsers native event method if applicable)
This also is straightforward to fix imo. It should be fine to just change this line to
!isClick && target && jQuery.acceptData( target ) ) {
This is better readable, uses acceptData instead of repeating the very same thing in the conditional. It also has the nice side effect of fixing a potential other bug: that this branch gets run even when target is e.g. undefined.
So when you tackle this I guess you could do some refactoring and save us a few bytes, as there are at least two other places where a similar pattern (along elem && elem.nodeName && jQuery.noData[elem.nodeName.toLowerCase()]) is used. These can probably be switched to use acceptData() too. Maybe we even can incorporate the check if elem is defined/undefined into acceptData() too?
Replying to [comment:1 dmethvin]:
Sounds like we should put a check into jQuery.event.trigger to see if the element is a DOM element, before someone decides this is a feature.
This "faulty" undocumented behavior is in jQuery since 1.1 (!!!!) as you can see with this live test case. So by now there surely are some people relying on this hidden "gem" but this should get fixed anyway.
Changed March 14, 2011 01:54PM UTC by comment:3
Whether or not this is changed should be defined by #7818.
Changed September 08, 2011 06:37PM UTC by comment:6
| resolution: | → fixed |
|---|---|
| status: | assigned → closed |
It's meant to handle the situation where an event named
Xis triggered on a DOM element and there's a correspondingXmethod on the element. Events likechange,focus, andblurfor example.So yes, in those cases it will call the correspondingly named function on the plain object if it exists. Actually it will try to call the property regardless of whether it's a function or not, because it's impossible to tell with IE.
Sounds like we should put a check into jQuery.event.trigger to see if the element is a DOM element, before someone decides this is a feature.