Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngAnimate): safe-guard against missing document #14633

Merged
merged 2 commits into from
May 24, 2016
Merged

fix(ngAnimate): safe-guard against missing document #14633

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9c1d33
fix(ngAnimate): safe-guard against missing document
Narretz May 18, 2016
874ef26
assume document.hidden if doc is missing
Narretz May 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/ngAnimate/animateQueue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
var activeAnimationsLookup = new $$HashMap();
var disabledElementsLookup = new $$HashMap();
var animationsEnabled = null;
// $document might be mocked and won't include a real document.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"... mocked out in tests ..."

// Providing an empty object will prevent property read errors
var rawDocument = $document[0] || {};

function postDigestTaskFactory() {
var postDigestCalled = false;
Expand Down Expand Up @@ -331,7 +334,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {

var isStructural = ['enter', 'move', 'leave'].indexOf(event) >= 0;

var documentHidden = animationsEnabled && $document[0].hidden;
var documentHidden = rawDocument.hidden;
Copy link

@ThomasBurleson ThomasBurleson May 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also need to include animationsEnabled ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasBurleson - that was a hack by @IgorMinar to get the tests passing in G3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean "not need"? animationsEnabled was added by Igor to fix a test in the g3 sync, but as I wrote in the commit message, this doesn't actually prevent the access to the document in all cases. If animations are enabled, $document[0] is still checked, and it might still be undefined


// this is a hard disable of all animations for the application or on
// the element itself, therefore there is no need to continue further
Expand Down Expand Up @@ -583,7 +586,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
* d) the element is not a child of the $rootElement
*/
function areAnimationsAllowed(element, parentElement, event) {
var bodyElement = jqLite($document[0].body);
var bodyElement = jqLite(rawDocument.body);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC there is one more access to $document[0].body. Should we change that too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, in animateCssDriver.js.

var bodyElementDetected = isMatchingElement(element, bodyElement) || element[0].nodeName === 'HTML';
var rootElementDetected = isMatchingElement(element, $rootElement);
var parentAnimationDetected = false;
Expand Down