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

Behavior of $location.path() inconsistent when back and forward buttons are used #4989

Closed
wilsonjackson opened this issue Nov 17, 2013 · 5 comments

Comments

@wilsonjackson
Copy link

Example plunk: http://plnkr.co/edit/DA3Oq6?p=preview

In a handler for the $locationChangeStart event, a call to $location.path() will normally return the URL being navigated to. But if you use the forward or back button (or use history.back() or history.forward()) it will return the URL being navigated from.

This isn't the case with the $locationChangeSuccess event, which behaves as expected.

@jeffbcross
Copy link
Contributor

Good catch. It looks like this issue has been around since 1.1.5 (possibly earlier).

@phelipemaia
Copy link

Well, I don't think that case is a bug.

When you call $location.path inside of locationChangeStart, you are getting the current path.

Let me use your plunk:

Click on Test 1 and then click on Test 2:

**$locationChangeStart ---**
$location.path(): /test2
newUrl: http://run.plnkr.co/test2
oldUrl: http://run.plnkr.co/test1

**$locationChangeSuccess ---**
$location.path(): /test2
newUrl: http://run.plnkr.co/test2
oldUrl: http://run.plnkr.co/test1

Now, if you click on Back link, the $locationChangeStart will show Test 2 on location.path, because you're still in test2 page. $locationChangeStart will run before everything else, including the $browser.url that really change the URL on location.js and locationChangeSuccess is being executed after everythin so will show the new page.

So, after click on back, you will see:

**$locationChangeStart ---**
$location.path(): /test2
newUrl: http://run.plnkr.co/test1
oldUrl: http://run.plnkr.co/test2

**$locationChangeSuccess ---**
$location.path(): /test1
newUrl: http://run.plnkr.co/test1
oldUrl: http://run.plnkr.co/test2

Location.path on locationChangeStart is showing test2, but the current page is: test1.

So, now if click on forward, you will have:

**$locationChangeStart ---**
$location.path(): /test1
newUrl: http://run.plnkr.co/test2
oldUrl: http://run.plnkr.co/test1

**$locationChangeSuccess ---**
$location.path(): /test2
newUrl: http://run.plnkr.co/test2
oldUrl: http://run.plnkr.co/test1

Because your current page is test1 and you are going to test2. The code on locationChangeStart was not executed yet, so you will see test1 (current page) when call location.page.

@wilsonjackson
Copy link
Author

@phelipemaia Your premise, "When you call $location.path inside of locationChangeStart, you are getting the current path", is invalidated by your first example. $location.path() doesn't return the current path when you click on a link, it returns the destination path. Only in a forward/back navigation does it return the current path.

Notice how during a link click the values of $location.path(), newUrl and oldUrl are precisely the same between $locationChangeStart and $locationChangeSuccess, but in a forward/back navigation $location.path() is different.

I'm not expressing any opinion about which is correct, just pointing out the inconsistency, which very much seems like a bug.

@phelipemaia
Copy link

Ok, let's take a look on location.path doc:
This method is getter / setter.

Return path of current url when called without any parameter.

Change path when called with parameter and return $location.

Note: Path should always begin with forward slash (/), this method will add the forward slash if it is missing.

As you can see, it will return the current URL.
So, in my opinion the problem is on all links. They should show location.path value as back link is showing.
When you call history.back, the function $browser.onUrlChange is being executed. When you are clicking on other links that doesn't use history, angular will execute $rootScope.$watch. When you are in $rootScope.$watch the path was already changed. In my opinion, if you access $locationChangeStart even in $rootScope.$watch, you should get the current path (URL of where you are, not the URL that you're going), because you're in a event that is still in current path, you didn't changed your location yet (you're starting to move).
You're right, are two differents behaviors for the same thing. But, to understand:

  • on history functions (back/forward): $locationChangeStart will be executed before location change the path to the new URL
  • on normal links: $locationChangeStart will be executed after location change the path to the new URL

I will create a pull request and we will see what angular team will say about that....

@skoszuta
Copy link
Contributor

I noticed it's also the case when you manually change the path in the browser's address bar. It turned out my auth service checked authorization for the url being navigated from in that case.

skoszuta pushed a commit to skoszuta/angular.js that referenced this issue Nov 25, 2013
…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
@ghost ghost assigned vojtajina Dec 2, 2013
@ghost ghost assigned IgorMinar Dec 30, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue 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 issue 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 a pull request may close this issue.

6 participants