Splattributes, {{action}} modifier and component event handlers

[chancancode]

See https://github.com/emberjs/ember.js/pull/17874#discussion_r272809665

As part of the fix we need to decide which one fires first, since it was not specified in the RFC. My gut feeling is that the component’s handler should fire first and given the option to cancel it.

[chancancode]

@simonihmig do you want to work on this one too?

[chancancode]

This one is [BUGFIX canary] only, the feature was added after the beta branch point.

/cc @cibernox

[simonihmig]

do you want to work on this one too?

I can do it, but probably only more towards the end of the week...

My gut feeling is that the component’s handler should fire first and given the option to cancel it.

Agree on the order. Not sure about the cancelation.

Currently returning false means we do event.stopPropagation() (or look into event.cancelBubble with the recent changes to see if the user called event.stopPropagation()), and prevent the event to bubble up within our own custom bubbling up implementation.

But here we are dealing with the case that we have multiple listeners on the same element, which event.stopPropagation() does not cancel. event.stopImmediatePropagation() would do that. But not sure if returning false should have stopImmediatePropagation() semantics? And when the user has called stopImmediatePropagation(), we have no way to find that out, as there seems no equivalent of cancelBubble for that case, AFAICT.

So not really sure if we can/should allow event cancelation for all the listeners of the same element here? Which is really ironic, as this was exactly the example I brought up as a "dangerous" thing during the RFC process! 🤷‍♂️😂

[chancancode]

But not sure if returning false should have stopImmediatePropagation() semantics?

I think it should not. return false is modeled after the jQuery semantics, which I believe is event.stopPropagation() not event.stopImmediatePropagation().

And when the user has called stopImmediatePropagation(), we have no way to find that out, as there seems no equivalent of cancelBubble for that case, AFAICT.

That's a good point 🤔

So not really sure if we can/should allow event cancelation for all the listeners of the same element here?

I don't "not allowing" it is not really an option, I think the main decision is whether the action handlers get run before or after the component handler. If it's after, then stopPropagation === stopImmediatePropagation, so that's a bit easier for us. Maybe that's our only option anyway?

[chancancode]

The {{on ...}} thing your brought up in the RFC actually has an even worse problem – because it uses addEventListener on the underlying element, it will not go through the EventDispatcher and so it will always fire first. But that is a general problem anyway, you can observe that difference by passing {{on ...}} and {{action ...}} on a regular element.

[kategengler]

I think the window for fixing this has flown. Probably the best we can do now is document what order they do fire in.