fix($browser): Cache location.href only during page reload phase

src/ng/browser.js

@@ -125,7 +125,7 @@ function Browser(window, document, $log, $sniffer) {
 
   var lastBrowserUrl = location.href,
       baseElement = document.find('base'),
-      newLocation = null;
+      reloadLocation = null;
 
   /**
    * @name $browser#url
@@ -166,7 +166,9 @@ function Browser(window, document, $log, $sniffer) {
           history.pushState(null, '', url);
         }
       } else {
-        newLocation = url;
+        if (!sameBase) {
+          reloadLocation = url;
+        }
         if (replace) {
           location.replace(url);
         } else {
@@ -176,22 +178,17 @@ function Browser(window, document, $log, $sniffer) {
       return self;
     // getter
     } else {
-      // - newLocation is a workaround for an IE7-9 issue with location.replace and location.href
-      //   methods not updating location.href synchronously.
+      // - reloadLocation is needed as browsers don't allow to read out
+      //   the new location.href if a reload happened.
       // - the replacement is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=407172
-      return newLocation || location.href.replace(/%27/g,"'");
+      return reloadLocation || location.href.replace(/%27/g,"'");
     }
   };
 
   var urlChangeListeners = [],
       urlChangeInit = false;
 
   function fireUrlChange() {
-    newLocation = null;
-    checkUrlChange();
-  }
-
-  function checkUrlChange() {
     if (lastBrowserUrl == self.url()) return;
 
     lastBrowserUrl = self.url();
@@ -247,7 +244,7 @@ function Browser(window, document, $log, $sniffer) {
    * Needs to be exported to be able to check for changes that have been done in sync,
    * as hashchange/popstate events fire in async.
    */
-  self.$$checkUrlChange = checkUrlChange;
+  self.$$checkUrlChange = fireUrlChange;
 
   //////////////////////////////////////////////////////////////
   // Misc API

test/ng/browserSpecs.js

@@ -495,6 +495,55 @@ describe('browser', function() {
       browser.url(current);
       expect(fakeWindow.location.href).toBe('dontchange');
     });
+
+    it('should not read out location.href if a reload was triggered but still allow to change the url', function() {
+      sniffer.history = false;
+      browser.url('http://server/someOtherUrlThatCausesReload');
+      expect(fakeWindow.location.href).toBe('http://server/someOtherUrlThatCausesReload');
+
+      fakeWindow.location.href = 'http://someNewUrl';
+      expect(browser.url()).toBe('http://server/someOtherUrlThatCausesReload');
+
+      browser.url('http://server/someOtherUrl');
+      expect(browser.url()).toBe('http://server/someOtherUrl');
+      expect(fakeWindow.location.href).toBe('http://server/someOtherUrl');
+    });
+
+    it('assumes that changes to location.hash occur in sync', function() {
+      // This is an asynchronous integration test that changes the
+      // hash in all possible ways and checks
+      // - whether the change to the hash can be read out in sync
+      // - whether the change to the hash can be read out in the hashchange event
+      var realWin = window,
+          $realWin = jqLite(realWin),
+          hashInHashChangeEvent = [];
+
+      runs(function() {
+        $realWin.on('hashchange', hashListener);
+
+        realWin.location.hash = '1';
+        realWin.location.href += '2';
+        realWin.location.replace(realWin.location.href + '3');
+        realWin.location.assign(realWin.location.href + '4');
+
+        expect(realWin.location.hash).toBe('#1234');
+      });
+      waitsFor(function() {
+        return hashInHashChangeEvent.length > 3;
+      });
+      runs(function() {
+        $realWin.off('hashchange', hashListener);
+
+        forEach(hashInHashChangeEvent, function(hash) {
+          expect(hash).toBe('#1234');
+        });
+      });
+
+      function hashListener() {
+        hashInHashChangeEvent.push(realWin.location.hash);
+      }
+    });
+
   });
 
   describe('urlChange', function() {
@@ -573,15 +622,15 @@ describe('browser', function() {
       beforeEach(function() {
         sniffer.history = false;
         sniffer.hashchange = false;
-        browser.url("http://server.current");
+        browser.url("http://server/#current");
       });
 
       it('should fire callback with the correct URL on location change outside of angular', function() {
         browser.onUrlChange(callback);
 
-        fakeWindow.location.href = 'http://server.new';
+        fakeWindow.location.href = 'http://server/#new';
         fakeWindow.setTimeout.flush();
-        expect(callback).toHaveBeenCalledWith('http://server.new');
+        expect(callback).toHaveBeenCalledWith('http://server/#new');
 
         fakeWindow.fire('popstate');
         fakeWindow.fire('hashchange');
@@ -645,33 +694,134 @@ describe('browser', function() {
 
   describe('integration tests with $location', function() {
 
-    beforeEach(module(function($provide, $locationProvider) {
-      spyOn(fakeWindow.history, 'pushState').andCallFake(function(stateObj, title, newUrl) {
-        fakeWindow.location.href = newUrl;
+    function setup(options) {
+      module(function($provide, $locationProvider) {
+        spyOn(fakeWindow.history, 'pushState').andCallFake(function(stateObj, title, newUrl) {
+          fakeWindow.location.href = newUrl;
+        });
+        spyOn(fakeWindow.location, 'replace').andCallFake(function(newUrl) {
+          fakeWindow.location.href = newUrl;
+        });
+        $provide.value('$browser', browser);
+        browser.pollFns = [];
+
+        sniffer.history = options.history;
+        $provide.value('$sniffer', sniffer);
+
+        $locationProvider.html5Mode(options.html5Mode);
       });
-      $provide.value('$browser', browser);
-      browser.pollFns = [];
+    }
 
-      sniffer.history = true;
-      $provide.value('$sniffer', sniffer);
+    describe('update $location when it was changed outside of Angular in sync '+
+       'before $digest was called', function() {
 
-      $locationProvider.html5Mode(true);
-    }));
+      it('should work with no history support, no html5Mode', function() {
+        setup({
+          history: false,
+          html5Mode: false
+        });
+        inject(function($rootScope, $location) {
+          $rootScope.$apply(function() {
+            $location.path('/initialPath');
+          });
+          expect(fakeWindow.location.href).toBe('http://server/#/initialPath');
 
-    it('should update $location when it was changed outside of Angular in sync '+
-       'before $digest was called', function() {
-      inject(function($rootScope, $location) {
-        fakeWindow.history.pushState(null, '', 'http://server/someTestHash');
+          fakeWindow.location.href = 'http://server/#/someTestHash';
 
-        // Verify that infinite digest reported in #6976 no longer occurs
-        expect(function() {
           $rootScope.$digest();
-        }).not.toThrow();
 
-        expect($location.path()).toBe('/someTestHash');
+          expect($location.path()).toBe('/someTestHash');
+        });
       });
+
+      it('should work with history support, no html5Mode', function() {
+        setup({
+          history: true,
+          html5Mode: false
+        });
+        inject(function($rootScope, $location) {
+          $rootScope.$apply(function() {
+            $location.path('/initialPath');
+          });
+          expect(fakeWindow.location.href).toBe('http://server/#/initialPath');
+
+          fakeWindow.location.href = 'http://server/#/someTestHash';
+
+          $rootScope.$digest();
+
+          expect($location.path()).toBe('/someTestHash');
+        });
+      });
+
+      it('should work with no history support, with html5Mode', function() {
+        setup({
+          history: false,
+          html5Mode: true
+        });
+        inject(function($rootScope, $location) {
+          $rootScope.$apply(function() {
+            $location.path('/initialPath');
+          });
+          expect(fakeWindow.location.href).toBe('http://server/#/initialPath');
+
+          fakeWindow.location.href = 'http://server/#/someTestHash';
+
+          $rootScope.$digest();
+
+          expect($location.path()).toBe('/someTestHash');
+        });
+      });
+
+      it('should work with history support, with html5Mode', function() {
+        setup({
+          history: true,
+          html5Mode: true
+        });
+        inject(function($rootScope, $location) {
+          $rootScope.$apply(function() {
+            $location.path('/initialPath');
+          });
+          expect(fakeWindow.location.href).toBe('http://server/initialPath');
+
+          fakeWindow.location.href = 'http://server/someTestHash';
+
+          $rootScope.$digest();
+
+          expect($location.path()).toBe('/someTestHash');
+        });
+      });
+
     });
 
+    it('should not reload the page on every $digest when the page will be reloaded due to url rewrite on load', function() {
+      setup({
+        history: false,
+        html5Mode: true
+      });
+      fakeWindow.location.href = 'http://server/some/deep/path';
+      var changeUrlCount = 0;
+      var _url = browser.url;
+      browser.url = function(newUrl, replace) {
+        if (newUrl) {
+          changeUrlCount++;
+        }
+        return _url.call(this, newUrl, replace);
+      };
+      spyOn(browser, 'url').andCallThrough();
+      inject(function($rootScope, $location) {
+        $rootScope.$digest();
+        $rootScope.$digest();
+        $rootScope.$digest();
+        $rootScope.$digest();
+
+        // from $location for rewriting the initial url into a hash url
+        expect(browser.url).toHaveBeenCalledWith('http://server/#/some/deep/path', true);
+        // from the initial call to the watch in $location for watching $location
+        expect(browser.url).toHaveBeenCalledWith('http://server/#/some/deep/path', false);
+        expect(changeUrlCount).toBe(2);
+      });
+
+    });
   });
 
   describe('integration test with $rootScope', function() {