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

fix($location): Infinite digest in $location replace in IE9 #11935

Conversation

petebacondarwin
Copy link
Contributor

@hamfastgamgee - I think this is the right unit test. Can you take a look. The nice thing is that it even fails on Chrome as well (since we are simulating no-history support) without the fix.

@hamfastgamgee
Copy link

Well, this is certainly a much more concise version of what I was directly fixing. I'm going to update my patch in the other pull request with something I was knocking around this morning that I'd like you to ponder. I believe the problem is somewhat more widespread than previously thought (impacting more cases) because the mock browser didn't expose the bug.

@petebacondarwin
Copy link
Contributor Author

Perhaps we do need to update ngMock browser across the board as you say?

@hamfastgamgee
Copy link

Seems at the least like it would be a more honest accounting of what's really going wrong in the app, which should lead to more bullet-proof fixes. :)

@hamfastgamgee
Copy link

Is there an easy way to run the karma tests on another browser locally? (I'm relatively new to grunt.)

@petebacondarwin
Copy link
Contributor Author

Easiest way to run tests locally is to use grunt autotest. This will setup karma to watch the source and test files and automatically run the tests on any change. Once this is running, you can then attach additional browsers by navigating them to http://localhost:9876

@hamfastgamgee
Copy link

Thanks! Please see new version over on #11675.

@hamfastgamgee
Copy link

So is this fixed, or...?

@petebacondarwin
Copy link
Contributor Author

Moving focus to #11675. I will try to land this in the next couple of days :-)

@hamfastgamgee
Copy link

Gotcha. Thanks again for all your help!
On Jun 1, 2015 7:04 AM, Pete Bacon Darwin notifications@github.com wrote:Moving focus to #11675. I will try to land this in the next couple of days :-)

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

$rootScope.$on('$locationChangeSuccess', function() {
if ($location.path() !== '/') {
$location.path('/').replace();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

add an assertion here

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Jun 11, 2015
petebacondarwin added a commit that referenced this pull request Jun 12, 2015
Thanks to @hamfastgamgee for getting this fix in place.

Closes #11439
Closes #11675
Closes #11935
Closes #12083
petebacondarwin added a commit that referenced this pull request Jun 12, 2015
Thanks to @hamfastgamgee for getting this fix in place.

Closes #11439
Closes #11675
Closes #11935
Closes #12083
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants