From f86c94588d9582fae6788da42b0c7dc9c22ae6a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Tue, 26 May 2015 13:35:39 -0700 Subject: [PATCH] fix($animate): ignore invalid option parameter values Prior to this fix there was another patch that threw an exception if the provided options value was not of an object type. While this is correct in terms of logic, it caused issues with plugins and tools that are designed to work with multiple version of Angular. This fix ensures that these plugins work since an invalid options value is ignored by `$animate`. Closes #11826 --- src/ng/animate.js | 27 +++++++++++---- test/ng/animateSpec.js | 76 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/ng/animate.js b/src/ng/animate.js index 963673cec022..142de7b87d01 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -40,6 +40,19 @@ function splitClasses(classes) { return obj; } +// if any other type of options value besides an Object value is +// passed into the $animate.method() animation then this helper code +// will be run which will ignore it. While this patch is not the +// greatest solution to this, a lot of existing plugins depend on +// $animate to either call the callback (< 1.2) or return a promise +// that can be changed. This helper function ensures that the options +// are wiped clean incase a callback function is provided. +function prepareAnimateOptions(options) { + return isObject(options) + ? options + : {}; +} + var $$CoreAnimateRunnerProvider = function() { this.$get = ['$q', '$$rAF', function($q, $$rAF) { function AnimateRunner() {} @@ -420,7 +433,7 @@ var $AnimateProvider = ['$provide', function($provide) { after = after && jqLite(after); parent = parent || after.parent(); domInsert(element, parent, after); - return $$animateQueue.push(element, 'enter', options); + return $$animateQueue.push(element, 'enter', prepareAnimateOptions(options)); }, /** @@ -446,7 +459,7 @@ var $AnimateProvider = ['$provide', function($provide) { after = after && jqLite(after); parent = parent || after.parent(); domInsert(element, parent, after); - return $$animateQueue.push(element, 'move', options); + return $$animateQueue.push(element, 'move', prepareAnimateOptions(options)); }, /** @@ -463,7 +476,7 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ leave: function(element, options) { - return $$animateQueue.push(element, 'leave', options, function() { + return $$animateQueue.push(element, 'leave', prepareAnimateOptions(options), function() { element.remove(); }); }, @@ -487,7 +500,7 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ addClass: function(element, className, options) { - options = options || {}; + options = prepareAnimateOptions(options); options.addClass = mergeClasses(options.addclass, className); return $$animateQueue.push(element, 'addClass', options); }, @@ -511,7 +524,7 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ removeClass: function(element, className, options) { - options = options || {}; + options = prepareAnimateOptions(options); options.removeClass = mergeClasses(options.removeClass, className); return $$animateQueue.push(element, 'removeClass', options); }, @@ -536,7 +549,7 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ setClass: function(element, add, remove, options) { - options = options || {}; + options = prepareAnimateOptions(options); options.addClass = mergeClasses(options.addClass, add); options.removeClass = mergeClasses(options.removeClass, remove); return $$animateQueue.push(element, 'setClass', options); @@ -564,7 +577,7 @@ var $AnimateProvider = ['$provide', function($provide) { * @return {Promise} the animation callback promise */ animate: function(element, from, to, className, options) { - options = options || {}; + options = prepareAnimateOptions(options); options.from = options.from ? extend(options.from, from) : from; options.to = options.to ? extend(options.to, to) : to; diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js index 543806bfd07f..4c3365fe5ca2 100644 --- a/test/ng/animateSpec.js +++ b/test/ng/animateSpec.js @@ -242,6 +242,82 @@ describe("$animate", function() { expect(element[0].previousSibling).toBe(after); }); }); + + they('$prop() should operate using a native DOM element', + ['enter', 'move', 'leave', 'addClass', 'removeClass', 'setClass', 'animate'], function(event) { + + var captureSpy = jasmine.createSpy(); + + module(function($provide) { + $provide.value('$$animateQueue', { + push: captureSpy + }); + }); + + inject(function($animate, $rootScope, $document, $rootElement) { + var element = jqLite('
'); + var parent2 = jqLite('
'); + var parent = $rootElement; + parent.append(parent2); + + if (event !== 'enter' && event !== 'move') { + parent.append(element); + } + + var fn, invalidOptions = function() { }; + + switch (event) { + case 'enter': + case 'move': + fn = function() { + $animate[event](element, parent, parent2, invalidOptions); + }; + break; + + case 'addClass': + fn = function() { + $animate.addClass(element, 'klass', invalidOptions); + }; + break; + + case 'removeClass': + element.className = 'klass'; + fn = function() { + $animate.removeClass(element, 'klass', invalidOptions); + }; + break; + + case 'setClass': + element.className = 'two'; + fn = function() { + $animate.setClass(element, 'one', 'two', invalidOptions); + }; + break; + + case 'leave': + fn = function() { + $animate.leave(element, invalidOptions); + }; + break; + + case 'animate': + var toStyles = { color: 'red' }; + fn = function() { + $animate.animate(element, {}, toStyles, 'klass', invalidOptions); + }; + break; + } + + expect(function() { + fn(); + $rootScope.$digest(); + }).not.toThrow(); + + var optionsArg = captureSpy.mostRecentCall.args[2]; + expect(optionsArg).not.toBe(invalidOptions); + expect(isObject(optionsArg)).toBeTruthy(); + }); + }); }); describe('CSS class DOM manipulation', function() {