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

Closed
petebacondarwin wants to merge 2 commits into
angular:masterfrom
petebacondarwin:inf-dig-location-replace-ie9
Closed

petebacondarwin wants to merge 2 commits into
angular:masterfrom
petebacondarwin:inf-dig-location-replace-ie9

Conversation

@petebacondarwin

Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
Contributor Author

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

@hamfastgamgee

Copy link
Copy Markdown

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
Copy Markdown

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

@petebacondarwin

Copy link
Copy Markdown
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
Copy Markdown

Thanks! Please see new version over on #11675.

@hamfastgamgee

Copy link
Copy Markdown

So is this fixed, or...?

@petebacondarwin

Copy link
Copy Markdown
Contributor Author

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

@hamfastgamgee

Copy link
Copy Markdown

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.

Comment thread test/ng/locationSpec.js

Copy link
Copy Markdown
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