Ember 2.12 action handlers can fire twice for IE and Edge

[workmanw]

Here is a JSBin: http://jsbin.com/cufupaw/edit?html,js,console,output

(JSBin instead of ember-twiddle because of IE)

If you click 'red', 'yellow' or 'blue' in chrome you'll notice the color of the text changes to green to reflect the "selected state", if you try this fiddle in IE it will not change. If you look at the JSBin console pane you'll see that the event handle logged that it was invoked twice. The first time toggled selected: true, the second back to false.

So why did this happen? The reason it happens is because the element with the action handler, also has a class="{{if item.selected 'selected'}}". To understand what's happening you'll need to first look at: event_dispatcher.js. Notice how it's looping over the element's attributes looking for an action handler. So here is the exact order of what happens:

1. Click
2. event_dispatcher finds the element and the action handler at attributes.item(2)
3. action handler is invoked and toggles state, which then changes the DOM by adding a class.
4. That DOM change shifts the action handlers position from attributes.item(2) to attributes.item(3).
5. The action handler returns, the next loop moves to attributes.item(3) finding the exact same handler, and recalls it.

Here is screenshot of the first invocation of the action handler:

Here is a screenshot of the second invocation of the action handler:

This works with Ember 2.11. So somewhere along the way something changed.

[workmanw]

So I bisected this and it turns out to that it was caused by this commit: ce35e6c . Reading my original description and looking at the commit, it's obvious why it broke. But it was also a little bit of a coincidence that it just worked before.

Honestly, I'm not sure why the loop over the element attributes continues after it finds a handler? I guess it's because you could have multiple {{action}} handlers? If that's the case it seems like the list be first filtered to matching action handlers, then each handler should be called.

CC: @GavinJoyce

[rwjblue]

I think the simplest fix would be to track the actions that have already been called during the single loop and if a given action were already ran, noop it.

[workmanw]

@rwjblue I'm 👍 on that. Shall I open a PR?

[rwjblue]

Yep, if you don't mind. Will need a test to help ensure we don't regress again in the future. I believe it should at least be [BUGFIX release].

[workmanw]

@rwjblue Roger that! I'm on it, you'll have a PR tonight because it's an upgrade blocker for us.

Side note, a test would be very easy to write, but does ember.js CI process test IE / Edge? Either way I'll test it, just curious.

[rwjblue]

Yes, we test in IE9, IE10, IE11, and Edge (as well as Chrome, Firefox, Safari, and phantom) in CI.

[Serabe]

@workmanw your PR has been merged. Can this be closed or is there anything left to do? Thank you!

[workmanw]

@Serabe Ooops, I thought it was auto closed with the merge. Sorry about that.