Permalink
Browse files

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
  • Loading branch information...
btford authored and IgorMinar committed Sep 24, 2013
1 parent 6aaae06 commit c785267eb8780d8b7658ef93ebb5ebddd566294d
Showing with 52 additions and 9 deletions.
  1. +12 −6 src/jqLite.js
  2. +6 −2 src/ng/animate.js
  3. +8 −1 test/helpers/matchers.js
  4. +14 −0 test/jqLiteSpec.js
  5. +12 −0 test/ng/animateSpec.js
@@ -279,29 +279,35 @@ 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) + " ", " "))
);
});
}
}

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));
}
}

@@ -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);
},

@@ -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({
@@ -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]);
@@ -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() {

0 comments on commit c785267

Please sign in to comment.