angular / angular.js

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

$location runs into infinite digest under certain circumstances #1417

Closed IgorMinar closed 10 years ago

IgorMinar commented 11 years ago

when adding Angular to an existing project that makes use of html5 history. Whenever the url is changed outside of Angular, the next $digest to run produces this error:

Error: 10 $digest() iterations reached. Aborting! Watchers fired in the last 5 iterations: [["fn: $locationWatch; newVal: 8; oldVal: 7"],["fn: $locationWatch; newVal: 9; oldVal: 8"],["fn: $locationWatch; newVal: 10; oldVal: 9"],["fn: $locationWatch; newVal: 11; oldVal: 10"],["fn: $locationWatch; newVal: 12; oldVal: 11"]]

I have my own routing in place, using goog.history.Html5History. I was trying to include some Angular stuff on a single page (works), but when the page changes, the first subsequent $apply produces the above error. It looks like the $location service is getting out of sync with the browser.

$locationWatch expects the $location service to be updated first, followed by the browser. But when I pushState to the browser directly, the $location service still has the old url.

The sad thing is that $location is not being used directly by this app, nor is $route. It's ngInclude, that depends on $anchorScroll, which depends on $location - maybe fixing this dependency should be a separate issue... (to be investigated)

dbinit commented 11 years ago

Here is a jsFiddle that reproduces the issue: http://jsfiddle.net/dbinit/tMmKf/

martinstein commented 11 years ago

I also ran into this bug recently. For me it happened while testing in IE8 with html5Mode(true) (which in IE8 falls back to html5Mode(false), of course).

kstep commented 11 years ago

I confirm this bug for AngularJS 1.1.0 and IE9.

fr0 commented 11 years ago

This is also happening to me in Chrome (Version 22.0.1229.94 m), when using window.history.pushState in response to a button click. Angular version: 1.0.2.

gaiottino commented 11 years ago

Any update to this? Giving us some IE9 headaches.

scottweinert commented 11 years ago

I am also experiencing this in IE9. Any update?

ThomasDeutsch commented 11 years ago

having the same problem. Still no fix for this?

scottweinert commented 11 years ago

For what it's worth, I was having this issue in IE9 and Android and here is what I did:

1) Make sure you have a route to '/' in Angular and that it works properly - for example: $routeProvider.when('/', {templateUrl: 'partials/home', controller: HomeCtrl});

2) Add $locationProvider.html5Mode(true).hashPrefix('!'); to your app configuration

3) I had to add the following code to get android to work and I just added it in a script tag before angular loads etc.

var buggyAndroid = parseInt((/android (\d+)/.exec(window.navigator.userAgent.toLowerCase()) || [])[1], 10) < 4; var buggyAndroid = parseInt((/android (\d+)/.exec(window.navigator.userAgent.toLowerCase()) || [])[1], 10) < 4; if (!history.pushState || buggyAndroid) { if (window.location.hash) { if(window.location.pathname !== '/') window.location.replace('/#!' + window.location.hash.substr(2)); //Hash and a path, just keep the hash (redirect) } else { if (window.location.pathname === '/') window.location.replace('/#!/'); //No hash, no path
else window.location.replace('/#!' + window.location.pathname); //No hash, but have a path, take path } }

dbinit commented 11 years ago

In my case, I wasn't using $location at all. It was just getting pulled in by $anchorScroll, which is used by ngInclude.

I wasn't using $anchorScroll either, so I just did this to remove the dependency:

angular.module('myApp', []).value('$anchorScroll', null);

If you're using $anchorScroll you could probably just null out $location instead.

pgte commented 11 years ago

I'm running into this and I don't have explicit watchers and I don't import $location. When I do history.pushState(...) I get the following error:

Error: 10 $digest() iterations reached. Aborting!
Watchers fired in the last 5 iterations: [["fn: function (){var a=d.url(),b=f.$$replace;if(!o||a!=f.absUrl())o++,c.$evalAsync(function(){c.$broadcast(\"$locationChangeStart\",f.absUrl(),a).defaultPrevented?f.$$parse(a):(d.url(f.absUrl(),b),i(a))});f.$$replace=\n!1;return o}; newVal: 8; oldVal: 7"],["fn: function (){var a=d.url(),b=f.$$replace;if(!o||a!=f.absUrl())o++,c.$evalAsync(function(){c.$broadcast(\"$locationChangeStart\",f.absUrl(),a).defaultPrevented?f.$$parse(a):(d.url(f.absUrl(),b),i(a))});f.$$replace=\n!1;return o}; newVal: 9; oldVal: 8"],["fn: function (){var a=d.url(),b=f.$$replace;if(!o||a!=f.absUrl())o++,c.$evalAsync(function(){c.$broadcast(\"$locationChangeStart\",f.absUrl(),a).defaultPrevented?f.$$parse(a):(d.url(f.absUrl(),b),i(a))});f.$$replace=\n!1;return o}; newVal: 10; oldVal: 9"],["fn: function (){var a=d.url(),b=f.$$replace;if(!o||a!=f.absUrl())o++,c.$evalAsync(function(){c.$broadcast(\"$locationChangeStart\",f.absUrl(),a).defaultPrevented?f.$$parse(a):(d.url(f.absUrl(),b),i(a))});f.$$replace=\n!1;return o}; newVal: 11; oldVal: 10"],["fn: function (){var a=d.url(),b=f.$$replace;if(!o||a!=f.absUrl())o++,c.$evalAsync(function(){c.$broadcast(\"$locationChangeStart\",f.absUrl(),a).defaultPrevented?f.$$parse(a):(d.url(f.absUrl(),b),i(a))});f.$$replace=\n!1;return o}; newVal: 12; oldVal: 11"]]
    at Error (<anonymous>)
    at Object.e.$digest (http://localhost:8080/js/lib/angular.min.js:85:217)
    at Object.e.$apply (http://localhost:8080/js/lib/angular.min.js:86:469)
    at HTMLAnchorElement.<anonymous> (http://localhost:8080/js/lib/angular.min.js:140:507)
    at HTMLAnchorElement.v.event.dispatch (http://localhost:8080/js/lib/jquery-1.8.3.min.js:2:38053)
    at HTMLAnchorElement.o.handle.u (http://localhost:8080/js/lib/jquery-1.8.3.min.js:2:33916) 

Any ideas?

brandonsalmon commented 11 years ago

Here is an example where the initial page load fails and reloads after throwing the above error. http://jsfiddle.net/v7HG5/ (Open in IE)

Note: jsfiddle doesn't handle any angular routing, so the rendered view is a blank jsfiddle. Also, nothing will happen at all on an html5 supported browser.

My understanding is that the page is going to need to refresh to convert the url from html5 mode to hash mode (adding the "#" http://jsfiddle.net/#/v7HG5/). In my case this is fine, but it should handle its errors.

ThomasBorghs commented 11 years ago

Also problem with html5Mode(true) and IE8. Everything works in Chrome/FF, but in IE Angular is unable to bootstrap because the $locationWatcher keeps getting a new value, and thus $digest fails. This results in a redirection loop (for reasons not totally clear to me) where IE keeps redirecting to the "index" page and Angular fails to bootstrap.

Any news on this issue?

Right now the team is contemplating setting html5Mode(false) and rewriting the routing on the server side.

gaiottino commented 11 years ago

I've managed to reduce the occurance of this bug significantly by using $location.path() and $location.search() instead of manually using location.href. Hope that helps someone else.

leonzinger commented 11 years ago

@gaiottino were you using angular for routing or an external plugin? because i would love to see the solution for app that uses an external plugin but want to watch the url change

gaiottino commented 11 years ago

@leonzinger I was iterating an app which relied on crossroads.js and Handlebars to Angular. It worked great for Chrome/FF/Safari, but IE would have $digest problems described above. I've removed crossroads.js now in favor of using Angular for routing but with a patch I'm hoping will make it's way into master: https://github.com/angular/angular.js/pull/1901

leonzinger commented 11 years ago

@gaiottino Thanks a lot for the answer, i would love to see it working with an outside routing library though...

gaiottino commented 11 years ago

@leonzinger the only part which took some investigation was how to compile Angular specific code. Once you render html with Angular markup you need to compile it. I don't have any code to show but using https://github.com/leshill/handlebars_assets it was basically

template = HandlebarsTemplates[template_name]
html = template()
$(selector).append(html)
compile(selector) if html.has('ng-controller')

Where the compile function is

compile: (selector) ->
  app = angular.element(document)
  compiler = app.injector().get('$compile')
  scope = app.scope()

  elements = angular.element("#{selector} [ng-controller]")
  if elements.exists()
    for el in elements
      element = angular.element(el)
      template = compiler(element)
      html = template(scope)
    scope.$apply() if(!scope.$$phase)

If you plan to render html dynamically you should also destroy any previous controllers that are no longer used

destroy: (selector) ->
  elements = $(selector).find('[ng-controller]')
  for element in elements
    app = angular.element(element)
    if app
      scope = app.scope()
      scope.$destroy() if scope

The app was bootstrapped manually using

$ ->
    angular.bootstrap(document)
rbygrave commented 11 years ago

In case it is useful... I was hitting the "10 $digest() iterations reached. Aborting!" error when using $window.history.back(); with IE9 (works fine in other browsers of course).

I got it to work by using:

setTimeout(function() {
  $window.history.back();
},100);
thebigredgeek commented 11 years ago

Thats not a very good solution though. It isn't guaranteed to work on slower machines. I have ran into a similar issue where I get the digest iteration error on some phonegap devices but not others. Quite bizare. Looking for a work around

Vbahole commented 11 years ago

I'm seeing this as well. Using IE9 and I'm not even using history. I just use $location.search()['searchTerm'] in one of my controllers so that i can capture something from the query string and act on it. Works fine in ff with html5mode=true. in ie9 with mode true it spins then goes to the IIS home page and puts a # in my url. With mode set to false it doesn't perform the qs search.

mezezo commented 11 years ago

I am seeing this on IE9 with angular 1.0.5. Is this by any chance fixed on 1.1.3? Or is there a known workaround?

BrainCrumbz commented 11 years ago

We are experiencing the same error on IE9 when invoking $window.history.back(). We tried with plain "window" and it's the same. At this fiddle http://jsfiddle.net/DGbNp/ you can see the first two functions from the Angular error "stack trace" from IE console error. (We edited the output to remove double quotes and \n 's). For now we could live with dropping completely the "back button", but it's not gonna last long. If anyone needs more info, please just ask.

sahglie commented 11 years ago

I'm also seeing this on IE9 with angular 1.0.5. Sadly this is a show stopper for our team as far as moving ahead with angular. :-(

craftgear commented 11 years ago

Bump, I have the exactly same thing on FF16 and Chrome22 with Angular1.0.5. This is really a pain in ass.

klebba commented 11 years ago

+1 - 1.1.3 / IE9

florianorben commented 11 years ago

Had the same issue recently, manually commenting out 577 - 611 in https://github.com/angular/angular.js/blob/master/src/ng/location.js did do the trick for me... Obviously this isnt the best solution as modifiying angular's source isn't what should be done actually...

What about adding an option to dis-/enable $location's watch on browser url completely.. something like:

angular.module('myApp', []).config('$locationProvider', function($locationProvider) {
    $locationProvider.disable(); //disables/removes all watchers on browser url change
})
cyberwombat commented 11 years ago

+1 IE8/9

glebm commented 11 years ago

$locationProvider.disable() would be a very useful addition. Is there a way to remove a listener from scope?

tonypee commented 11 years ago

Is this being looked at? We are experiencing this exact issue, a solution would be handy - even just the .disable() as we arnt using any routing or location functionality.

EDIT - can confirm that : angular.module('myApp', []).value('$anchorScroll', null); works to remove the dependancies - thanks @dbinit

ghost commented 11 years ago

Bump, very annoying when developing for IE.

pvasek commented 11 years ago

There is similar issue opened (https://github.com/angular/angular.js/issues/2007). Just submitted pull request for that issue https://github.com/angular/angular.js/pull/2782.

ceryash commented 11 years ago

i faced this issue in IE10. have tried all above solutions, but none of them work

Pete1138 commented 11 years ago

I am setting $location.path('somevalue') and getting the same error. It's a pretty big problem as I can't redirect to another location in the controllers.

UPDATE: Fixed it for my particular case. I had <a href="#" ng-click="something()>click</a>. The problem was href="#". Got rid of it and the problem was fixed

RobCherry commented 11 years ago

I would love to see this issue get resolved.

sparmboy commented 11 years ago

+1 I get this issue in Chrome

sparmboy commented 10 years ago

As I work around, instead of:


$location.path( partialURL );

I set the location directly and append the hash manually:


window.location = window.location.origin + window.location.pathname + "#" + partialURL;

Worked for me...

watsocd commented 10 years ago

I am having this issue on a redirect from PayPal. When the user hits the Buy Now button, the browser is redirected to PayPal. I have tried several methods mentioned above with this redirection with no success.

The error occurs on the return from PayPal after the user has approved the payment on the PayPal site.

The return URL from PayPal looks like: http://somewhere.com/paypalReturn?success=true&token=EC-1XT84966V66999999&PayerID=MNQ67EA999999

The return URL bounces right back into my angular APP to show the purchase confirmation. It works flawlessly in Chrome/Win7. It works in IE10/Win7 with a non-fatal $digest already in progress error. In IE8/XP it locks up the browser in an endless loop as described by others.

scesbron commented 10 years ago

In my case I have this issue on several webkit based browsers. I managed to reproduce it with browserstack emulator of iPhone 4 (iOS 4.0) and this plnkr : http://plnkr.co/edit/DFMpOC

The reproduction is not easy, you have to preview the plunkr in a separate window and add a trailing name at the end of the url (for example run.plnkr.co/OHkN002KOzZyrnkh/#/test).

In my case the problem seems to be tied to the use of http backend used in a view resolve phase or in conjunction with a route redirect (this is why you have to add /test in the plunkr url to generate a redirection).

It is not easy to debug in a virtual mobile environment but it seems that the resolve of the http promise trigger an update of the view that triggers an anchorScroll call. Not sure this is the root of the problem but I guess it is.

watsocd commented 10 years ago

I found out right where the error is occurring in angular.js.

Starting at line #5573 in angular.js v 1.0.6. // update browser var changeCounter = 0; $rootScope.$watch(function $locationWatch() { var oldUrl = $browser.url(); var currentReplace = $location.$$replace;

  if (!changeCounter || oldUrl != $location.absUrl()) {
    changeCounter++;
    $rootScope.$evalAsync(function() {
      if ($rootScope.$broadcast('$locationChangeStart', $location.absUrl(), oldUrl).
          defaultPrevented) {
        $location.$$parse(oldUrl);
      } else {
        $browser.url($location.absUrl(), currentReplace);
        afterLocationChange(oldUrl);
      }
    });
  }
  $location.$$replace = false;

  return changeCounter;
});

I think the issue is that changeCounter is being initialized outside the watch and then used inside the watch. I am not a JavaScript expert but I believe the inside the watch is a different scope and changeCounter does not exist inside the scope.

It IE8, the watch runs over and over again because I think changeCounter is not defined properly and does not get incremented with the changeCounter++ line. When I use the IE8 debugger on this code, changeCounter does not appear as one of the local variables.

pvasek commented 10 years ago

Have you had anyone chance to try my changed version of angular.js?

https://github.com/angular/angular.js/pull/2782

I am really curious if it works only for us or for anyone else as well.

scesbron commented 10 years ago

I tried your fix but it does not fix the bug in my case

kstep commented 10 years ago

You are right, you are not a JavaScript expert. The snippet works exactly as it should to. The changeCounter variable is caught into closure into $locationWatch function, and as it's defined in outer scope, it's value is preserved between function calls. It's not a local function variable, but it is visible inside function. If you move changeCounter definition inside the function it will be redefined on each function call, which will make it useless. Please read more about JavaScript functions, closures and scopes.

watsocd notifications@github.com wrote:

I found out right where the error is occurring in angular.js.

Starting at line #5573 in angular.js v 1.0.6. // update browser var changeCounter = 0; $rootScope.$watch(function $locationWatch() { var oldUrl = $browser.url(); var currentReplace = $location.$$replace;

if (!changeCounter || oldUrl != $location.absUrl()) { changeCounter++; $rootScope.$evalAsync(function() { if ($rootScope.$broadcast('$locationChangeStart', $location.absUrl(), oldUrl). defaultPrevented) { $location.$$parse(oldUrl); } else { $browser.url($location.absUrl(), currentReplace); afterLocationChange(oldUrl); } }); } $location.$$replace = false; return changeCounter; });

I think the issue is that changeCounter is being initialized outside the watch and then used inside the watch. I am not a JavaScript expert but I believe the inside the watch is a different scope and changeCounter does not exist inside the scope.

It IE8, the watch runs over and over again because I think changeCounter is not defined properly and does not get incremented with the changeCounter++ line. When I use the IE8 debugger on this code, changeCounter does not appear as one of the local variables.

— Reply to this email directly or view it on GitHub.

martinstein commented 10 years ago

@kstep: I agree with your analysis, but @watsocd was making an honest attempt to help. Maybe the slightly condescending tone towards him/her could have been avoided.

watsocd commented 10 years ago

pvasek,

Your change #2782 fixed the issue for me.

pvasek commented 10 years ago

@watsocd great, thank you for letting me know.

watsocd commented 10 years ago

When will this change be rolled into the next revision?

adamdude828 commented 10 years ago

@pvasek Your change did work for me but am not using as its not merged. I'll check by latter and see if they have merged it. I was getting this error when IE8 when I try to open a new tab that is also part of my application. If I tell angular about the change first and then open the window it doesn't have the problem. (but may cause others)

$location.url(imosOrmanViewEndPoint + '/#orman_vtb'); window.open(imosOrmanViewEndPoint + '/#orman_vtb', windowName);

This is my first angular project, so is their something wrong with this approach?

Edit: never mind Neither works. :)

kencaron commented 10 years ago

Seeing this as well. Did a quick less than ideal workaround involving jQuery and a .lt-ie10 conditional which would set html5Mode to false. I'm only using $location.path and $location.search

IgorMinar commented 10 years ago

fixed by dca23173e25a32cb740245ca7f7b01a84805f43f

danielcha commented 10 years ago

The above patch fixed the errors but broke the history for me. Anyone else seeing this behavior? https://github.com/angular/angular.js/commit/dca23173e25a32cb740245ca7f7b01a84805f43f#commitcomment-3833187

P.S: although it shows alot of errors it seems all seems to work in IE8/IE9 (browser mode, I'm using IE10). I guess for the time being this is better than not having errors and having a broken history...