From 94e59840e0fb66933176adba8b50cebc8c1f8ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 17 Sep 2015 16:44:51 -0700 Subject: [PATCH 1/2] revert: chore(core): introduce $$body service Relying on the body node to be present right at injection has caused issues with unit testing as well as some animations on the body element. Reverting this patch fixes these issues. Closes #12874 --- angularFiles.js | 1 - src/ngAnimate/animateCssDriver.js | 6 +- src/ngAnimate/animateQueue.js | 9 +- src/ngAnimate/body.js | 7 -- src/ngAnimate/module.js | 3 - test/ng/directive/ngClassSpec.js | 4 +- test/ngAnimate/animateCssDriverSpec.js | 4 +- test/ngAnimate/animateCssSpec.js | 168 ++++++++++++------------- test/ngAnimate/animateSpec.js | 58 ++++----- test/ngAnimate/animationSpec.js | 4 +- test/ngAnimate/bodySpec.js | 9 -- test/ngAnimate/integrationSpec.js | 4 +- 12 files changed, 129 insertions(+), 148 deletions(-) delete mode 100644 src/ngAnimate/body.js delete mode 100644 test/ngAnimate/bodySpec.js diff --git a/angularFiles.js b/angularFiles.js index 9d18fd831b6c..1e6eb6a6b77a 100755 --- a/angularFiles.js +++ b/angularFiles.js @@ -92,7 +92,6 @@ var angularFiles = { 'angularModules': { 'ngAnimate': [ 'src/ngAnimate/shared.js', - 'src/ngAnimate/body.js', 'src/ngAnimate/rafScheduler.js', 'src/ngAnimate/animateChildrenDirective.js', 'src/ngAnimate/animateCss.js', diff --git a/src/ngAnimate/animateCssDriver.js b/src/ngAnimate/animateCssDriver.js index 12afe34abc76..2e12b33a3df1 100644 --- a/src/ngAnimate/animateCssDriver.js +++ b/src/ngAnimate/animateCssDriver.js @@ -9,13 +9,13 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro var NG_OUT_ANCHOR_CLASS_NAME = 'ng-anchor-out'; var NG_IN_ANCHOR_CLASS_NAME = 'ng-anchor-in'; - this.$get = ['$animateCss', '$rootScope', '$$AnimateRunner', '$rootElement', '$$body', '$sniffer', '$$jqLite', - function($animateCss, $rootScope, $$AnimateRunner, $rootElement, $$body, $sniffer, $$jqLite) { + this.$get = ['$animateCss', '$rootScope', '$$AnimateRunner', '$rootElement', '$sniffer', '$$jqLite', '$document', + function($animateCss, $rootScope, $$AnimateRunner, $rootElement, $sniffer, $$jqLite, $document) { // only browsers that support these properties can render animations if (!$sniffer.animations && !$sniffer.transitions) return noop; - var bodyNode = getDomNode($$body); + var bodyNode = $document[0].body; var rootNode = getDomNode($rootElement); var rootBodyElement = jqLite(bodyNode.parentNode === rootNode ? bodyNode : rootNode); diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index 6337ec0c03c2..d15e608f00fb 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -66,9 +66,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { return (nO.addClass && nO.addClass === cO.removeClass) || (nO.removeClass && nO.removeClass === cO.addClass); }); - this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$body', '$$HashMap', + this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap', '$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow', - function($$rAF, $rootScope, $rootElement, $document, $$body, $$HashMap, + function($$rAF, $rootScope, $rootElement, $document, $$HashMap, $$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow) { var activeAnimationsLookup = new $$HashMap(); @@ -502,7 +502,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { } function areAnimationsAllowed(element, parentElement, event) { - var bodyElementDetected = isMatchingElement(element, $$body) || element[0].nodeName === 'HTML'; + var bodyElement = jqLite($document[0].body); + var bodyElementDetected = isMatchingElement(element, bodyElement) || element[0].nodeName === 'HTML'; var rootElementDetected = isMatchingElement(element, $rootElement); var parentAnimationDetected = false; var animateChildren; @@ -558,7 +559,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) { if (!bodyElementDetected) { // we also need to ensure that the element is or will be apart of the body element // otherwise it is pointless to even issue an animation to be rendered - bodyElementDetected = isMatchingElement(parentElement, $$body); + bodyElementDetected = isMatchingElement(parentElement, bodyElement); } parentElement = parentElement.parent(); diff --git a/src/ngAnimate/body.js b/src/ngAnimate/body.js deleted file mode 100644 index 8c8ce21ac9e8..000000000000 --- a/src/ngAnimate/body.js +++ /dev/null @@ -1,7 +0,0 @@ -'use strict'; - -function $$BodyProvider() { - this.$get = ['$document', function($document) { - return jqLite($document[0].body); - }]; -} diff --git a/src/ngAnimate/module.js b/src/ngAnimate/module.js index d48dd73e8d9c..2e4ec0171fdf 100644 --- a/src/ngAnimate/module.js +++ b/src/ngAnimate/module.js @@ -2,7 +2,6 @@ /* global angularAnimateModule: true, - $$BodyProvider, $$AnimateAsyncRunFactory, $$rAFSchedulerFactory, $$AnimateChildrenDirective, @@ -742,8 +741,6 @@ * Click here {@link ng.$animate to learn more about animations with `$animate`}. */ angular.module('ngAnimate', []) - .provider('$$body', $$BodyProvider) - .directive('ngAnimateChildren', $$AnimateChildrenDirective) .factory('$$rAFScheduler', $$rAFSchedulerFactory) diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js index a188ba129356..a50f611e4b18 100644 --- a/test/ng/directive/ngClassSpec.js +++ b/test/ng/directive/ngClassSpec.js @@ -468,12 +468,12 @@ describe('ngClass animations', function() { }; }); }); - inject(function($compile, $rootScope, $browser, $rootElement, $animate, $timeout, $$body) { + inject(function($compile, $rootScope, $browser, $rootElement, $animate, $document) { $animate.enabled(true); $rootScope.val = 'crazy'; element = angular.element('
'); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $compile(element)($rootScope); diff --git a/test/ngAnimate/animateCssDriverSpec.js b/test/ngAnimate/animateCssDriverSpec.js index 589d5932e3ff..99ba41edc620 100644 --- a/test/ngAnimate/animateCssDriverSpec.js +++ b/test/ngAnimate/animateCssDriverSpec.js @@ -121,7 +121,7 @@ describe("ngAnimate $$animateCssDriver", function() { var from, to, fromAnimation, toAnimation; beforeEach(module(function() { - return function($rootElement, $$body) { + return function($rootElement, $document) { from = element; to = jqLite('
'); fromAnimation = { element: from, event: 'enter' }; @@ -130,7 +130,7 @@ describe("ngAnimate $$animateCssDriver", function() { $rootElement.append(to); // we need to do this so that style detection works - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); }; })); diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index a4071782ab43..36fbdac82f97 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -35,12 +35,12 @@ describe("ngAnimate $animateCss", function() { }); it("should return false if neither transitions or keyframes are supported by the browser", - inject(function($animateCss, $sniffer, $rootElement, $$body) { + inject(function($animateCss, $sniffer, $rootElement, $document) { var animator; var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $sniffer.transitions = $sniffer.animations = false; animator = $animateCss(element, { @@ -54,13 +54,13 @@ describe("ngAnimate $animateCss", function() { if (!browserSupportsCssAnimations()) return; it("should not attempt an animation if animations are globally disabled", - inject(function($animateCss, $animate, $rootElement, $$body) { + inject(function($animateCss, $animate, $rootElement, $document) { $animate.enabled(false); var animator, element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); animator = $animateCss(element, { duration: 10, @@ -109,9 +109,9 @@ describe("ngAnimate $animateCss", function() { describe("rAF usage", function() { it("should buffer all requests into a single requestAnimationFrame call", - inject(function($animateCss, $$rAF, $rootScope, $$body, $rootElement) { + inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var count = 0; var runners = []; @@ -149,8 +149,8 @@ describe("ngAnimate $animateCss", function() { }; }); }); - inject(function($animateCss, $$rAF, $$body, $rootElement) { - $$body.append($rootElement); + inject(function($animateCss, $$rAF, $document, $rootElement) { + jqLite($document[0].body).append($rootElement); function makeRequest() { var element = jqLite('
'); @@ -169,10 +169,10 @@ describe("ngAnimate $animateCss", function() { describe("animator and runner", function() { var animationDuration = 5; var element, animator; - beforeEach(inject(function($animateCss, $rootElement, $$body) { + beforeEach(inject(function($animateCss, $rootElement, $document) { element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); animator = $animateCss(element, { event: 'enter', @@ -365,10 +365,10 @@ describe("ngAnimate $animateCss", function() { { timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration }); } - beforeEach(inject(function($rootElement, $$body) { + beforeEach(inject(function($rootElement, $document) { element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); options = { event: 'enter', structural: true }; })); @@ -638,9 +638,9 @@ describe("ngAnimate $animateCss", function() { describe("staggering", function() { it("should apply a stagger based when an active ng-EVENT-stagger class with a transition-delay is detected", - inject(function($animateCss, $$body, $rootElement, $timeout) { + inject(function($animateCss, $document, $rootElement, $timeout) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', 'transition-delay:0.2s'); ss.addRule('.ng-enter', 'transition:2s linear all'); @@ -679,9 +679,9 @@ describe("ngAnimate $animateCss", function() { })); it("should apply a stagger based when for all provided addClass/removeClass CSS classes", - inject(function($animateCss, $$body, $rootElement, $timeout) { + inject(function($animateCss, $document, $rootElement, $timeout) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.red-add-stagger,' + '.blue-remove-stagger,' + @@ -749,9 +749,9 @@ describe("ngAnimate $animateCss", function() { })); it("should block the transition animation between start and animate when staggered", - inject(function($animateCss, $$body, $rootElement) { + inject(function($animateCss, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', 'transition-delay:0.2s'); ss.addRule('.ng-enter', 'transition:2s linear all;'); @@ -780,9 +780,9 @@ describe("ngAnimate $animateCss", function() { })); it("should block (pause) the keyframe animation between start and animate when staggered", - inject(function($animateCss, $$body, $rootElement) { + inject(function($animateCss, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', prefix + 'animation-delay:0.2s'); ss.addRule('.ng-enter', prefix + 'animation:my_animation 2s;'); @@ -809,9 +809,9 @@ describe("ngAnimate $animateCss", function() { })); it("should not apply a stagger if the transition delay value is inherited from a earlier CSS class", - inject(function($animateCss, $$body, $rootElement) { + inject(function($animateCss, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.transition-animation', 'transition:2s 5s linear all;'); @@ -828,9 +828,9 @@ describe("ngAnimate $animateCss", function() { })); it("should apply a stagger only if the transition duration value is zero when inherited from a earlier CSS class", - inject(function($animateCss, $$body, $rootElement) { + inject(function($animateCss, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.transition-animation', 'transition:2s 5s linear all;'); ss.addRule('.transition-animation.ng-enter-stagger', @@ -854,9 +854,9 @@ describe("ngAnimate $animateCss", function() { it("should ignore animation staggers if only transition animations were detected", - inject(function($animateCss, $$body, $rootElement) { + inject(function($animateCss, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', prefix + 'animation-delay:0.2s'); ss.addRule('.transition-animation', 'transition:2s 5s linear all;'); @@ -874,9 +874,9 @@ describe("ngAnimate $animateCss", function() { })); it("should ignore transition staggers if only keyframe animations were detected", - inject(function($animateCss, $$body, $rootElement) { + inject(function($animateCss, $document, $rootElement) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', 'transition-delay:0.2s'); ss.addRule('.transition-animation', prefix + 'animation:2s 5s my_animation;'); @@ -894,9 +894,9 @@ describe("ngAnimate $animateCss", function() { })); it("should start on the highest stagger value if both transition and keyframe staggers are used together", - inject(function($animateCss, $$body, $rootElement, $timeout, $browser) { + inject(function($animateCss, $document, $rootElement, $timeout, $browser) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', 'transition-delay:0.5s;' + prefix + 'animation-delay:1s'); @@ -932,9 +932,9 @@ describe("ngAnimate $animateCss", function() { })); it("should apply the closing timeout ontop of the stagger timeout", - inject(function($animateCss, $$body, $rootElement, $timeout, $browser) { + inject(function($animateCss, $document, $rootElement, $timeout, $browser) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', 'transition-delay:1s;'); ss.addRule('.ng-enter', 'transition:10s linear all;'); @@ -959,9 +959,9 @@ describe("ngAnimate $animateCss", function() { })); it("should apply the closing timeout ontop of the stagger timeout with an added delay", - inject(function($animateCss, $$body, $rootElement, $timeout, $browser) { + inject(function($animateCss, $document, $rootElement, $timeout, $browser) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter-stagger', 'transition-delay:1s;'); ss.addRule('.ng-enter', 'transition:10s linear all; transition-delay:50s;'); @@ -986,9 +986,9 @@ describe("ngAnimate $animateCss", function() { })); it("should issue a stagger if a stagger value is provided in the options", - inject(function($animateCss, $$body, $rootElement, $timeout) { + inject(function($animateCss, $document, $rootElement, $timeout) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.ng-enter', 'transition:2s linear all'); var elm, i, elements = []; @@ -1025,9 +1025,9 @@ describe("ngAnimate $animateCss", function() { })); it("should only add/remove classes once the stagger timeout has passed", - inject(function($animateCss, $$body, $rootElement, $timeout) { + inject(function($animateCss, $document, $rootElement, $timeout) { - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var element = jqLite('
'); $rootElement.append(element); @@ -1052,13 +1052,13 @@ describe("ngAnimate $animateCss", function() { describe("closing timeout", function() { it("should close off the animation after 150% of the animation time has passed", - inject(function($animateCss, $$body, $rootElement, $timeout) { + inject(function($animateCss, $document, $rootElement, $timeout) { ss.addRule('.ng-enter', 'transition:10s linear all;'); var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, { event: 'enter', structural: true }); animator.start(); @@ -1075,13 +1075,13 @@ describe("ngAnimate $animateCss", function() { })); it("should close off the animation after 150% of the animation time has passed and consider the detected delay value", - inject(function($animateCss, $$body, $rootElement, $timeout) { + inject(function($animateCss, $document, $rootElement, $timeout) { ss.addRule('.ng-enter', 'transition:10s linear all; transition-delay:30s;'); var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, { event: 'enter', structural: true }); animator.start(); @@ -1098,13 +1098,13 @@ describe("ngAnimate $animateCss", function() { })); it("should still resolve the animation once expired", - inject(function($animateCss, $$body, $rootElement, $timeout, $animate, $rootScope) { + inject(function($animateCss, $document, $rootElement, $timeout, $animate, $rootScope) { ss.addRule('.ng-enter', 'transition:10s linear all;'); var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, { event: 'enter', structural: true }); @@ -1123,13 +1123,13 @@ describe("ngAnimate $animateCss", function() { })); it("should not resolve/reject after passing if the animation completed successfully", - inject(function($animateCss, $$body, $rootElement, $timeout, $rootScope, $animate) { + inject(function($animateCss, $document, $rootElement, $timeout, $rootScope, $animate) { ss.addRule('.ng-enter', 'transition:10s linear all;'); var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, { event: 'enter', structural: true }); @@ -1160,7 +1160,7 @@ describe("ngAnimate $animateCss", function() { })); it("should close all stacked animations after the last timeout runs on the same element", - inject(function($animateCss, $$body, $rootElement, $timeout, $animate) { + inject(function($animateCss, $document, $rootElement, $timeout, $animate) { var now = 0; spyOn(Date, 'now').andCallFake(function() { @@ -1177,7 +1177,7 @@ describe("ngAnimate $animateCss", function() { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); // timeout will be at 1500s animate(element, 'red', doneSpy); @@ -1218,11 +1218,11 @@ describe("ngAnimate $animateCss", function() { })); it("should not throw an error any pending timeout requests resolve after the element has already been removed", - inject(function($animateCss, $$body, $rootElement, $timeout, $animate) { + inject(function($animateCss, $document, $rootElement, $timeout, $animate) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.red', 'transition:1s linear all;'); @@ -1255,8 +1255,8 @@ describe("ngAnimate $animateCss", function() { } })); - return function($$body, $rootElement) { - $$body.append($rootElement); + return function($document, $rootElement) { + jqLite($document[0].body).append($rootElement); }; })); @@ -1340,7 +1340,7 @@ describe("ngAnimate $animateCss", function() { }); it('should avoid applying the same cache to an element a follow-up animation is run on the same element', - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { function endTransition(element, elapsedTime) { browserTrigger(element, 'transitionend', @@ -1357,7 +1357,7 @@ describe("ngAnimate $animateCss", function() { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); startAnimation(element, 0.5, 'red'); expect(element.attr('style')).toContain('transition'); @@ -1377,14 +1377,14 @@ describe("ngAnimate $animateCss", function() { })); it("should clear cache if no animation so follow-up animation on the same element will not be from cache", - inject(function($animateCss, $rootElement, $$body, $$rAF) { + inject(function($animateCss, $rootElement, $document, $$rAF) { var element = jqLite('
'); var options = { event: 'enter', structural: true }; $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, options); expect(animator.$$willAnimate).toBeFalsy(); @@ -1396,11 +1396,11 @@ describe("ngAnimate $animateCss", function() { })); it('should apply a custom temporary class when a non-structural animation is used', - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $animateCss(element, { event: 'super', @@ -1416,10 +1416,10 @@ describe("ngAnimate $animateCss", function() { describe("structural animations", function() { they('should decorate the element with the ng-$prop CSS class', ['enter', 'leave', 'move'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $animateCss(element, { event: event, @@ -1433,10 +1433,10 @@ describe("ngAnimate $animateCss", function() { they('should decorate the element with the ng-$prop-active CSS class', ['enter', 'leave', 'move'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, { event: event, @@ -1454,10 +1454,10 @@ describe("ngAnimate $animateCss", function() { they('should remove the ng-$prop and ng-$prop-active CSS classes from the element once the animation is done', ['enter', 'leave', 'move'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var animator = $animateCss(element, { event: event, @@ -1511,10 +1511,10 @@ describe("ngAnimate $animateCss", function() { they('should place a CSS transition block after the preparation function to block accidental style changes', ['enter', 'leave', 'move', 'addClass', 'removeClass'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.cool-animation', 'transition:1.5s linear all;'); element.addClass('cool-animation'); @@ -1541,10 +1541,10 @@ describe("ngAnimate $animateCss", function() { they('should not place a CSS transition block if options.skipBlocking is provided', ['enter', 'leave', 'move', 'addClass', 'removeClass'], function(event) { - inject(function($animateCss, $rootElement, $$body, $window) { + inject(function($animateCss, $rootElement, $document, $window) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.cool-animation', 'transition:1.5s linear all;'); element.addClass('cool-animation'); @@ -1582,10 +1582,10 @@ describe("ngAnimate $animateCss", function() { they('should place a CSS transition block after the preparation function even if a duration is provided', ['enter', 'leave', 'move', 'addClass', 'removeClass'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); ss.addRule('.cool-animation', 'transition:1.5s linear all;'); element.addClass('cool-animation'); @@ -1616,11 +1616,11 @@ describe("ngAnimate $animateCss", function() { }); it('should allow multiple events to be animated at the same time', - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $animateCss(element, { event: ['enter', 'leave', 'move'], @@ -1688,10 +1688,10 @@ describe("ngAnimate $animateCss", function() { they('should remove the class-$prop-add and class-$prop-active CSS classes from the element once the animation is done', ['enter', 'leave', 'move'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var options = {}; options.event = event; @@ -1713,7 +1713,7 @@ describe("ngAnimate $animateCss", function() { they('should allow the class duration styles to be recalculated once started if the CSS classes being applied result new transition styles', ['add', 'remove'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { var element = jqLite('
'); @@ -1728,7 +1728,7 @@ describe("ngAnimate $animateCss", function() { } $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var options = {}; options[event + 'Class'] = 'natural-class'; @@ -1749,13 +1749,13 @@ describe("ngAnimate $animateCss", function() { they('should force the class-based values to be applied early if no options.applyClassEarly is used as an option', ['enter', 'leave', 'move'], function(event) { - inject(function($animateCss, $rootElement, $$body) { + inject(function($animateCss, $rootElement, $document) { ss.addRule('.blue.ng-' + event, 'transition:2s linear all;'); var element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); var runner = $animateCss(element, { addClass: 'blue', @@ -1790,8 +1790,8 @@ describe("ngAnimate $animateCss", function() { describe("options", function() { var element; beforeEach(module(function() { - return function($rootElement, $$body) { - $$body.append($rootElement); + return function($rootElement, $document) { + jqLite($document[0].body).append($rootElement); element = jqLite('
'); $rootElement.append(element); @@ -2741,10 +2741,10 @@ describe("ngAnimate $animateCss", function() { describe("[easing]", function() { var element; - beforeEach(inject(function($$body, $rootElement) { + beforeEach(inject(function($document, $rootElement) { element = jqLite('
'); $rootElement.append(element); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); })); it("should apply easing to a transition animation if it exists", inject(function($animateCss) { @@ -2812,13 +2812,13 @@ describe("ngAnimate $animateCss", function() { describe('SVG', function() { it('should properly apply transitions on an SVG element', - inject(function($animateCss, $rootScope, $compile, $$body, $rootElement) { + inject(function($animateCss, $rootScope, $compile, $document, $rootElement) { var element = $compile('' + '' + '')($rootScope); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $rootElement.append(element); $animateCss(element, { diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 06e06a867cfa..0bc9f716012d 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -26,12 +26,12 @@ describe("animations", function() { }); }); - inject(function($animate, $rootScope, $$body) { + inject(function($animate, $rootScope, $document) { $animate.enabled(true); element = jqLite('
'); - $animate.enter(element, $$body); + $animate.enter(element, jqLite($document[0].body)); $rootScope.$digest(); expect(capturedAnimation).toBeTruthy(); @@ -116,7 +116,7 @@ describe("animations", function() { return overriddenAnimationRunner || defaultFakeAnimationRunner; }); - return function($rootElement, $q, $animate, $$AnimateRunner, $$body) { + return function($rootElement, $q, $animate, $$AnimateRunner, $document) { defaultFakeAnimationRunner = new $$AnimateRunner(); $animate.enabled(true); @@ -126,7 +126,7 @@ describe("animations", function() { $rootElement.append(parent); $rootElement.append(parent2); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); }; })); @@ -749,7 +749,7 @@ describe("animations", function() { })); it("should disable all child animations for atleast one turn when a structural animation is issued", - inject(function($animate, $rootScope, $compile, $$body, $rootElement, $$AnimateRunner) { + inject(function($animate, $rootScope, $compile, $document, $rootElement, $$AnimateRunner) { element = $compile( '
' + @@ -759,7 +759,7 @@ describe("animations", function() { '
' )($rootScope); - $$body.append($rootElement); + jqLite($document[0].body).append($rootElement); $rootElement.append(element); var runner = new $$AnimateRunner(); @@ -1284,8 +1284,8 @@ describe("animations", function() { return new $$AnimateRunner(); }; }); - return function($rootElement, $$body, $animate) { - $$body.append($rootElement); + return function($rootElement, $document, $animate) { + jqLite($document[0].body).append($rootElement); parent = jqLite('
'); element = jqLite('
'); child = jqLite('
'); @@ -1389,14 +1389,14 @@ describe("animations", function() { })); it('should allow an element to pinned elsewhere and still be available in animations', - inject(function($animate, $compile, $$body, $rootElement, $rootScope) { + inject(function($animate, $compile, $document, $rootElement, $rootScope) { var innerParent = jqLite('
'); - $$body.append(innerParent); + jqLite($document[0].body).append(innerParent); innerParent.append($rootElement); var element = jqLite('
'); - $$body.append(element); + jqLite($document[0].body).append(element); $animate.addClass(element, 'red'); $rootScope.$digest(); @@ -1412,16 +1412,16 @@ describe("animations", function() { })); it('should adhere to the disabled state of the hosted parent when an element is pinned', - inject(function($animate, $compile, $$body, $rootElement, $rootScope) { + inject(function($animate, $compile, $document, $rootElement, $rootScope) { var innerParent = jqLite('
'); - $$body.append(innerParent); + jqLite($document[0].body).append(innerParent); innerParent.append($rootElement); var innerChild = jqLite('
'); $rootElement.append(innerChild); var element = jqLite('
'); - $$body.append(element); + jqLite($document[0].body).append(element); $animate.pin(element, innerChild); @@ -1456,17 +1456,17 @@ describe("animations", function() { }; }); - return function($$body, $rootElement, $animate) { - $$body.append($rootElement); + return function($document, $rootElement, $animate) { + jqLite($document[0].body).append($rootElement); $animate.enabled(true); }; })); it('should trigger a callback for an enter animation', - inject(function($animate, $rootScope, $rootElement, $$body) { + inject(function($animate, $rootScope, $rootElement, $document) { var callbackTriggered = false; - $animate.on('enter', $$body, function() { + $animate.on('enter', jqLite($document[0].body), function() { callbackTriggered = true; }); @@ -1480,12 +1480,12 @@ describe("animations", function() { })); it('should fire the callback with the signature of (element, phase, data)', - inject(function($animate, $rootScope, $rootElement, $$body) { + inject(function($animate, $rootScope, $rootElement, $document) { var capturedElement; var capturedPhase; var capturedData; - $animate.on('enter', $$body, + $animate.on('enter', jqLite($document[0].body), function(element, phase, data) { capturedElement = element; @@ -1544,13 +1544,13 @@ describe("animations", function() { })); it('should remove all the event-based event listeners when $animate.off(event) is called', - inject(function($animate, $rootScope, $rootElement, $$body) { + inject(function($animate, $rootScope, $rootElement, $document) { element = jqLite('
'); var count = 0; $animate.on('enter', element, counter); - $animate.on('enter', $$body, counter); + $animate.on('enter', jqLite($document[0].body), counter); function counter(element, phase) { count++; @@ -1572,13 +1572,13 @@ describe("animations", function() { })); it('should remove the container-based event listeners when $animate.off(event, container) is called', - inject(function($animate, $rootScope, $rootElement, $$body) { + inject(function($animate, $rootScope, $rootElement, $document) { element = jqLite('
'); var count = 0; $animate.on('enter', element, counter); - $animate.on('enter', $$body, counter); + $animate.on('enter', jqLite($document[0].body), counter); function counter(element, phase) { if (phase === 'start') { @@ -1592,7 +1592,7 @@ describe("animations", function() { expect(count).toBe(2); - $animate.off('enter', $$body); + $animate.off('enter', jqLite($document[0].body)); $animate.enter(element, $rootElement); $rootScope.$digest(); @@ -1638,13 +1638,13 @@ describe("animations", function() { })); it('should fire a `start` callback when the animation starts with the matching element', - inject(function($animate, $rootScope, $rootElement, $$body) { + inject(function($animate, $rootScope, $rootElement, $document) { element = jqLite('
'); var capturedState; var capturedElement; - $animate.on('enter', $$body, function(element, phase) { + $animate.on('enter', jqLite($document[0].body), function(element, phase) { capturedState = phase; capturedElement = element; }); @@ -1658,13 +1658,13 @@ describe("animations", function() { })); it('should fire a `close` callback when the animation ends with the matching element', - inject(function($animate, $rootScope, $rootElement, $$body) { + inject(function($animate, $rootScope, $rootElement, $document) { element = jqLite('
'); var capturedState; var capturedElement; - $animate.on('enter', $$body, function(element, phase) { + $animate.on('enter', jqLite($document[0].body), function(element, phase) { capturedState = phase; capturedElement = element; }); diff --git a/test/ngAnimate/animationSpec.js b/test/ngAnimate/animationSpec.js index ff5d998076cf..04ba3e5949a3 100644 --- a/test/ngAnimate/animationSpec.js +++ b/test/ngAnimate/animationSpec.js @@ -836,8 +836,8 @@ describe('$$animation', function() { element = jqLite('
'); parent = jqLite('
'); - return function($$AnimateRunner, $q, $rootElement, $$body) { - $$body.append($rootElement); + return function($$AnimateRunner, $rootElement, $document) { + jqLite($document[0].body).append($rootElement); $rootElement.append(parent); mockedDriverFn = function(element, method, options, domOperation) { diff --git a/test/ngAnimate/bodySpec.js b/test/ngAnimate/bodySpec.js deleted file mode 100644 index f87dabf1e355..000000000000 --- a/test/ngAnimate/bodySpec.js +++ /dev/null @@ -1,9 +0,0 @@ -'use strict'; - -describe('$$body', function() { - beforeEach(module('ngAnimate')); - - it("should inject $document", inject(function($$body, $document) { - expect($$body).toEqual(jqLite($document[0].body)); - })); -}); diff --git a/test/ngAnimate/integrationSpec.js b/test/ngAnimate/integrationSpec.js index 5e1e335b72f4..35f4f1c3d3f7 100644 --- a/test/ngAnimate/integrationSpec.js +++ b/test/ngAnimate/integrationSpec.js @@ -7,12 +7,12 @@ describe('ngAnimate integration tests', function() { var element, html, ss; beforeEach(module(function() { - return function($rootElement, $document, $$body, $window, $animate) { + return function($rootElement, $document, $window, $animate) { $animate.enabled(true); ss = createMockStyleSheet($document, $window); - var body = $$body; + var body = jqLite($document[0].body); html = function(element) { body.append($rootElement); $rootElement.append(element); From a986d043eb9df3be1cb0685dbe936c3e4378da06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Thu, 17 Sep 2015 17:55:28 -0700 Subject: [PATCH 2/2] fix(ngAnimate): ensure anchoring uses body as a container when needed Prior to this fix anchoring would allow for a container to be a document node or something higher beyond the body tag. This patch makes it fall back to body incase the rootElement node exists as a parent ancestor. Closes #12872 --- src/ngAnimate/animateCssDriver.js | 11 ++++++- test/ngAnimate/animateCssDriverSpec.js | 43 ++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/ngAnimate/animateCssDriver.js b/src/ngAnimate/animateCssDriver.js index 2e12b33a3df1..a2c815d57b7a 100644 --- a/src/ngAnimate/animateCssDriver.js +++ b/src/ngAnimate/animateCssDriver.js @@ -9,6 +9,10 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro var NG_OUT_ANCHOR_CLASS_NAME = 'ng-anchor-out'; var NG_IN_ANCHOR_CLASS_NAME = 'ng-anchor-in'; + function isDocumentFragment(node) { + return node.parentNode && node.parentNode.nodeType === 11; + } + this.$get = ['$animateCss', '$rootScope', '$$AnimateRunner', '$rootElement', '$sniffer', '$$jqLite', '$document', function($animateCss, $rootScope, $$AnimateRunner, $rootElement, $sniffer, $$jqLite, $document) { @@ -18,7 +22,12 @@ var $$AnimateCssDriverProvider = ['$$animationProvider', function($$animationPro var bodyNode = $document[0].body; var rootNode = getDomNode($rootElement); - var rootBodyElement = jqLite(bodyNode.parentNode === rootNode ? bodyNode : rootNode); + var rootBodyElement = jqLite( + // this is to avoid using something that sites outside of the body + // we also special case the doc fragement case because our unit test code + // appens the $rootElement to the body after the app has been bootstrapped + isDocumentFragment(rootNode) || bodyNode.contains(rootNode) ? rootNode : bodyNode + ); var applyAnimationClasses = applyAnimationClassesFactory($$jqLite); diff --git a/test/ngAnimate/animateCssDriverSpec.js b/test/ngAnimate/animateCssDriverSpec.js index 99ba41edc620..923f9e3f821c 100644 --- a/test/ngAnimate/animateCssDriverSpec.js +++ b/test/ngAnimate/animateCssDriverSpec.js @@ -129,8 +129,14 @@ describe("ngAnimate $$animateCssDriver", function() { $rootElement.append(from); $rootElement.append(to); - // we need to do this so that style detection works - jqLite($document[0].body).append($rootElement); + var doc = $document[0]; + + // there is one test in here that expects the rootElement + // to superceed the body node + if (!$rootElement[0].contains(doc.body)) { + // we need to do this so that style detection works + jqLite(doc.body).append($rootElement); + } }; })); @@ -975,6 +981,39 @@ describe("ngAnimate $$animateCssDriver", function() { expect(completed).toBe(true); })); + + it("should use as the element container if the rootElement exists outside of the tag", function() { + module(function($provide) { + $provide.factory('$rootElement', function($document) { + return jqLite($document[0].querySelector('html')); + }); + }); + inject(function($rootElement, $rootScope, $animate, $document) { + ss.addRule('.ending-element', 'width:9999px; height:6666px; display:inline-block;'); + + var fromAnchor = jqLite('
'); + from.append(fromAnchor); + + var toAnchor = jqLite('
'); + to.append(toAnchor); + + $rootElement.append(fromAnchor); + $rootElement.append(toAnchor); + + var completed = false; + driver({ + from: fromAnimation, + to: toAnimation, + anchors: [{ + 'out': fromAnchor, + 'in': toAnchor + }] + }).start(); + + var clone = captureLog[2].element[0]; + expect(clone.parentNode).toBe($document[0].body); + }); + }); }); }); });