ngAnimate throws exception in cancelChildAnimations on deletion of repeated element when using ng-include

[gregolsky]

Here's plunker http://plnkr.co/edit/HrmBBmz949YnS7IcPyp7?p=preview
Please try to click one of the "Remove this" links.
I attach stack trace just in case.

TypeError: Object # has no method 'querySelectorAll'
at cancelChildAnimations (http://code.angularjs.org/1.2.0-rc.3/angular-animate.js:593:36)
at Object.leave (http://code.angularjs.org/1.2.0-rc.3/angular-animate.js:332:11)
at ngRepeatAction (http://code.angularjs.org/1.2.0-rc.3/angular.js:17770:24)
at Object.$watchCollectionAction as fn
at Scope.$digest (http://code.angularjs.org/1.2.0-rc.3/angular.js:10569:27)
at Scope.$apply (http://code.angularjs.org/1.2.0-rc.3/angular.js:10802:24)
at HTMLAnchorElement. (http://code.angularjs.org/1.2.0-rc.3/angular.js:16514:17)
at http://code.angularjs.org/1.2.0-rc.3/angular.js:2331:10
at Array.forEach (native)
at forEach (http://code.angularjs.org/1.2.0-rc.3/angular.js:213:11)

[gregolsky]

The error occurs, when the "element" argument passed to the cancelChildAnimations function contains comment element (at index 0), which does not have the querySelectorAll method. The fix may be to check, if the element supports the method in question.

[matsko]

I see what's going on here. I also got your email yesterday outlining the issue. Thanks for sending that over, I wouldn't have noticed this as quickly.

Checking for querySelectorAll does fix it, but you want the code to be as informative as possible. What's going on is that the comment node (which ngInclude uses) is being animated and that's where it breaks.

So just check for element[0].nodeType == ELEMENT_NODE.

Can you update your pull request to handle that check? Basically something like:

function cancelChildAnimations(element) {
  var ELEMENT_NODE = 1;
  var node = element[0];
  if(node.nodeType == ELEMENT_NODE) {
    angular.forEach(node.querySelectorAll('.' + NG_ANIMATE_CLASS_NAME), function(element) {
      element = angular.element(element);
      var data = element.data(NG_ANIMATE_STATE);
      if(data) {
        cancelAnimations(data.animations);
        cleanup(element);
      }
    });
  }
}

Once ready I can merge it in directly.

[matsko]

Landed as b9557b0

[thaoula]

Hi Guys,

This error is still happening in version 1.2.10 and every version since the issue was reported.

I always have to check if the following line is undefined -

function cancelChildAnimations(element) {
var node = extractElementNode(element);

Regards,
Tarek

[caitp]

@thaoula, can you provide some information on which nodes you're running into this on? It wouldn't totally surprise me if certain XML interfaces didn't implement this

[matsko]

This is a common issue with ngAnimate and this PR fixes it: #5570.

Waiting to merge in the class-based animation fix first since it has a bit of an internal refactor. That will happen next week.

[caitp]

I thought the nodeType test in cancelChildAnimations was already merged.

[matsko]

The problem occurs when an empty jqLite element shows up (when all it has are comment nodes within and no element node). The PR fixes it in a way that avoids the constant checking for comment nodes during the animations.

[robations]

I know this issue is old and closed but I'm still seeing this issue with v1.3.15 and OS X Safari.

The problem seems to be element passed from cancelChildAnimations --> extractElementNode is an array-like object (it has a length) but the keys are not numerical:

Object.keys(element)
["1", "length", "[object HTMLLabelElement]"]

element.length
2

Obviously iterating through this using a for-loop fails. element["1"] is a comment node created by ngRepeat.