$locationChangeStart handler called outside of $apply/$digest

[provegard]

I just upgraded to 1.0.8 and noticed that $locationChangeStart is now broadcast when the user hits the back button.

However, the behavior is inconsistent with normal location changes wrt $apply/$digest. Normally, the event handler for $locationChangeStart is called within $apply/$digest, but that's not the case when the user hits the back button.

Demonstration of the problem:

http://plnkr.co/edit/EiZVMK?p=preview

[petebacondarwin]

Is this a duplicate of #5089

[caitp]

So, since this is assigned to me, it should be pretty trivial to fix. However, it's not necessarily trivial to test the fix in an automated test.

Ignoring the fact that you'd have to use private API to do it, is there any reason you wouldn't want to just call $scope.$apply() as needed on $locationChangeStart if you're updating models?

I'll submit a pull-request but again I'm not totally sure how to test this currently (although it would be good to learn!), so... yeah.

[provegard]

My current workaround in my $locationChangeStart handler is:

# Workaround for https://github.com/angular/angular.js/issues/5118
# Force a $apply cycle, but simply calling $apply/$digest won't work.
unless $rootScope.$$phase
    $timeout(noop, 0)

Let's see if I can put together a fiddle/plunkr showing why a simple $apply didn't work.

[IgorMinar]

that workaround should have no effect on anything. if you do see some issue it's likely due to something else.

please take a look at this plunker and see how the counter is updated correctly after each change (even when you use the back button: http://plnkr.co/edit/R7Jdmd?p=preview

As Pete mentioned, this report if at all valid is a duplicate of #5089

the reason why you get phase == null in your tests is that we use $digest instead of $apply in the $location code. this is not ideal, but is no reason for panic as $digest will behave as $apply in this case.

[caitp]

@IgorMinar it is a related bug, not quite a duplicate as they refer to different symptoms of the same thing --- however looking at the fix in 5089 there are some changes in there which are sort of questionable. I'm not sure it's right to put the $locationChangeStart event in evalAsync.

Tests are passing, but that doesn't seem right.. but then again I suppose if a digest were already in progress, then it would have the same effect as other cases, so maybe it's not a big deal.