Animations: animation code is executed for Comment Nodes, throwing exceptions

[ZissisT]

Hello,

in angular-animate.js:

animateSetup() method and animateRun() method do not check if current node is an Element Node and try to apply styles there, throwing exceptions (no style attribute to set style property). This happens on Comment Nodes (e.g. nodes that were commented out by ng-if ). A fix was added in 1.2 for the same issue in cancelChildAnimations() but I guess it's needed in the other 2 methods as well (issue found in latest 1.2.1 version stable version)

The fix was

    var node = element[0];
    if(node.nodeType != ELEMENT_NODE) {
      return;
    }

in the beginning of every method

Zissis

[matsko]

There are a few other issues related to this and I'm planning on rewriting parts of ngAnimate to properly handle cases where comments are encountered. There's currently one big animation PR that will go into 1.2.2. After that I will get on this one :)

[matsko]

Fixed with:

https://s3.amazonaws.com/angularjs-dev/ng-animate-comment-nodes/build/angular.js
https://s3.amazonaws.com/angularjs-dev/ng-animate-comment-nodes/build/angular-animate.js
https://s3.amazonaws.com/angularjs-dev/ng-animate-comment-nodes/build/angular-route.js

You can expect this with AngularJS 1.2.4.

[ZissisT]

Thank you very much !

Zissis

[Pyppe]

Added a comment for commit: 958d3d5#commitcomment-4869962

I'm seeing exceptions due to node being undefined within the cancelChildAnimations function.

[matsko]

Herra @Pyppe, do you have an example of the code that was causing the issue? It may be a mixture of some directives that may be causing the bug. It would be better to see if there is a fix for the instead of patching $animate. Kiitos :)

[AGiorgetti]

Hello, I've faced the same problem and indeed the 'extractElementNode' function in angular-animate can return 'undefined'. My templates get parsed like this:

                    <!--: !visibleIfNotActive --><!-- ngInclude: 'pediatria/v/xxx.html' --><!-- end ngIf: !visibleIfNotActive -->
                    <!-- ngIf: visibleIfNotActive -->
                    <!-- ngIf: visibleIfNotActive -->

That's the result of having used nested directive that use transclusion.

Also looking at the first lines of the 'performAnimations' function we can read:

var node = extractElementNode(element);
//transcluded directives may sometimes fire an animation using only comment nodes
//best to catch this early on to prevent any animation operations from occurring
if(!node) {
        ...
          return;
        }

so it seems a quite common scenario, my quick fix was to check for the return value of the extractElementNode function in the cancelChildAnimations too, something like:

function cancelChildAnimations(element) {
          var node = extractElementNode(element);
          //transcluded directives may sometimes fire an animation using only comment nodes
          //best to catch this early on to prevent any animation operations from occurring
          if (!node)
              return;
        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);
          }
        });
      }

sorry for the long post.

[matsko]

@AGiorgetti yeah I can definitely see what's causing it. Can you paste your HTML (the template) + JS (the controller) code so that I can reproduce a bug so that I can create a matching test for the fix?

[AGiorgetti]

Hi, sorry for the late reply, I'll try to 'extract' the parts that you can use to reproduce the issue in this plunkr:

http://plnkr.co/edit/rJWdiIfyiUYwwoJSlQWU

The templates here are not optimized in any way yet ( :D )

If you run it and navigate su View1 in normal conditions you should see a sequence of subview: subview2,3,1,2,3,1,2,3.

Sometimes especially on page refresh (due to the error of the animation) the sequence contains as first element a subview1 which should be hidden.

This doesn't seem to happen in the few tries I did with plunker, but it happens a lot in my production code.

[AGiorgetti]

Hi, this is the whole stacktrace of the error (if it can help)

TypeError: Cannot call method 'querySelectorAll' of undefined
at cancelChildAnimations (http://localhost:5733/Scripts/angular-animate.js:802:22)
at Object.leave (http://localhost:5733/Scripts/angular-animate.js:418:11)
at Object.ngIfWatchAction as fn
at Scope.$digest (http://localhost:5733/Scripts/angular.js:11761:29)
at Scope.$apply (http://localhost:5733/Scripts/angular.js:12012:24)
at Scope.$rootScope.$safeApply (http://localhost:5733/Scripts/Scope.SafeApply.js:45:30)
at Object. (http://localhost:5733/angularjs/services/signalrHubProxy.js:37:44)
at fire (http://localhost:5733/Scripts/jquery-1.10.2.js:3048:30)
at Object.self.fireWith as resolveWith
at Object.callback (http://localhost:5733/Scripts/jquery.signalR-1.2.0.js:2359:27) undefined angular.js:9383
(anonymous function) angular.js:9383
(anonymous function) angular.js:6825
(anonymous function) app.js:364
Scope.$digest angular.js:11780
Scope.$apply angular.js:12012
$rootScope.$safeApply Scope.SafeApply.js:45
(anonymous function) signalrHubProxy.js:37
fire jquery-1.10.2.js:3048
self.fireWith jquery-1.10.2.js:3160
callback jquery.signalR-1.2.0.js:2359
(anonymous function) jquery.signalR-1.2.0.js:2445
(anonymous function) jquery.signalR-1.2.0.js:681
jQuery.event.dispatch jquery-1.10.2.js:5095
elemData.handle jquery-1.10.2.js:4766
jQuery.event.trigger jquery-1.10.2.js:5007
jQuery.event.trigger jquery-migrate-1.2.1.js:493
jQuery.fn.extend.triggerHandler jquery-1.10.2.js:5697
$.ajax.$.extend.success jquery.signalR-1.2.0.js:1088
fire jquery-1.10.2.js:3048
self.fireWith jquery-1.10.2.js:3160
done jquery-1.10.2.js:8235
callback jquery-1.10.2.js:8778

[Burgov]

The problem seems to appear when using ng-repeat in combination with ng-if:

http://plnkr.co/edit/RkoaKDOEXYtcAsDrcg4Z?p=preview

The problem in the example above occurs when you click the button and the filter is applied.

Removing the ngAnimate module from the app module solves the problem (but removes the animations, of course)

[rinatio]

Having same exception with ng-repeat and ng-if in one tag

[rinatio]

Also getting this exception for ng-if and ng-include, when ng-if is false:

<div ng-if="false" ng-include="'views/navbar.html'"></div>

[r3m0t]

I am getting this when putting ngIf and ngSwitchWhen on the same tag.

[kendugu]

Same issue with ng-repeat and ng-if for me

[fkarlsson]

I've got the same issue as @kendugu - how do I work around this?

[kendugu]

I removed the ngAnimate module for now, that's the simplest workaround.

[davidfarina]

A clean workaround would be to use a filter in the ng-repeat:

ng-repeat="object in objectList | filter: filterByObjectType" and use $scope.filterByObjectType = function(object) { return object.type === 'Category A' } to use a custom function (if access to nested objects is required) or

ng-repeat="object in objectList | filter: {type : 'Category A'}" for direct use

[matsko]

Fixed in master: c914cd9

[matsko]

Expect this for 1.2.14 (tomorrow or Friday).

[matsko]

Still idle on the release of 1.2.14. But this is sitting in master so you can use the snapshot version from code.angularjs.org if you want.

[kentcdodds]

I can confirm that @matsko's fix in 1.2.14 fixed my problem with this issue 👍 Thanks!