Side navigation
#10168 closed bug (wontfix)
Opened August 30, 2011 02:23PM UTC
Closed October 13, 2011 04:11AM UTC
Last modified July 29, 2013 07:16PM UTC
jQuery.event.trigger and inline handlers.
| Reported by: | stefen@mediaodyssee.com | Owned by: | dmethvin |
|---|---|---|---|
| Priority: | low | Milestone: | None |
| Component: | event | Version: | 1.6.2 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
Looking at the event.js file, in the jQuery.event.trigger method, it seems that a triggered event is first bubbled manually, then triggered natively to allow default browser behaviours. The problem occurs when you add a listener without jQuery, like an inline onclick listener.
As I understand it, it kind of looks that way :
current = evt.target;
do
execute current jquery handlers
execute current inline handler
current = current.parentNode
while not evt.isPropagationStopped
if not evt.defaultPrevented
deactivating jquery handlers
deactivating evt.target inline handler but not ancestor ones
evt.target[event.type]()
So, if the event is not defaultPrevented, the upward inline handlers are executed twice. If it is, listeners added using native attachEvent|addEventListener methods would be triggered but delayed compared to the jQuery handlers, and also the evt.isPropagationStopped eventually setted by a previous jQuery handler would be ignored.
Although the jQuery.event.triggered flag, which seems to deactivate the jQuery shared handler, can be used in the inline handlers as well, why not doing something like this :
if evt.target[event.type]
evt.target[event.type]()
else
bubble manually ...
I'm although curious about the way you will handle the DOM 3 capture behaviour.
Another thought : I can imagine how tough it is to maintain the consistency of the lib, but the more I'm reading it, the more I'm thinking : if I'm building a web app using jQuery, I shouldn't use anything else which is not build against the same jQuery, otherwise I would encounter strange behaviours. It would be nice to inform users about the interactions and caveats somewhere in the documentation. If it is, my bad, I'll do better reading it next time.
Seems to be related to the following tickets : #6593, #6705
Here's the use case : == http://jsfiddle.net/FrKyN/19/ ==
Tested using Chromium 13 and Aurora 6
Attachments (0)
Change History (9)
Changed August 30, 2011 03:20PM UTC by comment:1
| _comment0: | Those are some great questions! \ \ Your analysis is basically correct. There are a few related tickets like #3287 as well that show other issues with the `.trigger()` design peeking through. I think the general team consensus is that inline handlers are both bad practice and a pain in our butts due to issues like this. \ \ I suspect the reason this may not be reported very often is that the most common way for people to use inline handlers is to prevent the default action, which means they never see the double triggering. \ \ If we're going to fix this and continue supporting inline handlers, we have to run them in the trigger loop up front so they can stop propagation and prevent default. In that case we'd need to figure out some way to remove/restore all the inline handlers just like we do for the target -- really messy for something we don't even want people to do. \ \ Either that or we have to eliminate that trigger loop *entirely* for native events (still need it for custom events) and have the browser do the propagation. You're basically proposing that with the `evt.target[event.type]` test. That brings issues like #3287 into play, which would be a major breaking change. \ \ As for capture behavior, jQuery doesn't support it via the external API. It's used in a couple of places for special events (see `focusin` and `focusout`). → 1314717760618358 |
|---|---|
| description: | Looking at the `event.js` file, in the `jQuery.event.trigger` method, it seems that a triggered event is first bubbled manually, then triggered natively to allow default browser behaviours. The problem occurs when you add a listener without `jQuery`, like an inline `onclick` listener. \ \ As I understand it, it kind of looks that way : \ {{{ \ current = evt.target; \ do \ execute current jquery handlers \ execute current inline handler \ current = current.parentNode \ while not evt.isPropagationStopped \ \ if not evt.defaultPrevented \ deactivating jquery handlers \ deactivating evt.target inline handler but not ancestor ones \ evt.target[event.type]() \ }}} \ So, if the event is not `defaultPrevented`, the upward inline handlers are executed twice. If it is, listeners added using native `attachEvent|addEventListener` methods would be triggered but delayed compared to the `jQuery` handlers, and also the `evt.isPropagationStopped` eventually setted by a previous `jQuery` handler would be ignored. \ \ Although the `jQuery.event.triggered` flag, which seems to deactivate the `jQuery` shared handler, can be used in the inline handlers as well, why not doing something like this : \ {{{ \ if evt.target[event.type] \ evt.target[event.type]() \ else \ bubble manually ... \ }}} \ I'm although curious about the way you will handle the `DOM 3` capture behaviour. \ \ Another thought : I can imagine how tough it is to maintain the consistency of the lib, but the more I'm reading it, the more I'm thinking : if I'm building a web app using `jQuery`, I shouldn't use anything else which is not build against the same `jQuery`, otherwise I would encounter strange behaviours. It would be nice to inform users about the interactions and caveats somewhere in the documentation. If it is, my bad, I'll do better reading it next time. \ \ Seems to be related to the following tickets : [ticket: 6593], [ticket: 6705] \ \ Here's the use case : == http://jsfiddle.net/FrKyN/19/ == \ \ Tested using `Chromium 13` and `Aurora 6` → Looking at the `event.js` file, in the `jQuery.event.trigger` method, it seems that a triggered event is first bubbled manually, then triggered natively to allow default browser behaviours. The problem occurs when you add a listener without `jQuery`, like an inline `onclick` listener. \ \ As I understand it, it kind of looks that way : \ {{{ \ current = evt.target; \ do \ execute current jquery handlers \ execute current inline handler \ current = current.parentNode \ while not evt.isPropagationStopped \ \ if not evt.defaultPrevented \ deactivating jquery handlers \ deactivating evt.target inline handler but not ancestor ones \ evt.target[event.type]() \ }}} \ So, if the event is not `defaultPrevented`, the upward inline handlers are executed twice. If it is, listeners added using native `attachEvent|addEventListener` methods would be triggered but delayed compared to the `jQuery` handlers, and also the `evt.isPropagationStopped` eventually setted by a previous `jQuery` handler would be ignored. \ \ Although the `jQuery.event.triggered` flag, which seems to deactivate the `jQuery` shared handler, can be used in the inline handlers as well, why not doing something like this : \ {{{ \ if evt.target[event.type] \ evt.target[event.type]() \ else \ bubble manually ... \ }}} \ I'm although curious about the way you will handle the `DOM 3` capture behaviour. \ \ Another thought : I can imagine how tough it is to maintain the consistency of the lib, but the more I'm reading it, the more I'm thinking : if I'm building a web app using `jQuery`, I shouldn't use anything else which is not build against the same `jQuery`, otherwise I would encounter strange behaviours. It would be nice to inform users about the interactions and caveats somewhere in the documentation. If it is, my bad, I'll do better reading it next time. \ \ Seems to be related to the following tickets : #6593, #6705 \ \ Here's the use case : == http://jsfiddle.net/FrKyN/19/ == \ \ Tested using `Chromium 13` and `Aurora 6` |
| status: | new → open |
Changed August 30, 2011 04:18PM UTC by comment:2
No troll, but I want to make sure this is mentioned: http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers
Changed August 30, 2011 04:32PM UTC by comment:3
Replying to [comment:2 rwaldron]:
No troll, but I want to make sure this is mentioned: http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers
Oops, didn't see that one...
Changed August 30, 2011 04:57PM UTC by comment:4
Thanks for the explanation, I was just surprised by the way it was done, looking at the code. I think I understand the pros and cons of using obstrusive javascript, but it still kinda weird when using the native event listener subscribers. And as jQuery can be used with third party components, it's still something to warn about. I'm still thinking that using the native events (or document.createEvent) when available, for non custom events, would be more appropriate, but facing the major breaking changes which I can't see in all their extents, it's way over my head.
Changed October 13, 2011 04:11AM UTC by comment:5
| component: | unfiled → event |
|---|---|
| priority: | undecided → low |
| resolution: | → wontfix |
| status: | open → closed |
Although I would love to revisit this, the few changes I attempted to make on focus/blur handlers in 1.7 convince me that it won't be possible. One major issue is that if we let the native method fire the event it doesn't always fire the way it does currently. For example, the focus event handlers won't run if the element is already focused or is hidden. Currently we'd always run the handlers even if focusing the element would fail.
Changed October 13, 2011 04:11AM UTC by comment:6
| resolution: | wontfix |
|---|---|
| status: | closed → reopened |
Changed October 13, 2011 04:11AM UTC by comment:7
| owner: | → dmethvin |
|---|---|
| status: | reopened → assigned |
Changed October 13, 2011 04:11AM UTC by comment:8
| resolution: | → wontfix |
|---|---|
| status: | assigned → closed |
Those are some great questions!
Your analysis is basically correct. There are a few related tickets like #3827 as well that show other issues with the
.trigger()design peeking through. I think the general team consensus is that inline handlers are both bad practice and a pain in our butts due to issues like this.I suspect the reason this may not be reported very often is that the most common way for people to use inline handlers is to prevent the default action, which means they never see the double triggering.
If we're going to fix this and continue supporting inline handlers, we have to run them in the trigger loop up front so they can stop propagation and prevent default. In that case we'd need to figure out some way to remove/restore all the inline handlers just like we do for the target -- really messy for something we don't even want people to do.
Either that or we have to eliminate that trigger loop *entirely* for native events (still need it for custom events) and have the browser do the propagation. You're basically proposing that with the
evt.target[event.type]test. That brings issues like #3827 into play, which would be a major breaking change.As for capture behavior, jQuery doesn't support it via the external API. It's used in a couple of places for special events (see
focusinandfocusout).