-
Notifications
You must be signed in to change notification settings - Fork 27.5k
feat($location): test $location enhancements from #6421 and fix them #6899
Conversation
@@ -621,6 +631,38 @@ function $LocationProvider(){ | |||
absHref = urlResolve(absHref.animVal).href; | |||
} | |||
|
|||
// Make relative links work in HTML5 mode for legacy browsers (or at least IE8 & 9) | |||
// The href should be a regular url e.g. /link/somewhere or link/somewhere or ../somewhere or somewhere#anchor or http://example.com/somewhere | |||
if (LocationMode === LocationHashbangInHtml5Url) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it would make sense to have a "preprocessing" method in LocationMode, so that this code could be removed from here, and more easily extended for the other location modes. I guess it's sort of like a special version of $$rewrite that wants the relative url rather than the absolute
…x them This CL fixes problems and adds test cases for changes from angular#6421. Changes include fixing the algorithm for preprocessing href attribute values, as well as supporting xlink:href attributes.
|
||
this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash; | ||
// include hashPrefix in $$absUrl when $$url is empty so IE8 & 9 do not reload page because of removal of '#' | ||
this.$$absUrl = appBase + hashPrefix + this.$$url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we cause the page reload here? when an app uses routing, the path is always set, so this override shouldn't change anything.
if the path is not set then we should investigate why.
the original thinking was that we don't want to append '#' in hashbang mode unless you actually use $location, otherwise you end up with apps that change the url needlessly, which is something we wanted to avoid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line was added by the original PR, so I think we have to ask them. I don't have access to IE9 to debug that easily except on SL, so it's hard for me to say.
ok, I think we resolved everything in person. let's get this in. |
Closed by 3f04770 |
…Mode Previously, LocationHashbangInHtml5Url, which is used when html5Mode is enabled in browsers which do not support the history API (IE8/9), would behave very inconsistently WRT relative URLs always being resolved relative to the app root url. This fix enables these legacy browsers to behave like history enabled browsers, by processing href attributes in order to resolve urls correctly. Closes angular#6162 Closes angular#6421 Closes angular#6899 Closes angular#6832 Closes angular#6834
…Mode Previously, LocationHashbangInHtml5Url, which is used when html5Mode is enabled in browsers which do not support the history API (IE8/9), would behave very inconsistently WRT relative URLs always being resolved relative to the app root url. This fix enables these legacy browsers to behave like history enabled browsers, by processing href attributes in order to resolve urls correctly. Closes #6162 Closes #6421 Closes #6899 Closes #6832 Closes #6834
This is temporary, just to run a travis build and hopefully make the patch easier to review.