SVGAnimatedString handling in animatedRun()

[shirro]

animateRun() in angular-animate.js doesn't handle SVGAnimatedString giving undefined indexOf method messages when changing classes on SVG nodes.

I wasn't confident to submit a pull request but the following patch seems to work around it for me.

--- ../../vendor/angular.js/build/angular-animate.js    2014-01-28 01:45:13.000000000 +1030
+++ angular-animate.js  2014-01-29 10:21:04.000000000 +1030
@@ -1217,7 +1217,11 @@
       function animateRun(element, className, activeAnimationComplete) {
         var elementData = element.data(NG_ANIMATE_CSS_DATA_KEY);
         var node = extractElementNode(element);
-        if(node.className.indexOf(className) == -1 || !elementData) {
+        var nodeClassName = node.className;
+        if (angular.isObject(nodeClassName) && nodeClassName.toString() === '[object SVGAnimatedString]') {
+                  nodeClassName = nodeClassName.baseVal;
+        }
+        if(nodeClassName.indexOf(className) == -1 || !elementData) {
           activeAnimationComplete();
           return;
         }

[caitp]

So did you want to submit a pull request for this? :p I don't know how much ngAnimate is being used for SVG (CSS was not the right word to use here!) animation, but it probably doesn't hurt to support it

[shirro]

@caitp not really. Perhaps someone who is a regular committer could clean it up and do it for me? If I make a habit of hacking on angular I might do it properly next time.

The problem I have is with a heap of ng-class in a ng-repeat in a directive changing classes on certain conditions in an SVG. It works fine until I include the animation module which I wanted for view animations then it starts spitting out indexOf errors in Chrome and Firefox and randomly doesn't make the required class changes.

Edit: And not sure if should be getting baseVal or animVal. I just checked elsewhere in angular and animVal is being used with href elsewhere so perhaps that is the better way.

[matsko]

This code will be changed (animateRun/animateSetup) this week anyways so this issue can be put into consideration.

[shirro]

Thanks. Meanwhile if anyone sees this and wants to avoid the problem just make sure there are no css transitions on svg elements with directives that support ngAnimate. I had a css hover transition on my svg elements which was triggering animateRun though I had no intention of animating those elements with angular.

It is pretty easy to duplicate: http://embed.plnkr.co/ZIhUhkwd1hDH6qOWH5Wk/preview

[mprokopowicz]

Any updates on this one?

[matsko]

Yes. I plan on working on getting this in during this week.

[matsko]

@shirro and @mprokopowicz turns out that all you need is node.getAttribute('class') instead of detecting an SVG element yourself to acquire the className value.

The fix is ready #6973, but I'm not 100% sure if it works since I don't have a visual SVG demo. Please test this out using the files below:

http://ci.angularjs.org/view/Matias'/job/angular.js-matias/704/artifact/build/angular.js http://ci.angularjs.org/view/Matias'/job/angular.js-matias/704/artifact/build/angular-animate.js http://ci.angularjs.org/view/Matias'/job/angular.js-matias/704/artifact/build/angular-route.js

Also, as I'm sure that you already know, you need to include jquery.svg.js to make this work for when you're using jQuery on your page. Unfortunately jQuery alone isn't diverse enough to add and remove CSS classes on SVG DOM elements. Fortunately, jqLite works (that is used when jQuery isn't found on the page and Angular uses its own, internal jQuery-style wrapper).

[matsko]

Fixed. Landed as https://github.com/angular/angular.js/commit/c67bd69c58812da82b1a3a31d430df7aad8a50a8 and https://github.com/angular/angular.js/commit/38ea542662b2b74703d583e3a637d65369fc26eb for 1.2.x.