-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix($location): path to work with full url #8924
base: master
Are you sure you want to change the base?
Conversation
What is the use case really? It seems kind of bad because the hostname/protocol shouldn't be an important part of the app (it's really just a deployment detail). Just my opinion though :> |
@caitp See the referenced case for the use case, where you commented on this same issue previously. |
This isn't a real use case though, the current design doesn't prevent this from happening. No use case is provided for changing the app location using the full URL |
I can assure you it's a real (and real common) use case. I must be missing something. You seemed to agree with this before, now you seem to disagree. What am I missing? It's a fact that the very url the event provides is not usable to continue navigation so there's a fundamental issue here no matter the use case. |
I never said I agree that it's a good use case, I said it's kind of annoying that we don't let you set an absolute URL --- but there are good reasons for this. Anyways, the decision on whether to include this in the framework isn't up to me really, however I can think of good reasons why we wouldn't want to support that. |
Interesting, I feel that you might not fully understand what is going on here or have a skewed view of it that I can't quite fathom. I view it as a straightforward inconsistency and design mistake in implementing the API, but I have a workaround and more pressing issues to attend to, so I'll leave it to the collective wisdom of the crowd. |
I assure you, I understand what you're trying to do --- but one should not be specifying full urls here, it just doesn't make any sense (in my opinion). There are other workarounds, like replacing the application base url with the empty string in the event handler. I think that is probably a much better (safer) solution to this problem than what is implemented currently in this PR. |
@caitp hostname and protocol is used :
Why i am not checking if this is just the full path with hostname ? It will be good to navigate using the full url in the $location.path(). |
@caitp You mentioned other work around will be to replace the application base url with empty string. are you mentioning to event handler for the event locationChangeStart. sorry i am quite new to angular if i am missing something. in solution you proposed Just to understand the solution, we only have access to event object, newUrl and oldUrl. are you proposing to create a new Location object and pass the empty string in appBaseUrl and then use There is some method in event object and it does the parsing properly. Thanks for commenting and reviewing the PR 👍 |
2eeea57
to
51c198a
Compare
I'm saying the path() routine could replace the appBase with the empty string, if found --- because that is the only full url that should ever be used |
02dc2aa
to
fd2d6c0
Compare
51c198a
to
fc9976d
Compare
Changed the way $location.path is handled. Now path function will check if the path contain the base url (protocol://host) is this is there in the path. Mostly user has passed the full url instead of path. We process it differently, by calling the parse method of the respective location object. Since parse method takes care if the url is HTML5 mode , HTML5 hashbang mode or non HTML5 urls and parses them respectively. Currently i have made directly used the parse function which updates the whole location object. We can also ass a parsePath method to each of LocationHashbangInHtml5Url,LocationHashbangUrl, LocationHtml5Url classes which will copy implementation from parse method and get the path out of newUrl but won't update the location object. If this implementation have some issue i can work on making this change. Closes angular#8617
fc9976d
to
0b85ae0
Compare
cad9560
to
f294244
Compare
e8dc429
to
e83fab9
Compare
4dd5a20
to
998c61c
Compare
Changed the way $location.path is handled. Now path function will check if the path contain the base url (protocol://host) is this is there in the path. Mostly user has passed the full url instead of path.
We process it differently, by calling the parse method of the respective location object. Since parse method takes care if the url is HTML5 mode , HTML5 hashbang mode or non HTML5 urls and parses them respectively.
Currently i have made directly used the parse function which updates the whole location object. We can also ass a parsePath method to each of LocationHashbangInHtml5Url,LocationHashbangUrl, LocationHtml5Url classes which will copy implementation from parse method and get the path out of newUrl but won't update the location object. If this implementation have some issue i can work on making this change.
Closes #8617