Skip to content

Commit 1cf35d5

Browse files
fix($animate): invalid CSS class names should not break subsequent elements
The postDigest handler was not being added if the first element in to modify the CSS classes contained invalid CSS class names. This meant that subsequent valid CSS class changes were not being handled since we were not then adding the handler for those correct cases. Closes angular#12674
1 parent 7d2c6ee commit 1cf35d5

File tree

2 files changed

+68
-48
lines changed

2 files changed

+68
-48
lines changed

src/ng/animate.js

+53-48
Original file line numberDiff line numberDiff line change
@@ -105,61 +105,66 @@ var $$CoreAnimateQueueProvider = function() {
105105
}
106106
};
107107

108-
function addRemoveClassesPostDigest(element, add, remove) {
109-
var classVal, data = postDigestQueue.get(element);
110-
111-
if (!data) {
112-
postDigestQueue.put(element, data = {});
113-
postDigestElements.push(element);
114-
}
115-
116-
var updateData = function(classes, value) {
117-
var changed = false;
118-
if (classes) {
119-
classes = isString(classes) ? classes.split(' ') :
120-
isArray(classes) ? classes : [];
121-
forEach(classes, function(className) {
122-
if (className) {
123-
changed = true;
124-
data[className] = value;
108+
function handleClassChanges() {
109+
forEach(postDigestElements, function(element) {
110+
var data = postDigestQueue.get(element);
111+
if (data) {
112+
var existing = splitClasses(element.attr('class'));
113+
var toAdd = '';
114+
var toRemove = '';
115+
forEach(data, function(status, className) {
116+
var hasClass = !!existing[className];
117+
if (status !== hasClass) {
118+
if (status) {
119+
toAdd += (toAdd.length ? ' ' : '') + className;
120+
} else {
121+
toRemove += (toRemove.length ? ' ' : '') + className;
122+
}
125123
}
126124
});
125+
126+
forEach(element, function(elm) {
127+
toAdd && jqLiteAddClass(elm, toAdd);
128+
toRemove && jqLiteRemoveClass(elm, toRemove);
129+
});
130+
postDigestQueue.remove(element);
127131
}
128-
return changed;
129-
};
130-
131-
var classesAdded = updateData(add, true);
132-
var classesRemoved = updateData(remove, false);
133-
if ((!classesAdded && !classesRemoved) || postDigestElements.length > 1) return;
134-
135-
$rootScope.$$postDigest(function() {
136-
forEach(postDigestElements, function(element) {
137-
var data = postDigestQueue.get(element);
138-
if (data) {
139-
var existing = splitClasses(element.attr('class'));
140-
var toAdd = '';
141-
var toRemove = '';
142-
forEach(data, function(status, className) {
143-
var hasClass = !!existing[className];
144-
if (status !== hasClass) {
145-
if (status) {
146-
toAdd += (toAdd.length ? ' ' : '') + className;
147-
} else {
148-
toRemove += (toRemove.length ? ' ' : '') + className;
149-
}
150-
}
151-
});
132+
});
133+
134+
postDigestElements.length = 0;
135+
}
152136

153-
forEach(element, function(elm) {
154-
toAdd && jqLiteAddClass(elm, toAdd);
155-
toRemove && jqLiteRemoveClass(elm, toRemove);
156-
});
157-
postDigestQueue.remove(element);
137+
function updateData(data, classes, value) {
138+
var changed = false;
139+
if (classes) {
140+
classes = isString(classes) ? classes.split(' ') :
141+
isArray(classes) ? classes : [];
142+
forEach(classes, function(className) {
143+
if (className) {
144+
changed = true;
145+
data[className] = value;
158146
}
159147
});
148+
}
149+
return changed;
150+
}
160151

161-
postDigestElements.length = 0;
162-
});
152+
function addRemoveClassesPostDigest(element, add, remove) {
153+
var classVal,
154+
data = postDigestQueue.get(element) || {};
155+
156+
var classesAdded = updateData(data, add, true);
157+
var classesRemoved = updateData(data, remove, false);
158+
159+
if (classesAdded || classesRemoved) {
160+
161+
postDigestQueue.put(element, data);
162+
postDigestElements.push(element);
163+
164+
if (postDigestElements.length === 1) {
165+
$rootScope.$$postDigest(handleClassChanges);
166+
}
167+
}
163168
}
164169
}];
165170
};

test/ng/animateSpec.js

+15
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,21 @@ describe("$animate", function() {
341341
});
342342
});
343343

344+
345+
it('should not break postDigest for subsequent elements if addClass contains non-valid CSS class names', function() {
346+
inject(function($animate, $rootScope, $rootElement) {
347+
var element1 = jqLite('<div></div>');
348+
var element2 = jqLite('<div></div>');
349+
350+
$animate.enter(element1, $rootElement, null, { addClass: ' ' });
351+
$animate.enter(element2, $rootElement, null, { addClass: 'valid-name' });
352+
$rootScope.$digest();
353+
354+
expect(element2.hasClass('valid-name')).toBeTruthy();
355+
});
356+
});
357+
358+
344359
it('should not issue a call to removeClass if the provided class value is not a string or array', function() {
345360
inject(function($animate, $rootScope, $rootElement) {
346361
var spy = spyOn(window, 'jqLiteRemoveClass').andCallThrough();

0 commit comments

Comments
 (0)