fix($location): parse query parameters which precede hash in hashbang mode #7245
Conversation
| var withoutHashUrl = withoutBaseUrl.charAt(0) == '#' | ||
| ? beginsWith(hashPrefix, withoutBaseUrl) | ||
| ? tmp || (isUndefined(tmp) ? (withoutBaseUrl[1] === '?' && withoutBaseUrl.slice(1)) : '') |
There was a problem hiding this comment.
I'm not sure of a good way to word this, it looks pretty horrible no matter how it's written
The algorithm is basically, "if we don't start with hashPrefix at all, and we have a ? after the hash, treat it as a query, otherwise it's an invalid hash prefix"
There was a problem hiding this comment.
So, LocationHashbangInHtml5Url still uses LocationHashbangUrl#$$parse(). All of the old tests are still passing, but this potentially changes behaviour a tiny bit for hashbang mode. I'm not totally sure in what way, though.
|
The last update to this fix introduced a breaking change, however to be honest, this doesn't seem like it's breaking anything desirable, because the original search query from the base URL wouldn't be parsed by the app anyways. This is very awkward and not really usable. Because of this, I don't really believe it's a big deal to break that. |
|
right, I'll change this to apply to html5mode only, and push it to the next beta |
| url = new LocationHashbangInHtml5Url('http://server/base', '#!'); | ||
| url.$$parse(url.$$rewrite('http://server/base?a=b#!?c=d')); | ||
| expect(url.search()).toEqual({ | ||
| 'a': 'b', |
There was a problem hiding this comment.
this doesn't look right to me. if the url contains # then we should ignore ?c=d since that's the fragment part.
the conversion from html5 urls to hashbang urls should parse the url into segments and then remap the proper segments into the virtual namespace with url fragment.
given http://server/base?a=b#!?c=d as html5 url, it should be parsed into:
- base:
"/base" - path:
"" - query:
{a: 'b'} - hash:
"!?c=d"
to convert this into the hashbang url we should end up with:
http://server/base#?a=b#!?c=d
the parsed representation of this url is the same as what we had for html5 url.
There was a problem hiding this comment.
in legacy html5 mode, the hash prefix isn't treated as the hash fragment, because we only look at the portion of the url following it, so it's like it's not actually there at all
…5mlMode This fix causes LocationHashbangInHtml5Url#$$rewrite() to move a pre-hash query string to after the hash. This, in turn, causes urlResolve() to correctly parse the query string, and report the correct dictionary. BREAKING CHANGE Previously, LocationHashbangInHtml5Url would keep query parameters from the initial URL preceeding a hash symbol as part of the application base url. This is no longer the case, as it is desirable to parse these as part of the search query. To migrate, one should create special urls rather than relying on query string, for instance, if a base url was previously "http://server.com/base?ie=true", it would be adviseable to instead use "http://server.com/base/ie" as the base url. Closes angular#7239
petebacondarwin
force-pushed
the
master
branch
from
02dc2aa to
fd2d6c0
Compare
This fix causes
LocationHashbangUrl#$$rewrite()to move a pre-hash querystring to after the hash.
This, in turn, causes urlResolve() to correctly parse the query string, and
report the correct dictionary.
Closes #7239