Skip to main content

Bug Tracker

Side navigation

#10699 closed bug (wontfix)

Opened November 07, 2011 07:21AM UTC

Closed March 04, 2012 07:48PM UTC

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/

Attachments (0)
Change History (17)

Changed November 07, 2011 03:30PM UTC by dmethvin comment:1

component: unfiledevent
milestone: None1.7.1
owner: → dmethvin
priority: undecidedhigh
status: newassigned

Yep, it was caused by adding support for .stopPropagation on delegated events. Relatively easy to fix though.

Changed November 14, 2011 12:21AM UTC by dmethvin comment:2

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?

Changed November 14, 2011 01:46PM UTC by gyoshev comment:3

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

Changed November 14, 2011 01:52PM UTC by dmethvin comment:4

Can't you use .triggerHandler() for that, since there is no default action for your custom event? That doesn't bubble.

Changed November 14, 2011 03:50PM UTC by anonymous comment:5

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

Changed November 14, 2011 04:05PM UTC by dmethvin comment:6

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?

Changed November 15, 2011 11:13AM UTC by gyoshev comment:7

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?

Changed November 15, 2011 03:16PM UTC by dmethvin comment:8

blockedby: → 10794

Changed November 15, 2011 03:26PM UTC by dmethvin comment:9

resolution: → wontfix
status: assignedclosed

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!

Changed February 13, 2012 11:17PM UTC by maranomynet comment:10

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.

Changed February 13, 2012 11:31PM UTC by dmethvin comment:11

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.

Changed February 14, 2012 06:38AM UTC by maranomynet comment:12

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

Changed February 14, 2012 11:01AM UTC by gyoshev comment:13

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.

Changed February 15, 2012 01:08AM UTC by maranomynet comment:14

gyoshev, thanks for illustrating perfectly what I was pointing out... Working around this bug is a horrible mess.

Please fix the bug.

Changed February 16, 2012 11:31PM UTC by maranomynet comment:15

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.

Changed February 16, 2012 11:43PM UTC by dmethvin comment:16

blockedby: 10794
milestone: 1.7.11.next
priority: highlow
resolution: wontfix
status: closedreopened

@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?

Changed March 04, 2012 07:48PM UTC by dmethvin comment:17

resolution: → wontfix
status: reopenedclosed

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.