Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit fe378ea

Browse files
committed
fix(ngAnimate): ensure that only string-based addClass/removeClass values are applied
Closes #12458
1 parent addb1ae commit fe378ea

File tree

4 files changed

+92
-11
lines changed

4 files changed

+92
-11
lines changed

src/ng/animate.js

+12-3
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ var $$CoreAnimateQueueProvider = function() {
107107

108108
function addRemoveClassesPostDigest(element, add, remove) {
109109
var data = postDigestQueue.get(element);
110+
var dataSet = false;
110111
var classVal;
111112

112113
if (!data) {
@@ -115,22 +116,30 @@ var $$CoreAnimateQueueProvider = function() {
115116
}
116117

117118
if (add) {
118-
forEach(add.split(' '), function(className) {
119+
add = isString(add)
120+
? add.split(' ')
121+
: isArray(add) ? add : [];
122+
forEach(add, function(className) {
119123
if (className) {
124+
dataSet = true;
120125
data[className] = true;
121126
}
122127
});
123128
}
124129

125130
if (remove) {
126-
forEach(remove.split(' '), function(className) {
131+
remove = isString(remove)
132+
? remove.split(' ')
133+
: isArray(remove) ? remove : [];
134+
forEach(remove, function(className) {
127135
if (className) {
136+
dataSet = true;
128137
data[className] = false;
129138
}
130139
});
131140
}
132141

133-
if (postDigestElements.length > 1) return;
142+
if (!dataSet || postDigestElements.length > 1) return;
134143

135144
$rootScope.$$postDigest(function() {
136145
forEach(postDigestElements, function(element) {

src/ngAnimate/animateQueue.js

+16-8
Original file line numberDiff line numberDiff line change
@@ -239,22 +239,22 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
239239
// These methods will become available after the digest has passed
240240
var runner = new $$AnimateRunner();
241241

242-
// there are situations where a directive issues an animation for
243-
// a jqLite wrapper that contains only comment nodes... If this
244-
// happens then there is no way we can perform an animation
245-
if (!node) {
246-
close();
247-
return runner;
248-
}
249-
250242
if (isArray(options.addClass)) {
251243
options.addClass = options.addClass.join(' ');
252244
}
253245

246+
if (options.addClass && !isString(options.addClass)) {
247+
options.addClass = null;
248+
}
249+
254250
if (isArray(options.removeClass)) {
255251
options.removeClass = options.removeClass.join(' ');
256252
}
257253

254+
if (options.removeClass && !isString(options.removeClass)) {
255+
options.removeClass = null;
256+
}
257+
258258
if (options.from && !isObject(options.from)) {
259259
options.from = null;
260260
}
@@ -263,6 +263,14 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
263263
options.to = null;
264264
}
265265

266+
// there are situations where a directive issues an animation for
267+
// a jqLite wrapper that contains only comment nodes... If this
268+
// happens then there is no way we can perform an animation
269+
if (!node) {
270+
close();
271+
return runner;
272+
}
273+
266274
var className = [node.className, options.addClass, options.removeClass].join(' ');
267275
if (!isAnimatableClassName(className)) {
268276
close();

test/ng/animateSpec.js

+35
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,41 @@ describe("$animate", function() {
320320
});
321321
});
322322

323+
they('should not issue a call to $prop if the provided class value is not a string or array', ['addClass', 'removeClass'], function(prop) {
324+
inject(function($animate, $rootScope, $rootElement) {
325+
var spyProp = prop === 'addClass' ? 'jqLiteAddClass' : 'jqLiteRemoveClass';
326+
var spy = spyOn(window, spyProp).andCallThrough();
327+
328+
var element = jqLite('<div></div>');
329+
var parent = $rootElement;
330+
331+
var options1 = {};
332+
options1[prop] = function() {};
333+
$animate.enter(element, parent, null, options1);
334+
335+
$rootScope.$digest();
336+
expect(spy).not.toHaveBeenCalled();
337+
338+
var options2 = {};
339+
options2[prop] = true;
340+
$animate.leave(element, options2);
341+
342+
$rootScope.$digest();
343+
expect(spy).not.toHaveBeenCalled();
344+
345+
var options3 = {};
346+
if (prop === 'removeClass') {
347+
element.addClass('fatias');
348+
}
349+
350+
options3[prop] = ['fatias'];
351+
$animate.enter(element, parent, null, options3);
352+
353+
$rootScope.$digest();
354+
expect(spy).toHaveBeenCalled();
355+
});
356+
});
357+
323358
describe('CSS class DOM manipulation', function() {
324359
var element;
325360
var addClass;

test/ngAnimate/animateSpec.js

+29
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,35 @@ describe("animations", function() {
148148
});
149149
});
150150

151+
they('should nullify both options.$prop when passed into an animation if it is not a string or an array', ['addClass', 'removeClass'], function(prop) {
152+
inject(function($animate, $rootScope) {
153+
var options1 = {};
154+
options1[prop] = function() {};
155+
$animate.enter(element, parent, null, options1);
156+
157+
expect(options1[prop]).toBeFalsy();
158+
$rootScope.$digest();
159+
160+
var options2 = {};
161+
options2[prop] = true;
162+
$animate.leave(element, options2);
163+
164+
expect(options2[prop]).toBeFalsy();
165+
$rootScope.$digest();
166+
167+
capturedAnimation = null;
168+
169+
var options3 = {};
170+
if (prop === 'removeClass') {
171+
element.addClass('fatias');
172+
}
173+
174+
options3[prop] = ['fatias'];
175+
$animate.enter(element, parent, null, options3);
176+
expect(options3[prop]).toBe('fatias');
177+
});
178+
});
179+
151180
it('should throw a minErr if a regex value is used which partially contains or fully matches the `ng-animate` CSS class', function() {
152181
module(function($animateProvider) {
153182
assertError(/ng-animate/, true);

0 commit comments

Comments
 (0)