diff --git a/src/ng/browser.js b/src/ng/browser.js index e3942170a4d4..55cb01a7af73 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -96,7 +96,6 @@ function Browser(window, document, $log, $sniffer) { }; cacheState(); - lastHistoryState = cachedState; /** * @name $browser#url @@ -150,8 +149,6 @@ function Browser(window, document, $log, $sniffer) { if ($sniffer.history && (!sameBase || !sameState)) { history[replace ? 'replaceState' : 'pushState'](state, '', url); cacheState(); - // Do the assignment again so that those two variables are referentially identical. - lastHistoryState = cachedState; } else { if (!sameBase) { pendingLocation = url; @@ -200,8 +197,7 @@ function Browser(window, document, $log, $sniffer) { function cacheStateAndFireUrlChange() { pendingLocation = null; - cacheState(); - fireUrlChange(); + fireStateOrUrlChange(); } // This variable should be used *only* inside the cacheState function. @@ -215,11 +211,16 @@ function Browser(window, document, $log, $sniffer) { if (equals(cachedState, lastCachedState)) { cachedState = lastCachedState; } + lastCachedState = cachedState; + lastHistoryState = cachedState; } - function fireUrlChange() { - if (lastBrowserUrl === self.url() && lastHistoryState === cachedState) { + function fireStateOrUrlChange() { + var prevLastHistoryState = lastHistoryState; + cacheState(); + + if (lastBrowserUrl === self.url() && prevLastHistoryState === cachedState) { return; } @@ -285,7 +286,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 = fireUrlChange; + self.$$checkUrlChange = fireStateOrUrlChange; ////////////////////////////////////////////////////////////// // Misc API diff --git a/src/ng/location.js b/src/ng/location.js index 905407e4e191..b92ff8ca38c4 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -137,6 +137,8 @@ function LocationHtml5Url(appBase, appBaseNoFile, basePrefix) { this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash; this.$$absUrl = appBaseNoFile + this.$$url.substr(1); // first char is always '/' + + this.$$urlUpdatedByLocation = true; }; this.$$parseLinkUrl = function(url, relHref) { @@ -270,6 +272,8 @@ function LocationHashbangUrl(appBase, appBaseNoFile, hashPrefix) { this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash; this.$$absUrl = appBase + (this.$$url ? hashPrefix + this.$$url : ''); + + this.$$urlUpdatedByLocation = true; }; this.$$parseLinkUrl = function(url, relHref) { @@ -327,6 +331,8 @@ function LocationHashbangInHtml5Url(appBase, appBaseNoFile, hashPrefix) { this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash; // include hashPrefix in $$absUrl when $$url is empty so IE9 does not reload page because of removal of '#' this.$$absUrl = appBase + hashPrefix + this.$$url; + + this.$$urlUpdatedByLocation = true; }; } @@ -656,6 +662,7 @@ forEach([LocationHashbangInHtml5Url, LocationHashbangUrl, LocationHtml5Url], fun // but we're changing the $$state reference to $browser.state() during the $digest // so the modification window is narrow. this.$$state = isUndefined(state) ? null : state; + this.$$urlUpdatedByLocation = true; return this; }; @@ -968,36 +975,40 @@ function $LocationProvider() { // update browser $rootScope.$watch(function $locationWatch() { - 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); + if (initializing || $location.$$urlUpdatedByLocation) { + $location.$$urlUpdatedByLocation = false; + + 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); + } + afterLocationChange(oldUrl, oldState); } - afterLocationChange(oldUrl, oldState); - } - }); + }); + } } $location.$$replace = false; diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 89279608464c..690d42d430c8 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -876,6 +876,10 @@ function $RootScopeProvider() { } } postDigestQueue.length = postDigestQueuePosition = 0; + + // Check for changes to browser url that happened during the $digest + // (for which no event is fired; e.g. via `history.pushState()`) + $browser.$$checkUrlChange(); }, diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index a2501bea0016..3ab431234dff 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -710,6 +710,7 @@ describe('$location', function() { ); }); + it('should not infinitely digest when using a semicolon in initial path', function() { initService({html5Mode:true,supportHistory:true}); mockUpBrowser({initialUrl:'http://localhost:9876/;jsessionid=foo', baseHref:'/'}); @@ -721,6 +722,63 @@ describe('$location', function() { }); + describe('when changing the browser URL/history directly during a `$digest`', function() { + + beforeEach(function() { + initService({supportHistory: true}); + mockUpBrowser({initialUrl: 'http://foo.bar/', baseHref: '/'}); + }); + + + it('should correctly update `$location` from history and not digest infinitely', inject( + function($browser, $location, $rootScope, $window) { + $location.url('baz'); + $rootScope.$digest(); + + var originalUrl = $window.location.href; + + $rootScope.$apply(function() { + $rootScope.$evalAsync(function() { + $window.history.pushState({}, null, originalUrl + '/qux'); + }); + }); + + expect($browser.url()).toBe('http://foo.bar/#!/baz/qux'); + expect($location.absUrl()).toBe('http://foo.bar/#!/baz/qux'); + + $rootScope.$apply(function() { + $rootScope.$evalAsync(function() { + $window.history.replaceState({}, null, originalUrl + '/quux'); + }); + }); + + expect($browser.url()).toBe('http://foo.bar/#!/baz/quux'); + expect($location.absUrl()).toBe('http://foo.bar/#!/baz/quux'); + }) + ); + + + it('should correctly update `$location` from URL and not digest infinitely', inject( + function($browser, $location, $rootScope, $window) { + $location.url('baz'); + $rootScope.$digest(); + + $rootScope.$apply(function() { + $rootScope.$evalAsync(function() { + $window.location.href += '/qux'; + }); + }); + + jqLite($window).triggerHandler('hashchange'); + + expect($browser.url()).toBe('http://foo.bar/#!/baz/qux'); + expect($location.absUrl()).toBe('http://foo.bar/#!/baz/qux'); + }) + ); + + }); + + function updatePathOnLocationChangeSuccessTo(newPath) { inject(function($rootScope, $location) { $rootScope.$on('$locationChangeSuccess', function(event, newUrl, oldUrl) { @@ -1039,11 +1097,21 @@ describe('$location', function() { }); it('should update $location when browser state changes', function() { - initService({html5Mode:true, supportHistory: true}); - mockUpBrowser({initialUrl:'http://new.com/a/b/', baseHref:'/a/b/'}); - inject(function($location, $window) { + initService({html5Mode: true, supportHistory: true}); + mockUpBrowser({initialUrl: 'http://new.com/a/b/', baseHref: '/a/b/'}); + inject(function($location, $rootScope, $window) { $window.history.pushState({b: 3}); + $rootScope.$digest(); + expect($location.state()).toEqual({b: 3}); + + $window.history.pushState({b: 4}, null, $window.location.href + 'c?d=e#f'); + $rootScope.$digest(); + + expect($location.path()).toBe('/c'); + expect($location.search()).toEqual({d: 'e'}); + expect($location.hash()).toBe('f'); + expect($location.state()).toEqual({b: 4}); }); }); @@ -2666,12 +2734,10 @@ describe('$location', function() { replaceState: function(state, title, url) { win.history.state = copy(state); if (url) win.location.href = url; - jqLite(win).triggerHandler('popstate'); }, pushState: function(state, title, url) { win.history.state = copy(state); if (url) win.location.href = url; - jqLite(win).triggerHandler('popstate'); } }; win.addEventListener = angular.noop;