Relative links treated like absolute links in html5Mode with legacy browsers in 1.2.12 #6162

richardcrichardc · 2014-02-06T22:27:04Z

In legacy browsers such as IE9, relative links are treated like absolute links in html5mode.

E.g. in Chrome:
The link subdir/ from http://angular-relative-links.richardc.net/ goes to http://angular-relative-links.richardc.net/subdir/ and,
the link subdir/ from http://angular-relative-links.richardc.net/subdir/ goes to http://angular-relative-links.richardc.net/subdir/subdir/ ...

As expected.

However In IE9:
The link subdir/ from http://angular-relative-links.richardc.net/ goes to http://angular-relative-links.richardc.net/#/subdir/ but,
the link subdir/ from http://angular-relative-links.richardc.net/#/subdir/ goes to http://angular-relative-links.richardc.net/#/subdir/, i.e. it stays on the same page.

I have a minimal example to demonstrate this at: http://angular-relative-links.richardc.net/

To test this, you need IE9 (or presumably another legacy browser that does not support history.pushState). You can download a Windows WM with IE9 from Microsoft from here: http://www.modern.ie/en-us/virtualization-tools#downloads

Here is the source code of the minimal example:

<!doctype html>
<html ng-app="test">
  <head>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.11/angular.min.js"></script>
    <script>
        angular.module('test', [])
            .config(function ($locationProvider) {
                $locationProvider.html5Mode(true);
            })
            .controller('MyCtrl', function($location) {
                // empty controller using $location so location services runs 
            })
    </script>
  </head>
  <body ng-controller="MyCtrl">
    <div><a href="subdir/">subdir/</a></div>
  </body>
</html>

Also in Chrome in html5mode hash links are treated as regular links when $location.path() is '/'. Click on the #hash on my test page http://angular-relative-links.richardc.net/ to see. When you click on #hash a second time it starts behaving as expected.

The text was updated successfully, but these errors were encountered:

caitp · 2014-02-06T22:37:36Z

It would be super helpful if you could write a unit test for this that fails on IE9, and put that in the location tests. Could you set up a pull request with a failing test? I'd like to see what browsers it is failing on, and maybe get some idea of how it can be solved

richardcrichardc · 2014-02-06T23:31:45Z

Hi caitp,

I'm sure it would be super helpful. However it's probably a couple of days for me getting up to speed with core angular development before I can even start trying to write a unit test. It's probably a 15 minute job for a staffer who deals with the internals of angular on a regular basis.

I've manually tested it in IE8, and get the same behaviour.

Richard

caitp · 2014-02-09T20:50:32Z

@richardcrichardc is this is a regression, as in does not affect older versions of angular?

But actually, I think this is the expected behaviour for hashbang urls. I think I see what's happening here. Anyways, there's a chance I may be able to improve on this, let me see.

You know what, it's a lot of work to make this work consistently, we sort of need to fake the history API, as far as I can tell, and that's probably not going to happen.

There might be a polyfill out there somewhere, though. https://github.com/Modernizr/Modernizr/wiki/HTML5-Cross-Browser-Polyfills#wiki-html5-history-api-pushstate-replacestate-popstate

richardcrichardc · 2014-02-09T21:27:33Z

@caitp I do not know if it affects older versions of angular. I have only tested this on 1.2.11. If you can suggest an earlier version you'd like this tested against I can modify my test site and manually test.

I have also noticed that links to anchors within a page do not work in html5mode for legacy browsers. This is not surprising as the legacy workaround for no history.pushState() uses the hash link usually used for anchors. But on the other hand the $anchorScroll module exist, which suggests this is a feature angular considers important. You don't happen to know whether there anchors are supposed to work in html5mode for legacy browsers?

caitp · 2014-02-09T21:36:43Z

@richardcrichardc, hash links don't work at all without some extra work, if the $location service gets instantiated, because it will preventDefault() on click events to hrefs which begin with the app url (and the browser gives us the full URL for hash urls).

You can simulate that functionality with ng-click + $anchorScroll, but yeah it won't work on its own if the $location service gets instantiated.

richardcrichardc · 2014-02-09T21:44:09Z

@caitp. Thanks for the info. I am using a custom router which calls $anchorScroll after loading a page, so I was unsure what happened with the default routers.

caitp · 2014-02-09T21:47:38Z

There's been some talk about merging $anchorScroll and $location together, so in ~1.3 that might not be an issue.

The history API behaviour inconsistency is a bit more of a problem, so it would be good to make a note of this in the documentation and provide links to one of those polyfills.

If you'd like to submit a patch to the documentation, I strongly encourage it. I think one of those polyfills should solve the issue here.

richardcrichardc · 2014-02-09T22:13:12Z

What polyfills are those precisely?

I've also just noticed that in Chrome in html5mode hash links are treated as regular links when $location.path() is '/'. Click on the #hash on my test page http://angular-relative-links.richardc.net/ to see. When you click on #hash a second time it starts behaving as expected.

So do you think fixes to these kind of problem will be considered as 'breaking changes'?

caitp · 2014-02-10T18:01:55Z

@richardcrichardc The polyfills are for emulating the history API at some level or another. I'm not positive that it would work properly, but the issue relates to the browser not really understanding what page it's on, and resolving the link as relative to the current path (which, as far as the browser is concerned, is not a subdirectory).

richardcrichardc · 2014-02-10T20:21:56Z

@caitp How do we get this bug assigned to a milestone? I don't know what the angular team's processes are, but I cannot help but think it will be off the radar if it is not in a milestone.

caitp · 2014-02-10T20:23:53Z

We'll discuss it today either during the meeting or during triage. I'm not totally sure what I'd call this, I mean it is a problem, but we've been okay for a long time without worrying about this, and it's not totally clear how to fix this.

richardcrichardc · 2014-02-10T20:28:34Z

Great, I will update the bug report with the #hash treated as ordinary link issue.

richardcrichardc · 2014-02-23T22:24:57Z

I've made a fix. However it is not 'merge ready', so I've supplied it below as a diff from the 1.2.13 branch. It might be a good starting point for whoever takes care of this issue.
@caitp

It would be great to know what what sort of timeframe to expect to see this bug fixed in an official release.

I really think you need to fix this fairly quickly, or document the fact that there is a bug, so people starting new projects can make informed decisions about using Angular. I don't think people expect to have to check through the bug database to find out whether whether the stable release of Angular does what the documentation says it does. I think the expectation is that bugs will be fixed in the stable version in a timely manner. It really isn't fun realising at the end of the project that the framework you choose is not as IE8/9 compatible as documented. Maybe I have got things wrong, but the timeline of comments on #2186, suggests there is not much focus on fixing/documenting Internet Explorer bugs in a timely manner.

Some notes about the patch:

It fixes the issue with relative paths
It does so in a way that anchors work if AnchorScroll is called after the location change
It also fixes a bug where the browser reloading the page completely when navigating to the root of the app
- I spent about four hours tracing through the code looking for the bad code that forced a reload - it does not exist - the problem is that IE reloads the page if the URL changes and the new URL does not contain a #
Despite what the comments say, I have not tested it in IE8 - our app happens to completely crash IE8, so we have decided not to support IE8 at this time
I have only tested in in HTML5 mode
The changes cause the unit tests fail
I'm not really able to do a better job, as I do not understand what much of the code is intended to do. The exact purpose of $$rewrite in particular has me somewhat confused. I really think this is a job for whoever wrote the code in the first place.
From what I do understand, it appears things could be simplified a bit by using the actual href as supplied in the html, instead of the absolute Href supplied by the browser, this is certianly necessary to make relative links work in legacy browsers

diff --git a/src/ng/location.js b/src/ng/location.js
index c4b7545..131917b 100644
--- a/src/ng/location.js
+++ b/src/ng/location.js
@@ -268,6 +268,16 @@ function LocationHashbangInHtml5Url(appBase, hashPrefix) {
       return appBaseNoFile;
     }
   };
+
+  this.$$compose = function() {
+    var search = toKeyValue(this.$$search),
+        hash = this.$$hash ? '#' + encodeUriSegment(this.$$hash) : '';
+
+    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;
+  };
+
 }


@@ -621,6 +631,35 @@ 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) {
+        // get the actual href attribute - see http://msdn.microsoft.com/en-us/library/ie/dd347148(v=vs.85).aspx
+        // TODO check browser is in standards mode
+        var href = elm[0].getAttribute('href');
+        
+        if (href.indexOf('://' == -1)) {         // Ignore absolute URLs 
+          if (href[0] == '/') {
+            // absolute path - replace old path
+            absHref = serverBase(absHref) + href;
+          } else {
+            // relative path - join with current path 
+            var stack = $location.path().split("/"),
+              parts = href.split("/");
+            stack.pop(); // remove top file
+            for (var i=0; i<parts.length; i++) {
+              if (parts[i] == ".")
+                continue;
+              else if (parts[i] == "..")
+                stack.pop();
+              else
+                stack.push(parts[i]);
+            }
+            absHref = serverBase(absHref) + stack.join("/");
+          }
+        } 
+      }
+
       var rewrittenUrl = $location.$$rewrite(absHref);

       if (absHref && !elm.attr('target') && rewrittenUrl && !event.isDefaultPrevented()) {

caitp · 2014-02-23T22:34:49Z

@richardcrichardc, please submit a pull request with your changes so that they're easier to review, I'll do some review today.

The next release is next friday, IIRC, as we're now in a bi-weekly release cycle, and getting ready to focus primarily on Angular 2.0. But we haven't forgotten about the 1.x iteration and will certainly make an effort to fix issues here.

…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

…` url BREAKING CHANGE (since 1.2.0 and 1.3.0-beta.1): Angular now requires a `<base>` tag when html5 mode of `$location` is enabled. Reasoning: Using html5 mode without a `<base href="...">` tag makes relative links for images, links, ... relative to the current url if the browser supports the history API. However, if the browser does not support the history API Angular falls back to using the `#`, and then all those relative links would be broken. The `<base>` tag is also needed when a deep url is loaded from the server, e.g. `http://server/some/page/url`. In that case, Angular needs to decide which part of the url is the base of the application, and which part is path inside of the application. To summarize: Now all relative links are always relative to the `<base>` tag. Exception (also a breaking change): Link tags whose `href` attribute starts with a `#` will only change the hash of the url, but nothing else (e.g. `<a href="#someAnchor">`). This is to make it easy to scroll to anchors inside a document. Related to angular#6162 Closes angular#8492 BREAKING CHANGE (since 1.2.17 and 1.3.0-beta.10): In html5 mode without a `<base>` tag on older browser that don't support the history API relative paths were adding up. E.g. clicking on `<a href="page1">` and then on `<a href="page2">` would produce `$location.path()==='/page1/page2'. The code that introduced this behavior was removed and Angular now also requires a `<base>` tag to be present when using html5 mode. Closes angular#8172, angular#8233

… master Backport of 2294880 without enforcing the `<base>` tag and without the new handling for links that only contain hash fragments. Related to angular#6162 Closes angular#8492

tbosch assigned tbosch and unassigned tbosch Feb 7, 2014

IgorMinar added the gh: issue label Feb 10, 2014

caitp added component: $location labels Feb 10, 2014

caitp added this to the Backlog milestone Feb 10, 2014

richardcrichardc mentioned this issue Feb 23, 2014

Potential fix for #6162 #6421

Closed

btford added the browser: IE9 label Mar 27, 2014

caitp pushed a commit to caitp/angular.js that referenced this issue Mar 28, 2014

Potential fix for angular#6162

2a8ff31

caitp pushed a commit to caitp/angular.js that referenced this issue Apr 9, 2014

Potential fix for angular#6162

e4e6176

caitp closed this as completed in 3f04770 Apr 17, 2014

NevilleS mentioned this issue Aug 15, 2014

$location rewrites absolute paths incorrectly in LocationHashbangInHtml5Url mode #8633

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Relative links treated like absolute links in html5Mode with legacy browsers in 1.2.12 #6162

Relative links treated like absolute links in html5Mode with legacy browsers in 1.2.12 #6162

richardcrichardc commented Feb 6, 2014

caitp commented Feb 6, 2014

richardcrichardc commented Feb 6, 2014

caitp commented Feb 9, 2014

richardcrichardc commented Feb 9, 2014

caitp commented Feb 9, 2014

richardcrichardc commented Feb 9, 2014

caitp commented Feb 9, 2014

richardcrichardc commented Feb 9, 2014

caitp commented Feb 10, 2014

richardcrichardc commented Feb 10, 2014

caitp commented Feb 10, 2014

richardcrichardc commented Feb 10, 2014

richardcrichardc commented Feb 23, 2014

caitp commented Feb 23, 2014

Relative links treated like absolute links in html5Mode with legacy browsers in 1.2.12 #6162

Relative links treated like absolute links in html5Mode with legacy browsers in 1.2.12 #6162

Comments

richardcrichardc commented Feb 6, 2014

caitp commented Feb 6, 2014

richardcrichardc commented Feb 6, 2014

caitp commented Feb 9, 2014

richardcrichardc commented Feb 9, 2014

caitp commented Feb 9, 2014

richardcrichardc commented Feb 9, 2014

caitp commented Feb 9, 2014

richardcrichardc commented Feb 9, 2014

caitp commented Feb 10, 2014

richardcrichardc commented Feb 10, 2014

caitp commented Feb 10, 2014

richardcrichardc commented Feb 10, 2014

richardcrichardc commented Feb 23, 2014

caitp commented Feb 23, 2014