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

Fixed inconsistency in $location.path() behaviour on the $locationChangeStart event when using back/forward buttons in the browser or manually changing the url in the address bar. #5089

Closed
wants to merge 1 commit into from

Conversation

skoszuta
Copy link
Contributor

This is related to #4989

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@skoszuta
Copy link
Contributor Author

This is my first pull request. I see the commit message does not exactly follow the rules, so I'll fix it in a moment.

@skoszuta
Copy link
Contributor Author

Commit message fixed. How do I sign the CLA?

@caitp
Copy link
Contributor

caitp commented Nov 22, 2013

There is a link in CONTRIBUTING.md (https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#signing-the-cla)

Caitlin Potter
[email protected]

On Nov 22, 2013, at 9:00 AM, Sebastian [email protected] wrote:

Commit message fixed. How do I sign the CLA?


Reply to this email directly or view it on GitHub.

@skoszuta
Copy link
Contributor Author

I have signed the CLA. My name is Sebastian Koszuta

@matsko
Copy link
Contributor

matsko commented Nov 22, 2013

@skoszuta please add some test code to this.

@petebacondarwin
Copy link
Contributor

+1 @matsko
@skoszuta - ideally you should have a test that demonstrates the bug you are fixing.

@skoszuta
Copy link
Contributor Author

Sure. I'll push some tomorrow.

@phelipemaia
Copy link

I think locationChangeStart should be executed before everything when you're changing the URL. Am I wrong?

@skoszuta
Copy link
Contributor Author

Alright. I pushed a test for this. Now, whether the $location should contain the old or new url during $locationChangeStart is something we can discuss. But it's a sure thing that it should be consistent. It would be great if one could access the old and the new url in their parsed forms when $locationChangeStart occurs. "Parsed" as in similar to a raw route, like: /bla. As for now only full URLs are passed to the event listener.

…is triggered by the browser

Fixed inconsistency in $location.path() behaviour on the $locationChangeStart event when using
back/forward buttons in the browser or manually changing the url in the address bar.
$location.path() now returns the target url in these cases.

Closes angular#4989
@IgorMinar
Copy link
Contributor

thanks. I improved the test and merged this in.

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…is triggered by the browser

Fixed inconsistency in $location.path() behaviour on the $locationChangeStart event when using
back/forward buttons in the browser or manually changing the url in the address bar.
$location.path() now returns the target url in these cases.

Closes angular#4989
Closes angular#5089
Closes angular#5118
Closes angular#5580
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
…is triggered by the browser

Fixed inconsistency in $location.path() behaviour on the $locationChangeStart event when using
back/forward buttons in the browser or manually changing the url in the address bar.
$location.path() now returns the target url in these cases.

Closes angular#4989
Closes angular#5089
Closes angular#5118
Closes angular#5580
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.

8 participants