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

fix($location): parse query string when path is empty in hashbang mode #5977

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Jan 24, 2014

Before this fix, search queries in hashbang mode were ignored if the hash was not present in the url. This patch corrects this by ensuring that the search query is available to be parsed by urlResolve when the hashbang is not present.

Closes #5964

@petebacondarwin
Copy link
Member

LGTM

@@ -178,6 +178,11 @@ function LocationHashbangUrl(appBase, hashPrefix) {
throw $locationMinErr('ihshprfx', 'Invalid url "{0}", missing hash prefix "{1}".', url,
hashPrefix);
}

if (!withoutHashUrl.length && withoutBaseUrl.charAt(0) === '?') {
Copy link
Member

Choose a reason for hiding this comment

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

BTW, why !withoutHashUrl.length rather than withoutHashUrl === ''? Is this because you want to match null too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's guaranteed to not be null because of the condition above, so it's just a style thing. whatever is preferred works for me

@caitp
Copy link
Contributor Author

caitp commented Jan 31, 2014

If nobody has any comments about my question re: the weird absolute url in the tests, this is probably okay

@@ -1487,6 +1487,30 @@ describe('$location', function() {
expect(location.url()).toBe('/not-starting-with-slash');
expect(location.absUrl()).toBe('http://server/pre/index.html#/not-starting-with-slash');
});

describe("search()", function() {
it("should not return an empty object when url has search query and path is empty", function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you reword this so that we are describing what it should do rather than what it shouldn't do?

Before this fix, search queries in hashbang mode were ignored if the hash was not present in the
url. This patch corrects this by ensuring that the search query is available to be parsed by
urlResolve when the hashbang is not present.

Closes angular#5964

this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
this.$$absUrl = appBase + (this.$$url ? hashPrefix + this.$$url : '');
if (this.$$url) {
url = this.$$path ? hashPrefix + this.$$url : this.$$url;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes the weird looking absUrl()... once the change to the way path() works is merged, this would need to be updated to test that path !== '/'

@tbosch tbosch modified the milestones: 1.2.12, 1.2.11 Feb 3, 2014
@jeffbcross jeffbcross removed their assignment Feb 6, 2014
@jeffbcross jeffbcross modified the milestones: Backlog, 1.2.12 Feb 6, 2014
@IgorMinar
Copy link
Contributor

lgtm

@caitp
Copy link
Contributor Author

caitp commented Feb 22, 2014

Closed by cad717b

@caitp caitp closed this Feb 22, 2014
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.

Should $location extract search params from before the #! ?
5 participants