Opened 11 years ago
Closed 11 years ago
#10699 closed bug (wontfix)
Calling stopPropagation() before trigger() doesn't call handlers on element
Reported by: | gyoshev | Owned by: | dmethvin |
---|---|---|---|
Priority: | low | Milestone: | 1.next |
Component: | event | Version: | git |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
Calling stopPropagation() on an event object before triggering it doesn't call any immediate event handlers. This works for any jQuery version before 1.7. Test case: http://jsfiddle.net/UG7bF/
Change History (17)
comment:1 Changed 11 years ago by
Component: | unfiled → event |
---|---|
Milestone: | None → 1.7.1 |
Owner: | set to dmethvin |
Priority: | undecided → high |
Status: | new → assigned |
comment:2 follow-up: 3 Changed 11 years ago by
Hmmm, looking at this some more it isn't as easy to fix as I had thought. Can you describe the situation where you have an event with propagation stopped that you want to trigger?
comment:3 Changed 11 years ago by
Replying to dmethvin:
Hmmm, looking at this some more it isn't as easy to fix as I had thought. Can you describe the situation where you have an event with propagation stopped that you want to trigger?
We're using that for custom events on nested components. For example, if you have a tabstrip within a tabstrip and want to trigger a custom event (i.e. "select" for selecting a tab) only on the nested component. If the event propagates, it would trigger handlers on the outer component. On the other hand, if the event doesn't trigger at all, none of its immediate event handlers will be executed.
comment:4 Changed 11 years ago by
Can't you use .triggerHandler() for that, since there is no default action for your custom event? That doesn't bubble.
comment:5 Changed 11 years ago by
Just tried it out -- unfortunately, triggerHandler internally calls preventDefault(), which in turn reports that every triggered custom event has a prevented default action. That is not desired, as no user code has done this. Consider the following:
var e = $.extend({ url: "/foo" }, new $.Event("dataRequest")); $(element).triggerHandler(e); // now e.isDefaultPrevented() == true, yet none of the handlers // has prevented the dataRequest
comment:6 Changed 11 years ago by
Yeah, I agree with that and it's a legitimate use case. I wonder, however, if that should be considered a bug with triggerHandler, which doesn't document that it has a side-effect like that. It's pretty easy to fix that in .trigger() by not calling event.preventDefault() and checking the onlyHandlers param before trying the default action.
What do you think?
comment:7 Changed 11 years ago by
Sure, that should do the trick -- I just tried it out with our codebase and it works perfectly. Plus, triggerHandler() seems more appropriate in this use case. Great thing about the fix is that it actually results in a better jQuery size (one if
less), so bytes are actually removed when fixing it. Should the side-effect be logged as a new bug so that it gets to the release notes for 1.7.1?
comment:8 Changed 11 years ago by
Blocked by: | 10794 added |
---|
comment:9 Changed 11 years ago by
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Yes, I agree, it seems logically right and keeping the code small doesn't hurt either. This is a behavior change, but I don't think it will be very common and we've provided a better solution by fixing .triggerHandler()
.
Thanks for your help and advice on this. I've opened #10794 and will get it patched for 1.7.1!
comment:10 Changed 11 years ago by
So we need to change our code from using a simple .trigger(ev)
to .each(function(){ $(this).triggerHandler(ev); });
This seems like a woefully inelegant workaround for this bug.
You should at the very least document this behavioral oddity that a early .stopPropagation()
effectively *disables* the Event.
comment:11 Changed 11 years ago by
This case isn't something that can ever happen naturally; an event is never delivered from the browser with propagation initially stopped. If you just want to run handlers because it's not a browser-related event, then .triggerHandler()
does seem like the correct choice here, especially since .trigger()
will attempt to run a method with the same name as the event.
comment:12 Changed 11 years ago by
Actually it's worse than it appears.
Wontfixing this bug along with bug #10794 causes an unfortunate conflict where I can't fix my existing code to work with jQuery 1.7.1 without breaking backwards compatibility with older jQuery versions. (Which I can't guarantee will never be used in production.)
So now I have to fork my code based on jQuery versions.
Moreover, I have several cases where .trigger()
is what I really really want, and I wanna do .stopPropagation()
only when plugin's options.noBubble
is set to true
.
Again I need to fork the whole .trigger
ing mechanism based on the value of .noBubble
.
Pretty please consider reopening and fixing this bug.
jQuery.Event()
is such a neat method and offers such a clean way for advanced triggering of events (custom or not). It a shame to see this bug ruin its elegance and make it all kludgy, all of a sudden.
comment:13 Changed 11 years ago by
You can do a test of what the jQuery version supports the fixed triggerHandler or trigger+stopPropagation -- that's what we did in the end:
var useTriggerHandler = (function(){ var e = new $.Event("triggerHandlerTest"); $("<div />").triggerHandler(e); return !e.isDefaultPrevented(); })(); function trigger(element, type, e) { e = $.extend(e || {}, new $.Event(type)); if (useTriggerHandler) { $(element).triggerHandler(e); } else { e.stopPropagation(); $(element).trigger(e); } return e.isDefaultPrevented(); }
Theoretically, you can modify this to make it depend on your plugin options.
It introduces a fork in your code if you decide wish to be backwards-compatible, which is usually the case with plug-ins.
comment:14 Changed 11 years ago by
gyoshev, thanks for illustrating perfectly what I was pointing out... Working around this bug is a horrible mess.
Please fix the bug.
comment:15 Changed 11 years ago by
Here's my actual use-case, and I hope you consider it a valid one:
"I'd like to create and trigger a *native* event, but I don't want it to bubble up the DOM, and I want the handlers to be able to determine that via .isPropagationStopped()."
As of jQuery 1.7.1 there is *no* workable way to accomplish this.
comment:16 Changed 11 years ago by
Blocked by: | 10794 removed |
---|---|
Milestone: | 1.7.1 → 1.next |
Priority: | high → low |
Resolution: | wontfix |
Status: | closed → reopened |
@maranomynet, I closed this ticket because I thought we'd addressed the common use cases. If I am to interpret your snarky posts and tweets, we haven't addressed a use case that is important to you.
Can you describe your situation a bit more? Is there a reason why the handlers themselves cannot stop propagation?
comment:17 Changed 11 years ago by
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
The ticket's been open for a while with no discussion, so I will close it again.
The reason this is difficult to revert is that we need to take delegated events into account. If a synthesized event arrives at jQuery.event.dispatch
with propagation stopped, there may be delegated events. Should we run those, even though propagation is supposedly stopped? If so, why are we then running the non-delegated events for this element and only then stopping? This kind of behavior is never caused by the browser since it won't ever call our handler with propagation stopped.
Yep, it was caused by adding support for .stopPropagation on delegated events. Relatively easy to fix though.