ngAnimate short circuts Dom removal during remove, causing off by one errors (ng339 error)

[mpolichette]

I think was experiencing a really nasty bug causing an error: TypeError: Cannot read property 'ng339' of undefined

This error happens while removing elements.
I think I've found the reason:

1. While removing elements, angular does a child querySelector of '*' to get all the elements beneath the to be removed element.
   • https://github.com/angular/angular.js/blob/master/src/jqLite.js#L258```
2. It then iterates over that HTMLCollection ('descendants') like an array to remove it's cached data about the element.
3. While removing cached data, "$destroy" events are fired on elements
   • https://github.com/angular/angular.js/blob/master/src/jqLite.js#L305
   • https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L1072
4. $destroy events trigger a cancel() on animations which are currently running, causing them to short circuit their DOM operation
   • https://github.com/angular/angular.js/blob/master/src/ngAnimate/animate.js#L1195

If before this remove an element had started a leave animation, (E.G. by ng-if condition changing), that element will be removed immediately.

• The removal causes the HTMLCollection to remove it also, which mutates the length of the HTMLCollection 'descendants' in the middle of the original removal loop, leading to an off by one error in the looping logic.

In the end, descendants.length is reduced but the loop variable l is kept at the original value. Once the loop variable i is larger than descendants.length, it passes undefined as the element to jqLightRemoveData which tries to access element.ng339
- https://github.com/angular/angular.js/blob/master/src/jqLite.js#L294

I'm not sure what the correct solution would be, as I have not been too involved in the development of angular. I've come up with three possible leads:

• Update our iteration cap if the the HTMLCollection's length changes.
• Some sort of check within ngAnimate to prevent short circuiting if it is going to be removed
• Some pre-check on element before accessing element.ng339

Thanks!
-Matt

---

PR: #8958

[dferrin]

I can confirm this issue using 1.2.23. It also happens on ng-include inside ng-switch.

[matsko]

Can you guys create a plunkr for this please?

[dferrin]

I just realized though that the bug is still happening even though ngAnimate is removed from the list of dependant modules.

[matsko]

Try testing this out using these files:

https://s3.amazonaws.com/angularjs-dev/broken-leave/angular.js
https://s3.amazonaws.com/angularjs-dev/broken-leave/angular-animate.js

[mpolichette]

👍

Using those files solved the problem in my app! Can't wait for it to be included in a release!

Thanks @matsko you rock!