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

Routing-Navigation on IE9 different to Chrome #7956

Closed
acostaf opened this issue Jun 23, 2014 · 20 comments
Closed

Routing-Navigation on IE9 different to Chrome #7956

acostaf opened this issue Jun 23, 2014 · 20 comments

Comments

@acostaf
Copy link

acostaf commented Jun 23, 2014

HI all

I am trying to migrate to version 1.2.18 from 1.2.16, implementing routing with .

So clicking /ContainerManagement/ContainersByUnit/1/749 will navgate to /ContainerManagement/#/ContainersByUnit/ContainersByUnit/1/749 on IE9 adding ContainersByUnit twice but the expected on chrome, if I change the link from /ContainerManagement/ContainersByUnit/1/749 to /ContainerManagement//1/749 the result is the expected on IE9 but failed in chrome navigating to /ContainerManagement//1/749

Help traped on version 1.2.16

@acostaf
Copy link
Author

acostaf commented Jun 23, 2014

I also have tested this on version 1.3.0-beta.13 with not luck, also broken in there

@tbosch tbosch self-assigned this Jun 23, 2014
@tbosch
Copy link
Contributor

tbosch commented Jun 23, 2014

Hi,
could you create a plunker to help us reproduce this?

@tbosch tbosch added this to the Purgatory milestone Jun 23, 2014
@tbosch tbosch removed their assignment Jun 23, 2014
@acostaf
Copy link
Author

acostaf commented Jun 24, 2014

Hi

I try to use the same angular doc for route on my IE9 under compatibility view and the first thing that I got is the indexof error on line 91 when I click on "Edit in Plunker", that just happens on IE9 when href is empty on an anchor

Error

HTML1113: Document mode restart from IE7 Standards to IE9 Standards
docs.angularjs.org
SCRIPT5007: Unable to get value of the property 'indexOf': object is null or undefined
angular.min.js, line 91 character 490

@acostaf
Copy link
Author

acostaf commented Jun 24, 2014

Ok, routing it is completly broken on IE9 using compatibility view or not, please try to go to https://docs.angularjs.org/examples/example-$route-service/index-production.html with IE9CV then you may get this error

SCRIPT5014: Array.prototype.slice: 'this' is not a JavaScript object
angular.min.js, line 26 character 204
SCRIPT5014: Array.prototype.slice: 'this' is not a JavaScript object
angular.min.js, line 26 character 204

Also try to go the same page https://docs.angularjs.org/examples/example-$route-service/index-production.html using IE9 not using compatibility view then you may see how the routing failed just clicking on the link

Url
https://docs.angularjs.org/examples/example-$route-service/index-production.html#/index-production.html/Book/Moby/Book/Moby/ch/1/Book/Gatsby/Book/Gatsby/ch/4/Book/Scarlet

path
$location.path() = /index-production.html/Book/Moby/Book/Moby/ch/1/Book/Gatsby/Book/Gatsby/ch/4/Book/Scarlet

BTW this worked as expected on Chrome

@acostaf
Copy link
Author

acostaf commented Jun 24, 2014

I have done the same test using 1.2.16 with the corresponding url and works perfectly as expected on IE9 https://code.angularjs.org/1.2.16/docs/examples/example-$route-service/

Same page for 1.2.17, 1.2.18 and 1.3.0 beta 13 are all broken

@acostaf
Copy link
Author

acostaf commented Jun 25, 2014

Hi
I've checking changes on routing and location and found some changes done on 1.2.17 that looks suspicious

02058bf
e020366

@acostaf
Copy link
Author

acostaf commented Jun 25, 2014

Hi @tbosch, it is this enough for replication?

@caitp
Copy link
Contributor

caitp commented Jun 25, 2014

at the time those commits were checked in, this particular example was verified to be working on old IE, but lets see

@caitp
Copy link
Contributor

caitp commented Jun 25, 2014

it is true though, the 1.2.17 example is broken in IE9, and the 1.2.16 example is working. hm.

@caitp
Copy link
Contributor

caitp commented Jun 25, 2014

my feeling is that the reason this isn't working here is because of the way relative urls are being resolved, but we had gone to some length to make sure that relative urls were being resolved basically correctly. It's hard for me to debug this one since I don't have a copy of IE9 handy, but somewhere this must have fallen apart.

If you have IE9 on your local machine, it would be really helpful if you would do a bisect to find the specific sha which broke this, so that we can think about reverting or fixing it

@acostaf
Copy link
Author

acostaf commented Jul 11, 2014

@caitp Back on the issue, I have quite busy for the last couple of week but I can now worked on this version again, the first thing that I did was to move to the latest version 1.2.19 and the issue still exist

@acostaf
Copy link
Author

acostaf commented Jul 11, 2014

@caitp the first issue found in the code was on Line 9714 on angular.js

Line 9714 var href = elm.attr('href') || elm.attr('xlink:href');

this will throw and error when href of an achor is empty

e.g.

This should be fixed since it is ok on chrome 35

@acostaf
Copy link
Author

acostaf commented Jul 11, 2014

@caitp Now the location issue when having a link like that
< a href="ContainersByUnit/{{container.NamespaceId}}/{{container.Id}}" class="btn-link text-blue" >
the link is produced as
http://localhost:56274/ContainerManagement/ContainersByUnit/1335/634
angular.js line 9714 will resolve that as "ContainersByUnit/1/749" then the process will enter into this bellow of code, since the first character of href var is not '/' or '#' will go into the else part line 9727 resulting as
"http://localhost:56274/ContainerManagement/#/ContainersByUnit/ContainersByUnit/1/749" duplicating the 'ContainersByUnit' part, that is happening becuase the code is adding $location.path() on line 9726

9711 if (LocationMode === LocationHashbangInHtml5Url) {
9712 // get the actual href attribute - see
9713 // http://msdn.microsoft.com/en-us/library/ie/dd347148(v=vs.85).aspx
9714 var href = elm.attr('href') || elm.attr('xlink:href');
9715
9716 if (href.indexOf('://') < 0) { // Ignore absolute URLs
9717 var prefix = '#' + hashPrefix;
9718 if (href[0] == '/') {
9719 // absolute path - replace old path
9720 absHref = appBase + prefix + href;
9721 } else if (href[0] == '#') {
9722 // local anchor
9723 absHref = appBase + prefix + ($location.path() || '/') + href;
9724 } else {
9725 // relative path - join with current path
9726 var stack = $location.path().split("/"), parts = href.split("/");
9727 for (var i=0; i<parts.length; i++) {
9728 if (parts[i] == ".")
9729 continue;
9730 else if (parts[i] == "..")
9731 stack.pop();
9732 else if (parts[i].length)
9733 stack.push(parts[i]);
9734 }
9735 absHref = appBase + prefix + stack.join('/');
9736 }
9737 }
9738 }

@acostaf
Copy link
Author

acostaf commented Jul 11, 2014

@caitp Second scenario using '/' at the beginning
e.g < a href="/ContainersByUnit/{{container.NamespaceId}}/{{container.Id}}" class="btn-link text-blue" > on hover you may see this 'http://localhost:56274/ContainersByUnit/1/749' missing the base 'ContainerManagement' so if you copy this link and paste it on another browser will never work but hoever the line 9720 will generate "http://localhost:56274/ContainerManagement/#/ContainersByUnit/1/749" which is the right link

9711 if (LocationMode === LocationHashbangInHtml5Url) {
9712 // get the actual href attribute - see
9713 // http://msdn.microsoft.com/en-us/library/ie/dd347148(v=vs.85).aspx
9714 var href = elm.attr('href') || elm.attr('xlink:href');
9715
9716 if (href.indexOf('://') < 0) { // Ignore absolute URLs
9717 var prefix = '#' + hashPrefix;
9718 if (href[0] == '/') {
9719 // absolute path - replace old path
9720 absHref = appBase + prefix + href;
9721 } else if (href[0] == '#') {
9722 // local anchor
9723 absHref = appBase + prefix + ($location.path() || '/') + href;
9724 } else {
9725 // relative path - join with current path
9726 var stack = $location.path().split("/"), parts = href.split("/");
9727 for (var i=0; i<parts.length; i++) {
9728 if (parts[i] == ".")
9729 continue;
9730 else if (parts[i] == "..")
9731 stack.pop();
9732 else if (parts[i].length)
9733 stack.push(parts[i]);
9734 }
9735 absHref = appBase + prefix + stack.join('/');
9736 }
9737 }
9738 }

@acostaf
Copy link
Author

acostaf commented Jul 11, 2014

@caitp Third and last scenario using '#' at the beginning
e.g. < a href="#ContainersByUnit/{{container.NamespaceId}}/{{container.Id}}" class="btn-link text-blue" > on hover or copying you may see http://localhost:56274/ContainerManagement/#ContainersByUnit/1/749 which is the right link hoever click on the link will run the code from line 9723 which will produce

"http://localhost:56274/ContainerManagement/#/ContainersByUnit#ContainersByUnit/1/749" which will never work att all

9711 if (LocationMode === LocationHashbangInHtml5Url) {
9712 // get the actual href attribute - see
9713 // http://msdn.microsoft.com/en-us/library/ie/dd347148(v=vs.85).aspx
9714 var href = elm.attr('href') || elm.attr('xlink:href');
9715
9716 if (href.indexOf('://') < 0) { // Ignore absolute URLs
9717 var prefix = '#' + hashPrefix;
9718 if (href[0] == '/') {
9719 // absolute path - replace old path
9720 absHref = appBase + prefix + href;
9721 } else if (href[0] == '#') {
9722 // local anchor
9723 absHref = appBase + prefix + ($location.path() || '/') + href;
9724 } else {
9725 // relative path - join with current path
9726 var stack = $location.path().split("/"), parts = href.split("/");
9727 for (var i=0; i<parts.length; i++) {
9728 if (parts[i] == ".")
9729 continue;
9730 else if (parts[i] == "..")
9731 stack.pop();
9732 else if (parts[i].length)
9733 stack.push(parts[i]);
9734 }
9735 absHref = appBase + prefix + stack.join('/');
9736 }
9737 }
9738 }

@acostaf
Copy link
Author

acostaf commented Jul 11, 2014

So @caitp I think we should roll-back the below checkin to prevent all those scenarios e020366

@acostaf
Copy link
Author

acostaf commented Jul 12, 2014

@caitp Any idea when that may be fixed

@acostaf
Copy link
Author

acostaf commented Jul 16, 2014

@caitp 1.2.20 have been release, is this issue fixed ?

@acostaf
Copy link
Author

acostaf commented Jul 23, 2014

@caitp @btford @winsontam @dolymood @rodyhaddad @effbcross I believed that this issue it is also related with

#7916 , #7892 and #8172

@btford btford removed the gh: issue label Aug 20, 2014
@Narretz
Copy link
Contributor

Narretz commented Jan 22, 2016

This is fixed.

@Narretz Narretz closed this as completed Jan 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants