From 197683045de2efde9680236f614cefa14ba09993 Mon Sep 17 00:00:00 2001
From: Tobias Bosch <tbosch1009@gmail.com>
Date: Mon, 6 Oct 2014 18:04:22 -0700
Subject: [PATCH] fix($browser): Cache `location.href` only during page reload
phase
Adds caching for url changes while a reload is happening,
as browsers do not allow to read out the new location the browser
is navigating to.
Removes unnecessary caching from $browser, as IE7-IE9 all
have the new hash value in `location.href` after changing it.
There was a wrong assumption in the previous version of this code
introduced by dca23173e25a32cb740245ca7f7b01a84805f43f and d70711481e6311c9cd283d650f07ca0cca72ecc2.
Adds more tests for #6976
Fixes #9235
---
src/ng/browser.js | 19 ++--
test/ng/browserSpecs.js | 190 +++++++++++++++++++++++++++++++++++-----
2 files changed, 178 insertions(+), 31 deletions(-)
diff --git a/src/ng/browser.js b/src/ng/browser.js
index e78c90ffd37b..1f86da6cb3f3 100644
--- a/src/ng/browser.js
+++ b/src/ng/browser.js
@@ -125,7 +125,7 @@ function Browser(window, document, $log, $sniffer) {
var lastBrowserUrl = location.href,
baseElement = document.find('base'),
- newLocation = null;
+ reloadLocation = null;
/**
* @name $browser#url
@@ -166,7 +166,9 @@ function Browser(window, document, $log, $sniffer) {
history.pushState(null, '', url);
}
} else {
- newLocation = url;
+ if (!sameBase) {
+ reloadLocation = url;
+ }
if (replace) {
location.replace(url);
} else {
@@ -176,10 +178,10 @@ 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,"'");
}
};
@@ -187,11 +189,6 @@ function Browser(window, document, $log, $sniffer) {
urlChangeInit = false;
function fireUrlChange() {
- newLocation = null;
- checkUrlChange();
- }
-
- function checkUrlChange() {
if (lastBrowserUrl == self.url()) return;
lastBrowserUrl = self.url();
@@ -247,7 +244,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
diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js
index ffd393b8dd24..a50a9ed4c7e2 100755
--- a/test/ng/browserSpecs.js
+++ b/test/ng/browserSpecs.js
@@ -495,6 +495,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() {
@@ -573,15 +622,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');
@@ -645,33 +694,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 = [];
+ }
- sniffer.history = true;
- $provide.value('$sniffer', sniffer);
+ describe('update $location when it was changed outside of Angular in sync '+
+ 'before $digest was called', function() {
- $locationProvider.html5Mode(true);
- }));
+ 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');
- it('should 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');
+ fakeWindow.location.href = 'http://server/#/someTestHash';
- // Verify that infinite digest reported in #6976 no longer occurs
- expect(function() {
$rootScope.$digest();
- }).not.toThrow();
- expect($location.path()).toBe('/someTestHash');
+ 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();
+
+ 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() {