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 dca23173e2 and d70711481e.

Adds more tests for #6976
Fixes #9235
Closes #9455
This commit is contained in:
Tobias Bosch
2014-10-06 18:04:22 -07:00
parent ab240196bf
commit 8ee1ba4b94
2 changed files with 179 additions and 32 deletions

View File

@@ -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

View File

@@ -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);
$locationProvider.html5Mode(true);
}));
it('should update $location when it was changed outside of Angular in sync '+
describe('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');
// Verify that infinite digest reported in #6976 no longer occurs
expect(function() {
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');
fakeWindow.location.href = 'http://server/#/someTestHash';
$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() {