IE9 infinite $digest bug if loading a base URL without a trailing slash

[hamfastgamgee]

I'm not honestly sure that this is reproducible in a plunker due to the specific circumstances, and unfortunately our app is secured and internal-only, so I'm not sure I can actually make a ready-to-use example.

IE9 reports infinite $digest loops in HTML5 mode when going to the base URL of an app with a default route in place if you don't use a trailing slash. This does not happen in browsers that actually support the History API and don't need the hashbangs.

Say my base URL is http://www.mysite.com/appBase and I have an angular app hosted there with a default route set to go to /Home. If I go to "http://www.mysite.com/appBase/" (note the trailing slash), I get redirected to http://www.mysite.com/appBase/Home and everything is perfectly all right. No errors in the console, etc.

If I instead go to "http://www.mysite.com/appBase" (note no trailing slash), I end up in the infinite $digest loop. Prior to angular 1.3.6, this case ended up pegging IE so that you had to force-kill (fun!). After 1.3.6 (I've tested up through 1.3.15), we still get an infinite $digest loop but it does finally bail out and succeed, so the problem is more on the order of performance and error log flooding rather than complete disaster like it was prior to 1.3.6. (In fact, I thought this was fixed for some time, but I must only have been testing locally and not on our deployed server with the separate base URL.)

It appears to have something to do with the $$rootScope.$watch(function $locationWatch()) call. I think what's happening (and mind you, debugging anything in IE9 on a deployed server is akin to self-inflicting a root canal) is that something is losing track of the hash symbol. I end up seeing calls to the $browser.url() setter that go back and forth between http://www.mysite.com/appBase/#/ and http://www.mysite.com/appBase/#/Home/. Eventually something changes (I'm wondering if it's simply the $watch bombing due to infinite $digests) to get it out of the cycle, and things in the app are fine.

In $browser.url()'s setter, everything ends up okay if I change the following lines:

if (!sameBase) {
    reloadLocation = url;
}

to:

if (replace || !sameBase) {
    reloadLocation = url;
}

"replace" is getting set to true twice in a row, the first time to http://www.mysite.com/appBase/#/ and the second time to http://www.mysite.com/appBase/#/Home/. Something is checking $browser.url()'s getter, and with that change, that place is seeing the changed reloadLocation variable after the second call with replace=true and ends up cutting out of the $digest loop.

I don't know if there are any other implications to that change, though, or if this is the right way to go about this. Without the ability to step through in a real debugger (since IE9's debugger seems to be useless for SPAs), I'm limited to what I've been able to log to the console or pop up in an alert (since apparently console.log doesn't always fire in IE9?!?). All I know is that Chrome and Firefox are still fine after that change as well, and I can't think of a compelling reason that we should hang on to the first "reloadLocation" if we're doing a "replace" twice.

I hope this makes at least some sense to someone. Again, if I had the means to make a reproducible testcase, I would, but it seems to be something that's hard to emulate without free reign to muck up a public server.

[hamfastgamgee]

Looks like the "something" that's checking $browser.url() is the $locationWatch function itself?

[GabrielDelepine]

I also experience it with angular 1.2.28. How can I help you @hamfastgamgee to isolate the case ?

[xie-qianyue]

@hamfastgamgee I submit an example of the bug in #9235, I think your problem has some links to this issue.

[hamfastgamgee]

Sweet, thanks!

[hamfastgamgee]

Looking at the sample app, that appears to be exactly the same case. My app starts in the case that's produced by the button in the sample app, but it's fundamentally the same, right down to including the trailing slash in the base URL (which is generated on the server in my app).

[hamfastgamgee]

$locationWatch is definitely how the infinite $digest is happening.

In the sample app, $browser.url() is set to http://localhost:3000/app/#. trimEmptyHash makes that http://localhost:3000/app/.

$location.absUrl() is http://localhost:3000/app/#/, and trimEmptyHash makes that http://localhost:3000/app/#/

Therefore, urlOrStateChanged gets set to true, and we call $rootScope.$evalAsync. But the code inside that doesn't actually result in us getting $browser.url() and $location.absUrl() in synch, so we end up hitting the same case recursively. Hence the infinite $digest call.

[hamfastgamgee]

Okay, I think I understand what's going on here finally.

When we get into $locationWatch the first time, $location.absUrl() and $browser.url() are in synch, both pointing to http://localhost:3000/app/#. The "initializing" variable is set to true. We get into the block that ends up calling $rootScope.$evalAsync, and get down to the afterLocationChange() call. The afterLocationChange() call is caught by ngRoute, which discovers there's a default redirect in place. This updates $location.absUrl() to have the trailing slash, but $browser.url() does not have the trailing slash.

$locationWatch fires again on the next digest. oldUrl and newUrl don't match, because trimEmptyHash() returns the hash and the trailing slash for $location.absUrl() but strips the hash (which has no trailing slash) from $browser.url(). We then repeat the vicious cycle, because $browser.url() always short-circuits without getting in proper synch with $location.absUrl().

So either we need to make sure that $browser.url() stays fully in synch with $location.absUrl(), or we need to change trimEmptyHash to return the same results if the "empty hash" has a trailing slash as it does if the "empty hash" stands alone.

[hamfastgamgee]

Well, I have a potential fix for this. Unfortunately, I'm having a hell of a time getting a local dev environment set up in our corporate network. Here's the code in location.js that I'm trying to get a pull request set up for:

this.$get = ['$rootScope', '$browser', '$sniffer', '$rootElement', '$window',
      function($rootScope, $browser, $sniffer, $rootElement, $window) {
    var $location,
        LocationMode,
        baseHref = $browser.baseHref(), // if base[href] is undefined, it defaults to ''
        initialUrl = $browser.url(),
        appBase;

    if (html5Mode.enabled) {
      if (!baseHref && html5Mode.requireBase) {
        throw $locationMinErr('nobase',
          "$location in HTML5 mode requires a <base> tag to be present!");
      }
      appBase = serverBase(initialUrl) + (baseHref || '/');
      LocationMode = $sniffer.history ? LocationHtml5Url : LocationHashbangInHtml5Url;
    } else {
      appBase = stripHash(initialUrl);
      LocationMode = LocationHashbangUrl;
    }
    $location = new LocationMode(appBase, '#' + hashPrefix);
    $location.$$parseLinkUrl(initialUrl, initialUrl);

    $location.$$state = $browser.state();

    var IGNORE_URI_REGEXP = /^\s*(javascript|mailto):/i;

    function setBrowserUrlWithFallback(url, replace, state) {
      var oldUrl = $location.url();
      var oldState = $location.$$state;
      try {
        $browser.url(url, replace, state);

        // Make sure $location.state() returns referentially identical (not just deeply equal)
        // state object; this makes possible quick checking if the state changed in the digest
        // loop. Checking deep equality would be too expensive.
        $location.$$state = $browser.state();
      } catch (e) {
        // Restore old values if pushState fails
        $location.url(oldUrl);
        $location.$$state = oldState;

        throw e;
      }
    }

    $rootElement.on('click', function(event) {
      // TODO(vojta): rewrite link when opening in new tab/window (in legacy browser)
      // currently we open nice url link and redirect then

      if (!html5Mode.rewriteLinks || event.ctrlKey || event.metaKey || event.shiftKey || event.which == 2 || event.button == 2) return;

      var elm = jqLite(event.target);

      // traverse the DOM up to find first A tag
      while (nodeName_(elm[0]) !== 'a') {
        // ignore rewriting if no A tag (reached root element, or no parent - removed from document)
        if (elm[0] === $rootElement[0] || !(elm = elm.parent())[0]) return;
      }

      var absHref = elm.prop('href');
      // get the actual href attribute - see
      // http://msdn.microsoft.com/en-us/library/ie/dd347148(v=vs.85).aspx
      var relHref = elm.attr('href') || elm.attr('xlink:href');

      if (isObject(absHref) && absHref.toString() === '[object SVGAnimatedString]') {
        // SVGAnimatedString.animVal should be identical to SVGAnimatedString.baseVal, unless during
        // an animation.
        absHref = urlResolve(absHref.animVal).href;
      }

      // Ignore when url is started with javascript: or mailto:
      if (IGNORE_URI_REGEXP.test(absHref)) return;

      if (absHref && !elm.attr('target') && !event.isDefaultPrevented()) {
        if ($location.$$parseLinkUrl(absHref, relHref)) {
          // We do a preventDefault for all urls that are part of the angular application,
          // in html5mode and also without, so that we are able to abort navigation without
          // getting double entries in the location history.
          event.preventDefault();
          // update location manually
          if ($location.absUrl() != $browser.url()) {
            $rootScope.$apply();
            // hack to work around FF6 bug 684208 when scenario runner clicks on links
            $window.angular['ff-684208-preventDefault'] = true;
          }
        }
      }
    });


    // rewrite hashbang url <> html5 url
    if (trimEmptyHash($location.absUrl()) != trimEmptyHash(initialUrl)) {
      $browser.url($location.absUrl(), true);
    }

    var initializing = true, skipLocationWatch = false;

    // update $location when $browser url changes
    $browser.onUrlChange(function(newUrl, newState) {
      $rootScope.$evalAsync(function() {
        var oldUrl = $location.absUrl();
        var oldState = $location.$$state;
        var defaultPrevented;

        $location.$$parse(newUrl);
        $location.$$state = newState;

        defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
            newState, oldState).defaultPrevented;

        // if the location was changed by a `$locationChangeStart` handler then stop
        // processing this location change
        if ($location.absUrl() !== newUrl) return;

        if (defaultPrevented) {
          $location.$$parse(oldUrl);
          $location.$$state = oldState;
          setBrowserUrlWithFallback(oldUrl, false, oldState);
        } else {
          initializing = false;
          afterLocationChange(oldUrl, oldState);
        }
      });
      if (!$rootScope.$$phase) $rootScope.$digest();
    });

    // update browser
    $rootScope.$watch(function $locationWatch() {
      if (skipLocationWatch && !$location.$$replace) {
        //prevent infinite $digest errors in hashbang mode
        skipLocationWatch = false;
        return;
      }
      var oldUrl = trimEmptyHash($browser.url());
      var newUrl = trimEmptyHash($location.absUrl());
      var oldState = $browser.state();
      var currentReplace = $location.$$replace;
      var urlOrStateChanged = oldUrl !== newUrl ||
        ($location.$$html5 && $sniffer.history && oldState !== $location.$$state);

      if (initializing || urlOrStateChanged) {
        initializing = false;

        $rootScope.$evalAsync(function() {
          var newUrl = $location.absUrl();
          var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
              $location.$$state, oldState).defaultPrevented;

          // if the location was changed by a `$locationChangeStart` handler then stop
          // processing this location change
          if ($location.absUrl() !== newUrl) return;

          if (defaultPrevented) {
            $location.$$parse(oldUrl);
            $location.$$state = oldState;
          } else {
            if (urlOrStateChanged) {
              setBrowserUrlWithFallback(newUrl, currentReplace,
                                        oldState === $location.$$state ? null : $location.$$state);
            }
            var oldLocationAbsUrl = $location.absUrl();
            afterLocationChange(oldUrl, oldState);
            if (oldLocationAbsUrl !== $location.absUrl() &&
                  trimEmptyHash($location.absUrl()) !== trimEmptyHash($browser.url())) {
                skipLocationWatch = true;
            }
          }
        });
      }

      $location.$$replace = false;

      // we don't need to return anything because $evalAsync will make the digest loop dirty when
      // there is a change
    });

    return $location;

    function afterLocationChange(oldUrl, oldState) {
      $rootScope.$broadcast('$locationChangeSuccess', $location.absUrl(), oldUrl,
        $location.$$state, oldState);
    }
}];

[hamfastgamgee]

Basically, notice that there's a skipLocationWatch flag in $locationWatch that gets set and kills the potential infinite $digest issue.

[hamfastgamgee]

Okay, I finally have a patch that succeeds every test (both the automated angular ones and runtime ones in both my app and the test app @xie-qianyue provided). Basically, it works by detecting if $locationWatch tries to update $browser.url() twice in a row with identical parameters, and if that happens, the cached variable for handling $browser.url()'s getter is forced to update even though it isn't updating because sameBase in the $browser.url() setter is true. It only fires for hashbang HTML5 mode.

I have yet to find a way to actually unit test this. The angular-mocks $browserdoes{n}^{\prime }tactuallyexposethebugbecauseitjustresets$$url every time it is called, so we have to construct a different $browser. Which is frustrating, to say the least. :)

[hamfastgamgee]

I was able to throw together a few tests, although their efficacy is open to some question. As I said, the case is very hard to directly hit.

[hamfastgamgee]

The latest iteration takes care of the unit tests in a way that the automated test apparatus is okay with. So now we just need someone from angular to start looking...