From e97856cc88c5cc674a3dd7d5111dc9153e760d9e Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 5 Nov 2014 13:49:01 +0000 Subject: [PATCH 1/5] fix($browser): prevent infinite digests when clearing the hash of a url By retaining a trailing `#` character in the URL we ensure that the browser does not attempt a reload, which in turn allows us to read the correct `location.href` value. Closes #9635 --- src/ng/browser.js | 8 ++++++++ test/ng/browserSpecs.js | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/ng/browser.js b/src/ng/browser.js index 381624567d5d..e08e9aa17077 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -188,6 +188,14 @@ function Browser(window, document, $log, $sniffer) { } else { if (!sameBase) { reloadLocation = url; + } else { + // If we are only changing the hash fragment then ensure that we retain a # character + // to prevent the page reloading, + // which stops us from reading the correct location.href, + // which causes $location watches to trigger an infinite digest. + if (url.indexOf('#') === -1) { + url += '#'; + } } if (replace) { location.replace(url); diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js index f14596a42e52..7aeca8e6a882 100755 --- a/test/ng/browserSpecs.js +++ b/test/ng/browserSpecs.js @@ -550,6 +550,17 @@ describe('browser', function() { expect(locationReplace).not.toHaveBeenCalled(); }); + it("should retain the # character when the only change is clearing the hash fragment, to prevent page reload", function() { + sniffer.history = true; + + browser.url('http://server/#123'); + expect(fakeWindow.location.href).toEqual('http://server/#123'); + + browser.url('http://server/'); + expect(fakeWindow.location.href).toEqual('http://server/#'); + + }); + it('should use location.replace when history.replaceState not available', function() { sniffer.history = false; browser.url('http://new.org', true); @@ -561,6 +572,7 @@ 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); @@ -572,6 +584,19 @@ describe('browser', function() { expect(fakeWindow.location.href).toEqual('http://server/'); }); + + it("should retain the # character when replacing and the only change is clearing the hash fragment, to prevent page reload", function() { + sniffer.history = true; + browser.url('http://server/#123'); + + expect(fakeWindow.location.href).toEqual('http://server/#123'); + + browser.url('http://server/', true); + + expect(locationReplace).toHaveBeenCalledWith('http://server/#'); + }); + + it('should return $browser to allow chaining', function() { expect(browser.url('http://any.com')).toBe(browser); }); From f45d6d994a6b651b3cf5add4143799dc486a8d77 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Wed, 5 Nov 2014 13:49:01 +0000 Subject: [PATCH 2/5] fix($location): strip off empty hash segments when comparing The url is the same whether or not there is an empty `#` marker at the end. This prevents unwanted calls to update the browser, since the browser is automatically applying an empty hash if necessary to prevent page reloads. Closes #9635 --- src/ng/location.js | 9 +++++++-- test/ng/locationSpec.js | 12 ++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/ng/location.js b/src/ng/location.js index a993757d5688..3621eb426380 100644 --- a/src/ng/location.js +++ b/src/ng/location.js @@ -68,6 +68,10 @@ function stripHash(url) { return index == -1 ? url : url.substr(0, index); } +function trimEmptyHash(url) { + return url.replace(/(#.+)|#$/, '$1'); +} + function stripFile(url) { return url.substr(0, stripHash(url).lastIndexOf('/') + 1); @@ -847,10 +851,11 @@ function $LocationProvider() { // update browser $rootScope.$watch(function $locationWatch() { - var oldUrl = $browser.url(); + var oldUrl = trimEmptyHash($browser.url()); + var newUrl = trimEmptyHash($location.absUrl()); var oldState = $browser.state(); var currentReplace = $location.$$replace; - var urlOrStateChanged = oldUrl !== $location.absUrl() || + var urlOrStateChanged = oldUrl !== newUrl || ($location.$$html5 && $sniffer.history && oldState !== $location.$$state); if (initializing || urlOrStateChanged) { diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 5a338729cf00..3a6b482e9826 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -639,6 +639,18 @@ describe('$location', function() { }; } + describe('location watch', function() { + beforeEach(initService({supportHistory: true})); + beforeEach(inject(initBrowser({url:'http://new.com/a/b#'}))); + + it('should not update browser if only the empty hash fragment is cleared by updating the search', inject(function($rootScope, $browser, $location) { + $browser.url('http://new.com/a/b#'); + var $browserUrl = spyOnlyCallsWithArgs($browser, 'url').andCallThrough(); + $rootScope.$digest(); + expect($browserUrl).not.toHaveBeenCalled(); + })); + }); + describe('wiring', function() { beforeEach(initService({html5Mode:false,hashPrefix: '!',supportHistory: true})); From 1433c34478adaf134b1ee69239cd2a3c4d2a216b Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Mon, 10 Nov 2014 12:09:29 +0000 Subject: [PATCH 3/5] test($location): fix typo --- test/ng/locationSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 3a6b482e9826..1052191981e9 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -1236,7 +1236,7 @@ describe('$location', function() { }); - it('should not rewrite full url links do different domain', function() { + it('should not rewrite full url links to different domain', function() { configureService({linkHref: 'http://www.dot.abc/a?b=c', html5Mode: true}); inject( initBrowser(), From a2be4b1ac8d8e6bd1d42a41ec89abc8da4ba9d58 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Mon, 10 Nov 2014 12:32:42 +0000 Subject: [PATCH 4/5] test($location): fix test of `{rewriteLinks:false}` The test for not rewriting links was invalid and just happened to be passing by chance (false-positive). --- test/ng/locationSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ng/locationSpec.js b/test/ng/locationSpec.js index 1052191981e9..9331490e2dc6 100644 --- a/test/ng/locationSpec.js +++ b/test/ng/locationSpec.js @@ -1302,7 +1302,7 @@ describe('$location', function() { it ('should not rewrite links when rewriting links is disabled', function() { - configureService('/a?b=c', true, true, '', 'some content', false); + configureService({linkHref: 'link?a#b', html5Mode: {enabled: true, rewriteLinks:false}, supportHist: true}); inject( initBrowser(), initLocation(), From e1d6f294afb007d2cf8ae7076646b8c1a72e2f9c Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 11 Nov 2014 17:10:29 +0000 Subject: [PATCH 5/5] WIP: try using hash again --- src/ng/browser.js | 17 ++++++++--------- test/ng/browserSpecs.js | 20 ++++++++------------ 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/ng/browser.js b/src/ng/browser.js index e08e9aa17077..8abcf87f1296 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -62,6 +62,11 @@ function Browser(window, document, $log, $sniffer) { } } + function getHash(url) { + var index = url.indexOf('#'); + return index === -1 ? '' : url.substr(index + 1); + } + /** * @private * Note: this method is used only by scenario runner @@ -188,19 +193,13 @@ function Browser(window, document, $log, $sniffer) { } else { if (!sameBase) { reloadLocation = url; - } else { - // If we are only changing the hash fragment then ensure that we retain a # character - // to prevent the page reloading, - // which stops us from reading the correct location.href, - // which causes $location watches to trigger an infinite digest. - if (url.indexOf('#') === -1) { - url += '#'; - } } if (replace) { location.replace(url); - } else { + } else if (!sameBase) { location.href = url; + } else { + location.hash = getHash(url); } } return self; diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js index 7aeca8e6a882..364e42b2a0cb 100755 --- a/test/ng/browserSpecs.js +++ b/test/ng/browserSpecs.js @@ -1,5 +1,7 @@ 'use strict'; +/* global getHash:true, stripHash:true */ + var historyEntriesLength; var sniffer = {}; @@ -51,6 +53,12 @@ function MockWindow(options) { mockWindow.history.state = null; historyEntriesLength++; }, + get hash() { + return getHash(locationHref); + }, + set hash(value) { + locationHref = stripHash(locationHref) + '#' + value; + }, replace: function(url) { locationHref = url; mockWindow.history.state = null; @@ -585,18 +593,6 @@ describe('browser', function() { }); - it("should retain the # character when replacing and the only change is clearing the hash fragment, to prevent page reload", function() { - sniffer.history = true; - browser.url('http://server/#123'); - - expect(fakeWindow.location.href).toEqual('http://server/#123'); - - browser.url('http://server/', true); - - expect(locationReplace).toHaveBeenCalledWith('http://server/#'); - }); - - it('should return $browser to allow chaining', function() { expect(browser.url('http://any.com')).toBe(browser); });