Bug Tracker

Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#10168 closed bug (wontfix)

jQuery.event.trigger and inline handlers.

Reported by: stefen@… Owned by: dmethvin
Priority: low Milestone: None
Component: event Version: 1.6.2
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by dmethvin)

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;
    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

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]
    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

Change History (9)

comment:1 Changed 6 years ago by dmethvin

Description: modified (diff)
Status: newopen

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

Version 0, edited 6 years ago by dmethvin (next)

comment:2 Changed 6 years ago by Rick Waldron

No troll, but I want to make sure this is mentioned: http://docs.jquery.com/Won't_Fix#Inline_Event_Handlers

comment:3 in reply to:  2 Changed 6 years ago by stefen@…

Replying to 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...

comment:4 Changed 6 years ago by stefen@…

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.

comment:5 Changed 6 years ago by dmethvin

Component: unfiledevent
Priority: undecidedlow
Resolution: wontfix
Status: openclosed

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.

comment:6 Changed 6 years ago by dmethvin

Resolution: wontfix
Status: closedreopened

comment:7 Changed 6 years ago by dmethvin

Owner: set to dmethvin
Status: reopenedassigned

comment:8 Changed 6 years ago by dmethvin

Resolution: wontfix
Status: assignedclosed

comment:9 Changed 4 years ago by dmethvin

#14199 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.