Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix($browser): Cache location.href only during page reload phase
Browse files Browse the repository at this point in the history 
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 dca2317 and d707114.

Adds more tests for #6976
Fixes #9235
Closes #9455
  • Loading branch information
@tbosch
tbosch committed Oct 7, 2014
1 parent ab24019 commit 8ee1ba4
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 31 deletions.
19 changes: 8 additions & 11 deletions src/ng/browser.js
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -176,22 +178,17 @@ 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,"'");
}
};

var urlChangeListeners = [],
urlChangeInit = false;

function fireUrlChange() {
newLocation = null;
checkUrlChange();
}

function checkUrlChange() {
if (lastBrowserUrl == self.url()) return;

lastBrowserUrl = self.url();
Expand Down Expand Up @@ -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
Expand Down
190 changes: 170 additions & 20 deletions test/ng/browserSpecs.js
View file
Expand Up @@ -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() {
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 8ee1ba4

Please sign in to comment.