Skip to content

Commit

Permalink
fix($animateQueue): handle listeners on containers without a `.contai…
Browse files Browse the repository at this point in the history
…ns()` method

When registering an animation listener (via `$animate.on()`), the container's `.contains()` method
was used to determine if the animated element is a child of the container. This resulted in errors
in IE, when `document` was used as a container, because IE does not define `.contains()` on
`Node`'s prototype (as prescribed by the spec), but on `HTMLElement`'s prototype instead. This is
not an issue on other DOM elements, because they inherit from both `Node` and `HTMLElement`, but
it is on `document`, which is not an instance of `HTMLElement`.

This change provides a way to determine if the animated element is a child of the container, even
if the latter does not have a `.contains()` method.

Fixes angular#13548
  • Loading branch information
gkalpak committed Jan 7, 2016
1 parent f7eab8d commit c0b10d0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/ngAnimate/animateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
var entries = callbackRegistry[event];
if (entries) {
forEach(entries, function(entry) {
if (entry.node.contains(targetNode)) {
if (entry.contains(targetNode)) {
matches.push(entry.callback);
} else if (event === 'leave' && entry.node.contains(targetParentNode)) {
} else if (event === 'leave' && entry.contains(targetParentNode)) {
matches.push(entry.callback);
}
});
Expand All @@ -196,6 +196,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
callbackRegistry[event] = callbackRegistry[event] || [];
callbackRegistry[event].push({
node: node,
contains: (node.contains || fallbackContainsFn).bind(node),
callback: callback
});
},
Expand Down Expand Up @@ -649,5 +650,12 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
: details;
activeAnimationsLookup.put(node, newValue);
}

function fallbackContainsFn(arg) {
//jshint bitwise: false, validthis: true
return (this === arg) || !!(this.compareDocumentPosition(arg) & 16);
//jshint bitwise: true, validthis: false
}

}];
}];
20 changes: 20 additions & 0 deletions test/ngAnimate/animateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,26 @@ describe("animations", function() {
expect(callbackTriggered).toBe(true);
}));

it('should handle containers that do not have a `contains()` method (e.g. `document` on IE)',
inject(function($animate, $rootElement, $rootScope) {
var rootNode = $rootElement[0];
rootNode.contains = undefined;

var callbackTriggered = false;

$animate.on('enter', rootNode, function() {
callbackTriggered = true;
});

element = jqLite('<div></div>');
$animate.enter(element, $rootElement);
$rootScope.$digest();
$animate.flush();

expect(callbackTriggered).toBe(true);
})
);

it('should remove all the event-based event listeners when $animate.off(event) is called',
inject(function($animate, $rootScope, $rootElement, $document) {

Expand Down

0 comments on commit c0b10d0

Please sign in to comment.