Fix/browser 1.2

src/ng/browser.js

@@ -1,4 +1,5 @@
 'use strict';
+/* global stripHash: true */
 
 /**
  * ! This is a private undocumented service !
@@ -124,7 +125,7 @@ function Browser(window, document, $log, $sniffer) {
 
   var lastBrowserUrl = location.href,
       baseElement = document.find('base'),
-      newLocation = null;
+      reloadLocation = null;
 
   /**
    * @name $browser#url
@@ -153,16 +154,23 @@ function Browser(window, document, $log, $sniffer) {
     // setter
     if (url) {
       if (lastBrowserUrl == url) return;
+      var sameBase = lastBrowserUrl && stripHash(lastBrowserUrl) === stripHash(url);
       lastBrowserUrl = url;
-      if ($sniffer.history) {
+      // Don't use history API if only the hash changed
+      // due to a bug in IE10/IE11 which leads
+      // to not firing a `hashchange` nor `popstate` event
+      // in some cases (see #9143).
+      if (!sameBase && $sniffer.history) {
         if (replace) history.replaceState(null, '', url);
         else {
           history.pushState(null, '', url);
           // Crazy Opera Bug: http://my.opera.com/community/forums/topic.dml?id=1185462
           baseElement.attr('href', baseElement.attr('href'));
         }
       } else {
-        newLocation = url;
+        if (!sameBase) {
+          reloadLocation = url;
+        }
         if (replace) {
           location.replace(url);
         } else {
@@ -172,22 +180,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();
@@ -243,7 +246,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

src/ng/location.js

@@ -682,6 +682,9 @@ function $LocationProvider(){
 
       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()) {

test/ng/browserSpecs.js

@@ -440,6 +440,17 @@ describe('browser', function() {
       expect(locationReplace).not.toHaveBeenCalled();
     });
 
+    it('should set location.href and not use pushState when the url only changed in the hash fragment to please IE10/11', function() {
+      sniffer.history = true;
+      browser.url('http://server/#123');
+
+      expect(fakeWindow.location.href).toEqual('http://server/#123');
+
+      expect(pushState).not.toHaveBeenCalled();
+      expect(replaceState).not.toHaveBeenCalled();
+      expect(locationReplace).not.toHaveBeenCalled();
+    });
+
     it('should use location.replace when history.replaceState not available', function() {
       sniffer.history = false;
       browser.url('http://new.org', true);
@@ -451,6 +462,17 @@ describe('browser', function() {
       expect(fakeWindow.location.href).toEqual('http://server/');
     });
 
+    it('should use location.replace and not use replaceState when the url only changed in the hash fragment to please IE10/11', function() {
+      sniffer.history = true;
+      browser.url('http://server/#123', true);
+
+      expect(locationReplace).toHaveBeenCalledWith('http://server/#123');
+
+      expect(pushState).not.toHaveBeenCalled();
+      expect(replaceState).not.toHaveBeenCalled();
+      expect(fakeWindow.location.href).toEqual('http://server/');
+    });
+
     it('should return $browser to allow chaining', function() {
       expect(browser.url('http://any.com')).toBe(browser);
     });
@@ -468,6 +490,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() {
@@ -546,15 +617,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');
@@ -618,30 +689,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 = [];
-
-      $locationProvider.html5Mode(true);
-    }));
+    }
 
-    it('should update $location when it was changed outside of Angular in sync '+
+    describe('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');
 
-        // Verify that infinite digest reported in #6976 no longer occurs
-        expect(function() {
+      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');
+
+          fakeWindow.location.href = 'http://server/#/someTestHash';
+
+          $rootScope.$digest();
+
+          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();
-        }).not.toThrow();
 
-        expect($location.path()).toBe('/someTestHash');
+          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() {