diff --git a/src/ng/browser.js b/src/ng/browser.js index 928de956cda2..55090832a740 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -87,7 +87,7 @@ function Browser(window, document, $log, $sniffer) { var cachedState, lastHistoryState, lastBrowserUrl = location.href, baseElement = document.find('base'), - reloadLocation = null; + pendingLocation = null; cacheState(); lastHistoryState = cachedState; @@ -147,8 +147,8 @@ function Browser(window, document, $log, $sniffer) { // Do the assignment again so that those two variables are referentially identical. lastHistoryState = cachedState; } else { - if (!sameBase || reloadLocation) { - reloadLocation = url; + if (!sameBase || pendingLocation) { + pendingLocation = url; } if (replace) { location.replace(url); @@ -157,14 +157,18 @@ function Browser(window, document, $log, $sniffer) { } else { location.hash = getHash(url); } + if (location.href !== url) { + pendingLocation = url; + } } return self; // getter } else { - // - reloadLocation is needed as browsers don't allow to read out - // the new location.href if a reload happened. + // - pendingLocation is needed as browsers don't allow to read out + // the new location.href if a reload happened or if there is a bug like in iOS 9 (see + // https://openradar.appspot.com/22186109). // - the replacement is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=407172 - return reloadLocation || location.href.replace(/%27/g,"'"); + return pendingLocation || location.href.replace(/%27/g,"'"); } }; @@ -186,6 +190,7 @@ function Browser(window, document, $log, $sniffer) { urlChangeInit = false; function cacheStateAndFireUrlChange() { + pendingLocation = null; cacheState(); fireUrlChange(); } diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js index 570e8df3cd3b..295f285bf46a 100755 --- a/test/ng/browserSpecs.js +++ b/test/ng/browserSpecs.js @@ -12,12 +12,20 @@ function MockWindow(options) { var events = {}; var timeouts = this.timeouts = []; var locationHref = 'http://server/'; + var committedHref = 'http://server/'; var mockWindow = this; var msie = options.msie; var ieState; historyEntriesLength = 1; + function replaceHash(href, hash) { + // replace the hash with the new one (stripping off a leading hash if there is one) + // See hash setter spec: https://url.spec.whatwg.org/#urlutils-and-urlutilsreadonly-members + return stripHash(href) + '#' + hash.replace(/^#/,''); + } + + this.setTimeout = function(fn) { return timeouts.push(fn) - 1; }; @@ -46,24 +54,28 @@ function MockWindow(options) { this.location = { get href() { - return locationHref; + return committedHref; }, set href(value) { locationHref = value; mockWindow.history.state = null; historyEntriesLength++; + if (!options.updateAsync) this.flushHref(); }, get hash() { - return getHash(locationHref); + return getHash(committedHref); }, set hash(value) { - // replace the hash with the new one (stripping off a leading hash if there is one) - // See hash setter spec: https://url.spec.whatwg.org/#urlutils-and-urlutilsreadonly-members - locationHref = stripHash(locationHref) + '#' + value.replace(/^#/,''); + locationHref = replaceHash(locationHref, value); + if (!options.updateAsync) this.flushHref(); }, replace: function(url) { locationHref = url; mockWindow.history.state = null; + if (!options.updateAsync) this.flushHref(); + }, + flushHref: function() { + committedHref = locationHref; } }; @@ -132,7 +144,7 @@ describe('browser', function() { logs = {log:[], warn:[], info:[], error:[]}; - var fakeLog = {log: function() { logs.log.push(slice.call(arguments)); }, + fakeLog = {log: function() { logs.log.push(slice.call(arguments)); }, warn: function() { logs.warn.push(slice.call(arguments)); }, info: function() { logs.info.push(slice.call(arguments)); }, error: function() { logs.error.push(slice.call(arguments)); }}; @@ -703,7 +715,11 @@ describe('browser', function() { describe('integration tests with $location', function() { function setup(options) { + fakeWindow = new MockWindow(options); + browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer); + module(function($provide, $locationProvider) { + spyOn(fakeWindow.history, 'pushState').andCallFake(function(stateObj, title, newUrl) { fakeWindow.location.href = newUrl; }); @@ -827,6 +843,32 @@ describe('browser', function() { }); }); + + // issue #12241 + it('should not infinite digest if the browser does not synchronously update the location properties', function() { + setup({ + history: true, + html5Mode: true, + updateAsync: true // Simulate a browser that doesn't update the href synchronously + }); + + inject(function($location, $rootScope) { + + // Change the hash within Angular and check that we don't infinitely digest + $location.hash('newHash'); + expect(function() { $rootScope.$digest(); }).not.toThrow(); + expect($location.absUrl()).toEqual('http://server/#newHash'); + + // Now change the hash from outside Angular and check that $location updates correctly + fakeWindow.location.hash = '#otherHash'; + + // simulate next tick - since this browser doesn't update synchronously + fakeWindow.location.flushHref(); + fakeWindow.fire('hashchange'); + + expect($location.absUrl()).toEqual('http://server/#otherHash'); + }); + }); }); describe('integration test with $rootScope', function() {