diff --git a/src/ng/animateCss.js b/src/ng/animateCss.js index 5aaf78bea637..3d5016170588 100644 --- a/src/ng/animateCss.js +++ b/src/ng/animateCss.js @@ -42,7 +42,12 @@ var $CoreAnimateCssProvider = function() { } }; - return function(element, options) { + return function(element, initialOptions) { + // we always make a copy of the options since + // there should never be any side effects on + // the input data when running `$animateCss`. + var options = copy(initialOptions); + // there is no point in applying the styles since // there is no animation that goes on at all in // this version of $animateCss. diff --git a/src/ngAnimate/.jshintrc b/src/ngAnimate/.jshintrc index ee79a690d2a8..187ecd1f3c9d 100644 --- a/src/ngAnimate/.jshintrc +++ b/src/ngAnimate/.jshintrc @@ -7,6 +7,7 @@ "angular": false, "noop": false, + "copy": false, "forEach": false, "extend": false, "jqLite": false, diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js index 3b61bec69c2e..9a0548fd0142 100644 --- a/src/ngAnimate/animateCss.js +++ b/src/ngAnimate/animateCss.js @@ -446,7 +446,12 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { return timings; } - return function init(element, options) { + return function init(element, initialOptions) { + // we always make a copy of the options since + // there should never be any side effects on + // the input data when running `$animateCss`. + var options = copy(initialOptions); + var restoreStyles = {}; var node = getDomNode(element); if (!node diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index 3345dce7691d..4420c4d2001d 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -235,7 +235,12 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { } }; - function queueAnimation(element, event, options) { + function queueAnimation(element, event, initialOptions) { + // we always make a copy of the options since + // there should never be any side effects on + // the input data when running `$animateCss`. + var options = copy(initialOptions); + var node, parent; element = stripCommentsFromElement(element); if (element) { diff --git a/src/ngAnimate/shared.js b/src/ngAnimate/shared.js index ad58a1506cab..634cd90bada4 100644 --- a/src/ngAnimate/shared.js +++ b/src/ngAnimate/shared.js @@ -2,6 +2,7 @@ /* jshint ignore:start */ var noop = angular.noop; +var copy = angular.copy; var extend = angular.extend; var jqLite = angular.element; var forEach = angular.forEach; diff --git a/test/ng/animateCssSpec.js b/test/ng/animateCssSpec.js index cce232e960af..f0a027805cfa 100644 --- a/test/ng/animateCssSpec.js +++ b/test/ng/animateCssSpec.js @@ -16,6 +16,21 @@ describe("$animateCss", function() { describe("without animation", function() { + it("should not alter the provided options input in any way", inject(function($animateCss) { + var initialOptions = { + from: { height: '50px' }, + to: { width: '50px' }, + addClass: 'one', + removeClass: 'two' + }; + + var copiedOptions = copy(initialOptions); + + expect(copiedOptions).toEqual(initialOptions); + $animateCss(element, copiedOptions).start(); + expect(copiedOptions).toEqual(initialOptions); + })); + it("should apply the provided [from] CSS to the element", inject(function($animateCss) { $animateCss(element, { from: { height: '50px' }}).start(); expect(element.css('height')).toBe('50px'); diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js index 76fae5ebcaa6..8c913114493a 100644 --- a/test/ng/animateSpec.js +++ b/test/ng/animateSpec.js @@ -378,6 +378,27 @@ describe("$animate", function() { }); }); + it("should not alter the provided options input in any way throughout the animation", inject(function($animate, $rootElement, $rootScope) { + var element = jqLite('
'); + var parent = $rootElement; + + var initialOptions = { + from: { height: '50px' }, + to: { width: '50px' }, + addClass: 'one', + removeClass: 'two' + }; + + var copiedOptions = copy(initialOptions); + expect(copiedOptions).toEqual(initialOptions); + + var runner = $animate.enter(element, parent, null, copiedOptions); + expect(copiedOptions).toEqual(initialOptions); + + $rootScope.$digest(); + expect(copiedOptions).toEqual(initialOptions); + })); + describe('CSS class DOM manipulation', function() { var element; var addClass; diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index a708d0f6e888..d0bd8d77e8a9 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -1798,6 +1798,37 @@ describe("ngAnimate $animateCss", function() { }; })); + it("should not alter the provided options input in any way throughout the animation", inject(function($animateCss) { + var initialOptions = { + from: { height: '50px' }, + to: { width: '50px' }, + addClass: 'one', + removeClass: 'two', + duration: 10, + delay: 10, + structural: true, + keyframeStyle: '1s rotate', + transitionStyle: '1s linear', + stagger: 0.5, + staggerIndex: 3 + }; + + var copiedOptions = copy(initialOptions); + expect(copiedOptions).toEqual(initialOptions); + + var animator = $animateCss(element, copiedOptions); + expect(copiedOptions).toEqual(initialOptions); + + var runner = animator.start(); + expect(copiedOptions).toEqual(initialOptions); + + triggerAnimationStartFrame(); + expect(copiedOptions).toEqual(initialOptions); + + runner.end(); + expect(copiedOptions).toEqual(initialOptions); + })); + describe("[$$skipPreparationClasses]", function() { it('should not apply and remove the preparation classes to the element when true', inject(function($animateCss) { diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index c08808150420..7a746304d13a 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -130,6 +130,24 @@ describe("animations", function() { }; })); + it("should not alter the provided options input in any way throughout the animation", inject(function($animate, $rootElement, $rootScope) { + var initialOptions = { + from: { height: '50px' }, + to: { width: '50px' }, + addClass: 'one', + removeClass: 'two' + }; + + var copiedOptions = copy(initialOptions); + expect(copiedOptions).toEqual(initialOptions); + + var runner = $animate.enter(element, parent, null, copiedOptions); + expect(copiedOptions).toEqual(initialOptions); + + $rootScope.$digest(); + expect(copiedOptions).toEqual(initialOptions); + })); + it('should animate only the specified CSS className matched within $animateProvider.classNameFilter', function() { module(function($animateProvider) { $animateProvider.classNameFilter(/only-allow-this-animation/); @@ -149,20 +167,24 @@ describe("animations", function() { }); }); - they('should nullify both options.$prop when passed into an animation if it is not a string or an array', ['addClass', 'removeClass'], function(prop) { + they('should not apply the provided options.$prop value unless it\'s a string or string-based array', ['addClass', 'removeClass'], function(prop) { inject(function($animate, $rootScope) { + var startingCssClasses = element.attr('class') || ''; + var options1 = {}; options1[prop] = function() {}; $animate.enter(element, parent, null, options1); - expect(options1[prop]).toBeFalsy(); + expect(element.attr('class')).toEqual(startingCssClasses); + $rootScope.$digest(); var options2 = {}; options2[prop] = true; $animate.leave(element, options2); - expect(options2[prop]).toBeFalsy(); + expect(element.attr('class')).toEqual(startingCssClasses); + $rootScope.$digest(); capturedAnimation = null; @@ -170,11 +192,15 @@ describe("animations", function() { var options3 = {}; if (prop === 'removeClass') { element.addClass('fatias'); + startingCssClasses = element.attr('class'); } options3[prop] = ['fatias']; $animate.enter(element, parent, null, options3); - expect(options3[prop]).toBe('fatias'); + + $rootScope.$digest(); + + expect(element.attr('class')).not.toEqual(startingCssClasses); }); }); diff --git a/test/ngAnimate/integrationSpec.js b/test/ngAnimate/integrationSpec.js index 8f6cde838b55..feb28e581456 100644 --- a/test/ngAnimate/integrationSpec.js +++ b/test/ngAnimate/integrationSpec.js @@ -562,5 +562,50 @@ describe('ngAnimate integration tests', function() { } }); }); + + it("should not alter the provided options values in anyway throughout the animation", function() { + var animationSpy = jasmine.createSpy(); + module(function($animateProvider) { + $animateProvider.register('.this-animation', function() { + return { + enter: function(element, done) { + animationSpy(); + done(); + } + }; + }); + }); + + inject(function($animate, $rootScope, $compile) { + element = jqLite('
'); + var child = jqLite('
'); + + var initialOptions = { + from: { height: '50px' }, + to: { width: '100px' }, + addClass: 'one', + removeClass: 'two' + }; + + var copiedOptions = copy(initialOptions); + expect(copiedOptions).toEqual(initialOptions); + + html(element); + $compile(element)($rootScope); + + $animate.enter(child, element, null, copiedOptions); + $rootScope.$digest(); + expect(copiedOptions).toEqual(initialOptions); + + $animate.flush(); + expect(copiedOptions).toEqual(initialOptions); + + expect(child).toHaveClass('one'); + expect(child).not.toHaveClass('two'); + + expect(child.attr('style')).toContain('100px'); + expect(child.attr('style')).toContain('50px'); + }); + }); }); });