fix($location): links should respect absolute paths and base href when in html5mode

[chrisirhc]

Fixes absolute links on legacy browsers when html5Mode is enabled.

Clicking on a link out of the base's href shouldn't trigger rewriting the URL. This behavior was changed since 24f7999 .

This resolves the issue I found on 24f7999#commitcomment-7815179 which has created different behavior when it comes to absolute links on legacy browsers.

Most of this is whitespace changes.
I've added these cases to consider:

1. Properly rewrite links starting with /base/ when the base tag is present with a value of /base/.
2. Don't rewrite links that have a href beginning with / not in within the base (i.e. /outer_base/link)
3. Don't rewrite links at all if no base tag is present and href begins with /
4. Rewrite absolute links with hashbang followed by / properly (.i.e. #!/base/thing).

This change is meant to resolve those cases that were not tested since 1.2.17 and have mismatching test cases. It's not to add any new features.

[caitp]

this isn't going to be merged IMHO, it will make replacing the current broken behaviour with the correct behaviour more difficult.

@tbosch I'll be unavailable most of the day, could you review this and see if it's worth landing as an intermediate solution? If not I'll get to it when I'm back home

[chrisirhc]

I've updated this PR so that it behaves correctly with base href. I've also added tests. This should now be fully consistent with the URL rewriting logic throughout $location.

I don't see how this change will make replacing the broken behavior more difficult since this code change only makes changes to code that was added in e020366 and 24f7999 which are removed on 2294880 anyway.

[chrisirhc]

This should also be forward compatible with changes in the spec in 2294880 and resolves #8233 and #8172 without requiring a <base> tag.

[chrisirhc]

Also, just verified that this fixes the docs website on IE9.

[chrisirhc]

I just read through #8233, I see the concern, I was making the assumption that the base href wouldn't be an absolute URL with protocol. I'm working on a solution that will consider that.

All in all, my intention is to fix existing location regressions 1.2.x without requiring base tags as it's a breaking change.

[caitp]

the docs site was verified to be working with this original fix in IE9, we tested this in SL extensively. The problem is that it breaks many peoples actual apps, in different ways. Basically it doesn't play nicely with a base tag at all, and it never resolved relative URLs correctly (or consistently with the way a history-enabled browser would resolve them).

So, requiring a base tag kind of sucks, but there isn't much else we can do to ensure consistent behaviour. It is sad that the base tag and app base are so tied together, but that's the way it is right now.

I've asked anne what he'd think about adding some kind of support for multiple base tags (similar to xml:base), but I don't think it was a very popular idea --- it's the kind of thing nobody really wants to support, url resolving in browsers is already ridiculously complicated and it would just make things worse. So unfortunately there's probably no way to do this that makes everyone happy.

We can certainly look at it and try it, but I think the most likely scenario is going to be the breaking change, or just reverting the fix entirely and using the default url resolution always

[jeffbcross]

I'll self-assign this one since @tbosch and I are discussing some changes to $location for the next release.

[chrisirhc]

I just looked through this PR again and found the following cases this PR doesn't handle yet:

1. base href that has a different domain than the current page's domain. In this case, all relative paths shouldn't be rewritten (they should invoke browser navigation) unless they begin with the hashbang prefix.
2. base href that is a relative path that doesn't begin with /. In this case, the href should be resolved first, and the pathname should be used instead.

For case 2, it might make sense to always urlResolve the base href since if it begins with /, it should still be correct.

I have a few questions:

1. Is there a reason document.baseURI isn't used? That should contain the resolved base href value. Was this not used because of cross-browser issues?
2. Is there a possibility of offering a ng-base-href directive that sets the base href of the app as an alternative to using a base tag?
   This way, we can get around all the issues of base tags for user's that can't use base tags due to their setup (SVGs etc.), but still offer an ability to set the base URL elsewhere.

[caitp]

It would be nice to have an ng-base directive or something in order to set up the appBase declaratively.

Unfortunately, this doesn't help us when it comes to resolving URLs correctly. In order to do this consistently, we really need to use URLUtils --- otherwise the difference between the real url and the "virtual" url just completely mess things up.

[chrisirhc]

How about shimming URLUtils' URL(url, base) constructor? This way, we can use an arbitrary appBase (ng-base), and we can also have one that's aware of hashbang URLs for legacy browsers.

As long as there's no ambiguity in what the virtual URL is by rewriting the current URL, it shouldn't be too messy. This of course has to take into the account the value of the base href (it's absence means the current url is the base url). Assuming we have that URL function, this is what finding a href looks like:

// On load
var documentBaseUri = hasNgBase ? URL(baseHref, initialUrl) : document.baseUri;

function rewrite(href) {
    var baseUrl = documentBaseUri || currentUrl;
    var newUrl = URL(url, baseUrl);

    if (hasBaseTagOrNgBase) {
        return inAppBase(newUrl) && newUrl;
    } else {
        return newUrl;
    }
}

I looked briefly into Apache's httpcomponents' URIUtils which I'm guessing is one of the more mature implementations of it. It looks like the algorithm we have in e020366 is doing what the normalizeSyntax function does. Of course, copying that implementation probably isn't a bad idea either.

[chrisirhc]

Just for clarification, this is the behavior I'm assuming right now. Do let me know if my assumptions are incorrect or if I missed some cases.

With base href: /base/ (or http://example/base/)

The following all navigate to http://example/base/abc

Hashbang URLs are always resolved to appBase. It doesn't make sense to have a hashbang href resolves to somewhere out of the application.

Current Url	href
http://example/base/	#!/abc
http://example/base/	#!abc
http://example/base/	#!./abc
http://example/base/	#!../base/abc
http://example/base/a	#!./abc
http://example/base/b/	#!../abc
http://example/base/b/c/	#!/abc
http://example/base/	/base/#!/abc
http://example/base/	/base/abc
http://example/base/	abc
http://example/base/	./abc
http://example/base/	../base/abc
http://example/base/	../base/#!abc
http://example/base/a	./abc
http://example/base/b/	../abc
http://example/base/b/c/	/base/abc

With no base href

Edit: Removed previous examples for case when no base href is set. When no base href is set, the app's base is set to the root.

There's no need to handle cases where the base href is pointing to another domain because no URL rewriting should occur in those cases, all relative links should navigate away to external URLs. In that case, only an absolute href to the current app itself can invoke navigation within the app.

[chrisirhc]

I've updated this PR with the URLResolve shim approach. This makes the logic much cleaner and more consistent with the html5 url API. In fact, there's now code shared with LocationHtml5Url to find the absUrl (html5Url).

The urlResolveShim method is intended to behave like the URLUtils's URL(url, base) constructor, taking both a url and a base argument.

I saw that the urlUtils.js's urlResolve wasn't respecting the base argument. This also does normalization of the path for IE7. The normalizePath method can be removed for >= 1.3 versions of AngularJS since IE7 is no longer supported there.

[chrisirhc]

I'm done with working on this PR for now, and hope to hear some feedback on it.
If the feedback is that it's not going to be merged, I won't work on it further.

[tbosch]

Hi,
thanks for the PR, but we will not merge this PR as the code that this affects was already removed in master and will also soon be removed in v1.2.x as it was wrong in the first place. Reverting those code passages will actually also solve your initial problem.

An ng-base directive will be really hard to write, as the <base> tag not only affects links but also img, link, and other elements that take a url...

Regarding requiring the <base> tag:

• we won't require it on 1.2.x as it would be a breaking change
• in 1.3.x there is an additional parameter to html5mode now that allows to disable this check for people that know that they are doing (e.g. they know that their app will only run in modern browsers with history support).

Closing this...