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

html5mode on IE9 leads to redirect loop behavior on every digest during initial page load #9235

Closed
chrisirhc opened this issue Sep 23, 2014 · 12 comments

Comments

@chrisirhc
Copy link
Contributor

Edit: Fixed URL in step 1.

This can lead to unresponsive pages as the browser attempts to make N number of requests during page load where N is the number of digests that occur after setting up $location.

Note that this is also occurring on angularjs.org. It's just less severe because there are few digests that occur after page load. As such, it maybe related to #6820 .

Reproducible: always
Browsers: IE9 (Most probably affects IE7 and IE8 too, but I haven't tested it)
Affects: Regression from AngularJS 1.2.24 and 1.3.0-RC.0

Steps to reproduce:

  1. Open IE9. Set it up to preserve logs across navigation (Tools -> Clear Entries on Navigate -> Uncheck Console and Network)
    Go to the following page:
    http://run.plnkr.co/plunks/EHfYEeONdOFa6Xu4owUx/test/index.html
  2. Observe that the logs show Navigating to <plunker>/#/test/index.html the same page multiple times.

As seen here, IE is actually making many requests that are aborted (possibly DDoSing some websites, depending on the number of digest cycles):
screen shot 2014-09-23 at 4 30 44 pm

Expected behavior:

  • Navigate to hashbang URL once and wait for it to load.

Source Plunker:
http://plnkr.co/edit/EHfYEeONdOFa6Xu4owUx?p=preview

This is caused by 2ece4d0 (#6976), since fireUrlChange sets newLocation to null (on this line) and it doesn't account for the fact that setting href is async on IE9.

As setting location.href isn't synchronous on IE9, each digest cycle is actually clearing the locationUrl state (that's used to save the navigating state for IE9) so the digest initiates another change to location.href on every digest and the browser actually sends another request instead of continuing to navigate to the same href set previously (in the previous digest cycle).

chrisirhc added a commit to chrisirhc/angular.js that referenced this issue Sep 24, 2014
@chrisirhc chrisirhc changed the title html5mode on IE9 redirects with new request on every digest during initial page load html5mode on IE9 leads to redirect loop behavior on every digest during initial page load Sep 24, 2014
chrisirhc added a commit to chrisirhc/angular.js that referenced this issue Sep 24, 2014
@jeffbcross jeffbcross self-assigned this Sep 26, 2014
@jeffbcross jeffbcross added this to the 1.3.0 milestone Sep 26, 2014
@jeffbcross jeffbcross removed their assignment Sep 26, 2014
@jeffbcross
Copy link
Contributor

Thanks for opening. I could reproduce when following your steps (although your link in step 1 is wrong. I used http://run.plnkr.co/GnOn2iv3QBy6pfTq/test/index.html)

Here is the log I ended up with:

LOG: Begin page load 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: Begin page load 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: Begin page load 
LOG: digest running 
LOG: Navgiating to http://run.plnkr.co/GnOn2iv3QBy6pfTq/#/test/index.html 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 
LOG: digest running 

@chrisirhc
Copy link
Contributor Author

Ah, I forgot to get the permanent plunker URL. I've fixed the url and it should work permanently now.

@jeffbcross jeffbcross modified the milestones: 1.3.0-rc.5, 1.3.0 Sep 29, 2014
@acostaf
Copy link

acostaf commented Oct 2, 2014

@btford @jeffbcross @chrisirhc that is it also related with #7916

@acostaf
Copy link

acostaf commented Oct 2, 2014

@btford @jeffbcross @chrisirhc I found that this happen always when you try to deep-linking with html5Mode(true), I've being having this issue since 1.2.18 till today with 1.3.0-rc.4, please fix it urgently

@btford
Copy link
Contributor

btford commented Oct 3, 2014

I can reproduce the issue, but not consistently.

@acostaf
Copy link

acostaf commented Oct 3, 2014

Using the deep-linking you will see it always, the issue is on fireUrlChange, basically it happens when you are in IE9 and the hash is added to the url but the new URL from deep linking does not contains the hash ( on if (lastBrowserUrl == self.url()) return;)

angular.js File from line 4678

angular.js File from line 4678

function fireUrlChange() {
  newLocation = null;
  if (lastBrowserUrl == self.url()) return;

  lastBrowserUrl = self.url();
  forEach(urlChangeListeners, function(listener) {
    listener(self.url());
  });
}  

@mgol
Copy link
Member

mgol commented Oct 4, 2014

I've just pushed a new version of my pushState PR that changes some things around this code so please have a look at #9027.

@tbosch
Copy link
Contributor

tbosch commented Oct 6, 2014

It turns out that this is a more general problem, see this plunker that shows the same problem in Chrome (when disabling history support): http://run.plnkr.co/plunks/jxWZIBNj9b0M9UgGqTzU/test/index.html

@tbosch tbosch reopened this Oct 6, 2014
tbosch added a commit to tbosch/angular.js that referenced this issue Oct 7, 2014
Adds caching for url changes after a reload happens

Removes unnecessary caching from $browser, as IE-IE9 all allow to change `location.href` synchronously, i.e. after changing it (via property write or `location.replace) it can be read out immediately with the up to date value.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for angular#6976
Fixes angular#9235
tbosch added a commit to tbosch/angular.js that referenced this issue Oct 7, 2014
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.

Removes unnecessary caching from $browser, as IE7-IE9 all 
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for angular#6976
Fixes angular#9235
tbosch added a commit to tbosch/angular.js that referenced this issue Oct 7, 2014
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.

Removes unnecessary caching from $browser, as IE7-IE9 all
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for angular#6976
Fixes angular#9235
tbosch added a commit to tbosch/angular.js that referenced this issue Oct 7, 2014
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.

Removes unnecessary caching from $browser, as IE7-IE9 all 
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for angular#6976
Fixes angular#9235
tbosch added a commit to tbosch/angular.js that referenced this issue Oct 7, 2014
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.

Removes unnecessary caching from $browser, as IE7-IE9 all
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for angular#6976
Fixes angular#9235
@tbosch
Copy link
Contributor

tbosch commented Oct 7, 2014

To reproduce in Chrome:
-> use network throtteling 3G (In device mode)
-> disable cache (Network Tab)
-> preserve logs (Console Tab / Dev Tools Settings)

@tbosch tbosch closed this as completed in 8ee1ba4 Oct 7, 2014
tbosch added a commit that referenced this issue Oct 7, 2014
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.

Removes unnecessary caching from $browser, as IE7-IE9 all
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for #6976
Fixes #9235
Closes #9470
bullgare pushed a commit to bullgare/angular.js that referenced this issue Oct 9, 2014
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.

Removes unnecessary caching from $browser, as IE7-IE9 all
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca2317 and d707114.

Adds more tests for angular#6976
Fixes angular#9235
Closes angular#9455
@bgourlie
Copy link

I'm still seeing this issue as of 1.3.2 in a particular scenario where the site is hosted on a sub folder and I visit the site without the trailing slash (IE9 only). For example, the site is hosted at a location like http://intranet/siteRoot/. If I visit that exact URL, the URL will update to http://intranet/siteRoot/#/ and everything works as expected. If I visit that location without the trailing slash, http://intranet/siteRoot, I get the redirect/infinite digest loop:

ie9issue

If the site is running locally, at a location such as http://localhost:56666/, removing the trailing slash doesn't cause the issue.

Should I open a new bug for this issue? Or should this one be re-opened.

@xie-qianyue
Copy link

@tbosch Unfortunately, I still have this problem. I verified your commit. In fact, when the page is loaded, the code comes to the method fireUrlChange :
if (lastBrowserUrl === self.url() && lastHistoryState === cachedState) { return; },
It returns as you wish. But it still has the problem of infinite $digest.
Here is a show case of my scenario : https://github.com/xie-qianyue/showcase-angular1.3-route-bug-ie9.

@hamfastgamgee
Copy link

I have a potential fix for the problem @bgourlie and @xie-qianyue are experiencing in PR #11675. (And I filed it separately as issue #11439)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.