diff options
| author | Brian Ford | 2013-09-23 17:29:51 -0700 | 
|---|---|---|
| committer | Igor Minar | 2013-09-27 12:38:27 -0700 | 
| commit | c785267eb8780d8b7658ef93ebb5ebddd566294d (patch) | |
| tree | 46fa85051d7aa768719b8c4572aec63ed30d52e2 | |
| parent | 6aaae062171bfc8e5046c3eae99bc9d63037120a (diff) | |
| download | angular.js-c785267eb8780d8b7658ef93ebb5ebddd566294d.tar.bz2 | |
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
| -rw-r--r-- | src/jqLite.js | 18 | ||||
| -rw-r--r-- | src/ng/animate.js | 8 | ||||
| -rw-r--r-- | test/helpers/matchers.js | 9 | ||||
| -rw-r--r-- | test/jqLiteSpec.js | 14 | ||||
| -rw-r--r-- | test/ng/animateSpec.js | 12 | 
5 files changed, 52 insertions, 9 deletions
| diff --git a/src/jqLite.js b/src/jqLite.js index 1e1d025c..d3110788 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 fbd17848..b3fb08de 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 57bf35c7..14430b37 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 79c0d0c6..e6e3a2ac 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 79115c1e..e982862a 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() { | 
