fix(ngAnimate): $timeout without invokeApply

[startswithaj]

Currently when using ngAnimate in-conjunction
with ng-repeat a $rootScope.$digest happens
per element animated.
This change calls $timeout with the invokeApply
parameter set to false which stops ngAnimate
from invoking its changes inside an $apply block.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

[startswithaj]

resolves #12281

I have signed the cla

[googlebot]

CLAs look good, thanks!

[wesleycho]

This looks like a good change to me, but probably needs another pair of eyes to be sure.

[simon-lang]

+1 thanks

[startswithaj]

There are plunkrs that demo this bug on #12281

[ntr-808]

solid change +1

[startswithaj]

@wesleycho the $timeout definitely shouldn't be forcing an apply but the original code doesn't make total sense to me. So I agree it needs another set of eyes.

The $timeout queues up onAnimationExpired() to run after a given time period but then says in the code comments the below.

        function onAnimationExpired() {
          // although an expired animation is a failed animation, getting to
          // this outcome is very easy if the CSS code screws up. Therefore we
          // should still continue normally as if the animation completed correctly.
          close();
        }

A reference to the $timeout isn't stored and cancelled if the animation is successful in onAnimationProgress(). So the $timedout function onAnimationExpired() runs for every element regardless of where its failed or not. So it goes
onAnimationExpired ->
close() ->
if (animationClosed || (animationCompleted && animationPaused)) return;

And then because of no invokeApply false passed to the timeout it does a full apply/digest cycle per element.

If we do end up in the close() block and indeed the animation hasn't finished and we need to clean up the element for whatever reason this may necessitate a digest() (but im not sure). I put in the code below "// IF WE GET HERE MAYBE DIGEST ON THE SCOPE OF THE ELEMENT" where this should probably happen.

function close(rejected) { // jshint ignore:line
        // if the promise has been called already then we shouldn't close
        // the animation again
        if (animationClosed || (animationCompleted && animationPaused)) return;
        animationClosed = true;
        animationPaused = false;

        $$jqLite.removeClass(element, setupClasses);
        $$jqLite.removeClass(element, activeClasses);

        blockKeyframeAnimations(node, false);
        blockTransitions(node, false);

        forEach(temporaryStyles, function(entry) {
          // There is only one way to remove inline style properties entirely from elements.
          // By using `removeProperty` this works, but we need to convert camel-cased CSS
          // styles down to hyphenated values.
          node.style[entry[0]] = '';
        });

        applyAnimationClasses(element, options);
        applyAnimationStyles(element, options);

        // the reason why we have this option is to allow a synchronous closing callback
        // that is fired as SOON as the animation ends (when the CSS is removed) or if
        // the animation never takes off at all. A good example is a leave animation since
        // the element must be removed just after the animation is over or else the element
        // will appear on screen for one animation frame causing an overbearing flicker.
        if (options.onDone) {
          // IF WE GET HERE MAYBE DIGEST ON THE SCOPE OF THE ELEMENT
          options.onDone();
        }

        // if the preparation function fails then the promise is not setup
        if (runner) {
          runner.complete(!rejected);
        }
      }

[matsko]

This is great. Thank you for finding this.

[matsko]

Merged in now to master. Thanks again for finding this.