From 8d39bd8abf423517b5bff70137c2a29e32bff76d Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Thu, 10 Sep 2015 23:20:23 +0100 Subject: [PATCH] fix($browser): handle async updates to location Both browser reloads and iOS 9 bugs cause the window.location to report a different href that which we have just set. The change does not become available until the next tick. This change generalises previous work to deal with reloads to deal with the iOS 9 bug in the UIWebView component. Closes #12241 Closes #12819 --- src/ng/browser.js | 17 ++++++++----- test/ng/browserSpecs.js | 54 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 12 deletions(-) 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() {