From c785267eb8780d8b7658ef93ebb5ebddd566294d Mon Sep 17 00:00:00 2001
From: Brian Ford <btford@umich.edu>
Date: Mon, 23 Sep 2013 17:29:51 -0700
Subject: [PATCH] fix(jqLite): use get/setAttribute so that jqLite works on SVG
 nodes

jqLite previously used `elt.className` to add and remove classes from a DOM Node, but
because the className property is not writable on SVG elements, it doesn't work with
them. This patch replaces accesses to `className` with `get/setAttribute`.

`classList` was also considered as a solution, but because only IE10+ supports it, we
have to wait. :'(

The JqLiteAddClass/JQLiteRemoveClass methods are now also used directly by $animate
to work around the jQuery not being able to handle class modifications on SVG elements.

Closes #3858
---
 src/jqLite.js            | 18 ++++++++++++------
 src/ng/animate.js        |  8 ++++++--
 test/helpers/matchers.js |  9 ++++++++-
 test/jqLiteSpec.js       | 14 ++++++++++++++
 test/ng/animateSpec.js   | 12 ++++++++++++
 5 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/src/jqLite.js b/src/jqLite.js
index 1e1d025ca826..d3110788b3a6 100644
--- a/src/jqLite.js
+++ b/src/jqLite.js
@@ -279,17 +279,17 @@ function JQLiteData(element, key, value) {
 }
 
 function JQLiteHasClass(element, selector) {
-  return ((" " + element.className + " ").replace(/[\n\t]/g, " ").
+  return ((" " + (element.getAttribute('class') || '') + " ").replace(/[\n\t]/g, " ").
       indexOf( " " + selector + " " ) > -1);
 }
 
 function JQLiteRemoveClass(element, cssClasses) {
   if (cssClasses) {
     forEach(cssClasses.split(' '), function(cssClass) {
-      element.className = trim(
-          (" " + element.className + " ")
+      element.setAttribute('class', trim(
+          (" " + (element.getAttribute('class') || '') + " ")
           .replace(/[\n\t]/g, " ")
-          .replace(" " + trim(cssClass) + " ", " ")
+          .replace(" " + trim(cssClass) + " ", " "))
       );
     });
   }
@@ -297,11 +297,17 @@ function JQLiteRemoveClass(element, cssClasses) {
 
 function JQLiteAddClass(element, cssClasses) {
   if (cssClasses) {
+    var existingClasses = (' ' + (element.getAttribute('class') || '') + ' ')
+                            .replace(/[\n\t]/g, " ");
+
     forEach(cssClasses.split(' '), function(cssClass) {
-      if (!JQLiteHasClass(element, cssClass)) {
-        element.className = trim(element.className + ' ' + trim(cssClass));
+      cssClass = trim(cssClass);
+      if (existingClasses.indexOf(' ' + cssClass + ' ') === -1) {
+        existingClasses += cssClass + ' ';
       }
     });
+
+    element.setAttribute('class', trim(existingClasses));
   }
 }
 
diff --git a/src/ng/animate.js b/src/ng/animate.js
index fbd17848eba8..b3fb08def812 100644
--- a/src/ng/animate.js
+++ b/src/ng/animate.js
@@ -156,7 +156,9 @@ var $AnimateProvider = ['$provide', function($provide) {
         className = isString(className) ?
                       className :
                       isArray(className) ? className.join(' ') : '';
-        element.addClass(className);
+        forEach(element, function (element) {
+          JQLiteAddClass(element, className);
+        });
         done && $timeout(done, 0, false);
       },
 
@@ -177,7 +179,9 @@ var $AnimateProvider = ['$provide', function($provide) {
         className = isString(className) ?
                       className :
                       isArray(className) ? className.join(' ') : '';
-        element.removeClass(className);
+        forEach(element, function (element) {
+          JQLiteRemoveClass(element, className);
+        });
         done && $timeout(done, 0, false);
       },
 
diff --git a/test/helpers/matchers.js b/test/helpers/matchers.js
index 57bf35c7cd9e..14430b374a75 100644
--- a/test/helpers/matchers.js
+++ b/test/helpers/matchers.js
@@ -31,7 +31,14 @@ beforeEach(function() {
   }
 
   function isNgElementHidden(element) {
-    return angular.element(element).hasClass('ng-hide');
+    // we need to check element.getAttribute for SVG nodes
+    var hidden = true;
+    forEach(angular.element(element), function (element) {
+      if ((' ' +(element.getAttribute('class') || '') + ' ').indexOf(' ng-hide ') === -1) {
+        hidden = false;
+      }
+    });
+    return hidden;
   };
 
   this.addMatchers({
diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js
index 79c0d0c6a433..e6e3a2ac3570 100644
--- a/test/jqLiteSpec.js
+++ b/test/jqLiteSpec.js
@@ -479,6 +479,20 @@ describe('jqLite', function() {
 
   describe('class', function() {
 
+    it('should properly do  with SVG elements', function() {
+      // this is a jqLite & SVG only test (jquery doesn't behave this way right now, which is a bug)
+      if (!window.SVGElement || !_jqLiteMode) return;
+      var svg = jqLite('<svg><rect></rect></svg>');
+      var rect = svg.children();
+
+      expect(rect.hasClass('foo-class')).toBe(false);
+      rect.addClass('foo-class');
+      expect(rect.hasClass('foo-class')).toBe(true);
+      rect.removeClass('foo-class');
+      expect(rect.hasClass('foo-class')).toBe(false);
+    });
+
+
     describe('hasClass', function() {
       it('should check class', function() {
         var selector = jqLite([a, b]);
diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js
index 79115c1e8e4c..e982862aed60 100644
--- a/test/ng/animateSpec.js
+++ b/test/ng/animateSpec.js
@@ -40,6 +40,18 @@ describe("$animate", function() {
       expect(element).toBeHidden();
     }));
 
+    it("should add and remove classes on SVG elements", inject(function($animate) {
+      if (!window.SVGElement) return;
+      var svg = jqLite('<svg><rect></rect></svg>');
+      var rect = svg.children();
+      $animate.enabled(false);
+      expect(rect).toBeShown();
+      $animate.addClass(rect, 'ng-hide');
+      expect(rect).toBeHidden();
+      $animate.removeClass(rect, 'ng-hide');
+      expect(rect).not.toBeHidden();
+    }));
+
     it("should throw error on wrong selector", function() {
       module(function($animateProvider) {
         expect(function() {