From f2679313b06203364012348cad66d19181adbe31 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 1 Sep 2015 22:56:06 +0100 Subject: [PATCH 1/2] refactor($animate): move CSS class update functions out of closure --- src/ng/animate.js | 93 ++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/src/ng/animate.js b/src/ng/animate.js index 88e0e160592c..26c9f237d059 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -105,61 +105,64 @@ var $$CoreAnimateQueueProvider = function() { } }; - function addRemoveClassesPostDigest(element, add, remove) { - var classVal, data = postDigestQueue.get(element); - if (!data) { - postDigestQueue.put(element, data = {}); - postDigestElements.push(element); + function updateData(data, classes, value) { + var changed = false; + if (classes) { + classes = isString(classes) ? classes.split(' ') : + isArray(classes) ? classes : []; + forEach(classes, function(className) { + if (className) { + changed = true; + data[className] = value; + } + }); } + return changed; + } - var updateData = function(classes, value) { - var changed = false; - if (classes) { - classes = isString(classes) ? classes.split(' ') : - isArray(classes) ? classes : []; - forEach(classes, function(className) { - if (className) { - changed = true; - data[className] = value; + function handleCSSClassChanges() { + forEach(postDigestElements, function(element) { + var data = postDigestQueue.get(element); + if (data) { + var existing = splitClasses(element.attr('class')); + var toAdd = ''; + var toRemove = ''; + forEach(data, function(status, className) { + var hasClass = !!existing[className]; + if (status !== hasClass) { + if (status) { + toAdd += (toAdd.length ? ' ' : '') + className; + } else { + toRemove += (toRemove.length ? ' ' : '') + className; + } } }); + + forEach(element, function(elm) { + toAdd && jqLiteAddClass(elm, toAdd); + toRemove && jqLiteRemoveClass(elm, toRemove); + }); + postDigestQueue.remove(element); } - return changed; - }; + }); + postDigestElements.length = 0; + } - var classesAdded = updateData(add, true); - var classesRemoved = updateData(remove, false); - if ((!classesAdded && !classesRemoved) || postDigestElements.length > 1) return; - $rootScope.$$postDigest(function() { - forEach(postDigestElements, function(element) { - var data = postDigestQueue.get(element); - if (data) { - var existing = splitClasses(element.attr('class')); - var toAdd = ''; - var toRemove = ''; - forEach(data, function(status, className) { - var hasClass = !!existing[className]; - if (status !== hasClass) { - if (status) { - toAdd += (toAdd.length ? ' ' : '') + className; - } else { - toRemove += (toRemove.length ? ' ' : '') + className; - } - } - }); + function addRemoveClassesPostDigest(element, add, remove) { + var classVal, data = postDigestQueue.get(element); - forEach(element, function(elm) { - toAdd && jqLiteAddClass(elm, toAdd); - toRemove && jqLiteRemoveClass(elm, toRemove); - }); - postDigestQueue.remove(element); - } - }); + if (!data) { + postDigestQueue.put(element, data = {}); + postDigestElements.push(element); + } - postDigestElements.length = 0; - }); + var classesAdded = updateData(data, add, true); + var classesRemoved = updateData(data, remove, false); + if ((!classesAdded && !classesRemoved) || postDigestElements.length > 1) return; + + $rootScope.$$postDigest(handleCSSClassChanges); } }]; }; From 94a241dede6142e06e729d0dcd8b9eeca48b1d0d Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 1 Sep 2015 23:01:26 +0100 Subject: [PATCH 2/2] 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 #12674 --- src/ng/animate.js | 18 ++++++++++-------- test/ng/animateSpec.js | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/ng/animate.js b/src/ng/animate.js index 26c9f237d059..f3b6ffdd4f3f 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -151,18 +151,20 @@ var $$CoreAnimateQueueProvider = function() { function addRemoveClassesPostDigest(element, add, remove) { - var classVal, data = postDigestQueue.get(element); - - if (!data) { - postDigestQueue.put(element, data = {}); - postDigestElements.push(element); - } + var data = postDigestQueue.get(element) || {}; var classesAdded = updateData(data, add, true); var classesRemoved = updateData(data, remove, false); - if ((!classesAdded && !classesRemoved) || postDigestElements.length > 1) return; - $rootScope.$$postDigest(handleCSSClassChanges); + if (classesAdded || classesRemoved) { + + postDigestQueue.put(element, data); + postDigestElements.push(element); + + if (postDigestElements.length === 1) { + $rootScope.$$postDigest(handleCSSClassChanges); + } + } } }]; }; diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js index 2e9fdcaa9d3c..76fae5ebcaa6 100644 --- a/test/ng/animateSpec.js +++ b/test/ng/animateSpec.js @@ -341,6 +341,21 @@ describe("$animate", function() { }); }); + + it('should not break postDigest for subsequent elements if addClass contains non-valid CSS class names', function() { + inject(function($animate, $rootScope, $rootElement) { + var element1 = jqLite('
'); + var element2 = jqLite('
'); + + $animate.enter(element1, $rootElement, null, { addClass: ' ' }); + $animate.enter(element2, $rootElement, null, { addClass: 'valid-name' }); + $rootScope.$digest(); + + expect(element2.hasClass('valid-name')).toBeTruthy(); + }); + }); + + it('should not issue a call to removeClass if the provided class value is not a string or array', function() { inject(function($animate, $rootScope, $rootElement) { var spy = spyOn(window, 'jqLiteRemoveClass').andCallThrough();