angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.8k stars 27.49k forks source link

Angular-animate.js / onAnimationProgress(event) function null error IE #10387

Closed ghost closed 8 years ago

ghost commented 9 years ago

Browser: IE 9, 10, 11 Angular-Animate Version: v1.2.21 Issue: ev.elaspedTime is null and calling the toFixed function on this causes an exception.

caitp commented 9 years ago

that would affect non-IE browsers too. Can you provide a minimal reproduction please so that we can write a test case and land a fix?

pkozlowski-opensource commented 9 years ago

@Jamertunes what is the AngularJS version you are using? I can't find any reference to elaspedTime in master...

caitp commented 9 years ago

@pkozlowski-opensource https://github.com/angular/angular.js/blob/master/src/ngAnimate/animate.js#L1958

pkozlowski-opensource commented 9 years ago

@caitp lol, there was a typo in the original bug report:

Issue: ev.elaspedTime is null and calling the toFixed function on this causes an exception.

and I've copied it blindly when grepping :-/ Sorry for the noise

ghost commented 9 years ago

Sorry for the typo. While moving around a page in IE, the event parameter appears to be populating properly. I can reproduce the error only when swapping out page content. When the page changes, the error occurs. The error is being generated in Visual Studio prior to the content loading. Angular version v1.2.21.

image

caitp commented 9 years ago

but could you please create a runnable example, @Jamertunes --- so that we can reproduce this.

matsko commented 9 years ago

@caitp please not that the animation code for 1.4 is now different in terms of structure. You'll want to edit src/ngAnimate/animateCss.js to edit that line in 1.4 and then src/ngAnimate/animate.js in 1.3. The elapsedTime code is the same though.

caitp commented 9 years ago

I'm just not sure what situation(s) the value ends up being null in, which is why I'd like to see a reproduction or some basic steps we could use to verify a fix.

ngAnimate is your machine, it would be good if you could identify all the entrypoints where the elapsedTime value would be set, and make sure it's always a number

matsko commented 9 years ago

I'm moving this over to backlog since we still don't have a working reproduction.

lucax88x commented 9 years ago

Gotcha, could reproduce it in the easiest form.

https://jsfiddle.net/h67n69s6/4/

seems that ng-form is doing something to the animations? bug happens only when the form is dirty

robertsimmons commented 9 years ago

Having the same issue after upgrading to 1.4 from 1.3 (wasn't present before). In our case, similar to your repo jsfiddle, it's a bootstrap modal button inside an element that has animations on it. We have another situation though where it's a bootstrap toggle that causes it as well.

If you put a prevent default in the onclick for the buttons, then it goes away as well. Hope that helps narrow it down.

rsigmond commented 9 years ago

Having the same issue after upgrading from 1.3 to 1.4.6. The error occurs when moving over a hyperlink that contains a callout/title message.

Issue: Unable to get property 'toFixed' of undefined or null reference.
Occurs in method onAnimationProgress in angular.animate.js

The error did not occur in the previous version. Not sure how the onAnimationProgress function is being invoked.

A solution would be greatly appreciated, a clue on how this method is being invoked.

wiredprogrammer commented 9 years ago

I'm getting this issue specifically on IE11 during a bootstrap carousel transition between slides. Running 1.4.7. Same error as rsigmond on the same method. I bump the document mode to IE10 and it goes away. I'm not seeing the error on chrome.

provegard commented 8 years ago

I have tried to understand when this happens, and this is what I've come up with so far:

Bootstrap has a function named emulateTransitionEnd. Here's the one from 3.3.4:

  $.fn.emulateTransitionEnd = function (duration) {
    var called = false
    var $el = this
    $(this).one('bsTransitionEnd', function () { called = true })
    var callback = function () { if (!called) $($el).trigger($.support.transition.end) }
    setTimeout(callback, duration)
    return this
  }

When an event is triggered from emulateTransitionEnd, there is no originalEvent property, so this line in angular-animate uses the jQuery event which doesn't have elapsedTime:

        function onAnimationProgress(event) {
          event.stopPropagation();
          var ev = event.originalEvent || event; // <--- this one here

Note that when emulateTransitionEnd observes a bsTransitionEnd event before the timeout has elapsed, no event is emulated. In this case, onAnimationProgress only observes a real event and no error occurs.

I have only seen the erroneous behavior in IE (11) and not in Chrome. The reason is that in Chrome $.support.transition.end contains "webkitTransitionEnd" rather than "transitionend" (which is what is used for IE). But angular-animate uses a different way (compared to Bootstrap) to determine which event(s) it should listen for and ends up only listening for "transitionend":

So in summary:

OscarBarrett commented 8 years ago

I use the following directive to disable ngAnimate on elements that might cause this error to be thrown (for example, bootstrap carousel items).

.directive 'noAnimate', ($animate) ->
  link: (scope, elem, attrs) ->
    $animate.enabled(false, elem)
Narretz commented 8 years ago

@provegard Thanks for the investigation. Do you have an idea how angular should fix this? Or is bootstrap doing something here it probably shouldn't?

provegard commented 8 years ago

@Narretz IMO angular should do better input validation, i.e. ignore the event if it isn't of the expected type. I don't know why Bootstrap emulates transitionend but I suppose there is a good reason. :)