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

fix($browser): handle async updates to location #12819

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/ng/browser.js
Original file line number Diff line number Diff line change
@@ -87,7 +87,7 @@ function Browser(window, document, $log, $sniffer) {
var cachedState, lastHistoryState,
lastBrowserUrl = location.href,
baseElement = document.find('base'),
reloadLocation = null;
pendingLocation = null;

cacheState();
lastHistoryState = cachedState;
@@ -147,8 +147,8 @@ function Browser(window, document, $log, $sniffer) {
// Do the assignment again so that those two variables are referentially identical.
lastHistoryState = cachedState;
} else {
if (!sameBase || reloadLocation) {
reloadLocation = url;
if (!sameBase || pendingLocation) {
pendingLocation = url;
}
if (replace) {
location.replace(url);
@@ -157,14 +157,18 @@ function Browser(window, document, $log, $sniffer) {
} else {
location.hash = getHash(url);
}
if (location.href !== url) {
pendingLocation = url;
}
}
return self;
// getter
} else {
// - reloadLocation is needed as browsers don't allow to read out
// the new location.href if a reload happened.
// - pendingLocation is needed as browsers don't allow to read out
// the new location.href if a reload happened or if there is a bug like in iOS 9 (see
// https://openradar.appspot.com/22186109).
// - the replacement is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=407172
return reloadLocation || location.href.replace(/%27/g,"'");
return pendingLocation || location.href.replace(/%27/g,"'");
}
};

@@ -186,6 +190,7 @@ function Browser(window, document, $log, $sniffer) {
urlChangeInit = false;

function cacheStateAndFireUrlChange() {
pendingLocation = null;
cacheState();
fireUrlChange();
}
54 changes: 48 additions & 6 deletions test/ng/browserSpecs.js
Original file line number Diff line number Diff line change
@@ -12,12 +12,20 @@ function MockWindow(options) {
var events = {};
var timeouts = this.timeouts = [];
var locationHref = 'http://server/';
var committedHref = 'http://server/';
var mockWindow = this;
var msie = options.msie;
var ieState;

historyEntriesLength = 1;

function replaceHash(href, hash) {
// replace the hash with the new one (stripping off a leading hash if there is one)
// See hash setter spec: https://url.spec.whatwg.org/#urlutils-and-urlutilsreadonly-members
return stripHash(href) + '#' + hash.replace(/^#/,'');
}


this.setTimeout = function(fn) {
return timeouts.push(fn) - 1;
};
@@ -46,24 +54,28 @@ function MockWindow(options) {

this.location = {
get href() {
return locationHref;
return committedHref;
},
set href(value) {
locationHref = value;
mockWindow.history.state = null;
historyEntriesLength++;
if (!options.updateAsync) this.flushHref();
},
get hash() {
return getHash(locationHref);
return getHash(committedHref);
},
set hash(value) {
// replace the hash with the new one (stripping off a leading hash if there is one)
// See hash setter spec: https://url.spec.whatwg.org/#urlutils-and-urlutilsreadonly-members
locationHref = stripHash(locationHref) + '#' + value.replace(/^#/,'');
locationHref = replaceHash(locationHref, value);
if (!options.updateAsync) this.flushHref();
},
replace: function(url) {
locationHref = url;
mockWindow.history.state = null;
if (!options.updateAsync) this.flushHref();
},
flushHref: function() {
committedHref = locationHref;
}
};

@@ -132,7 +144,7 @@ describe('browser', function() {

logs = {log:[], warn:[], info:[], error:[]};

var fakeLog = {log: function() { logs.log.push(slice.call(arguments)); },
fakeLog = {log: function() { logs.log.push(slice.call(arguments)); },
warn: function() { logs.warn.push(slice.call(arguments)); },
info: function() { logs.info.push(slice.call(arguments)); },
error: function() { logs.error.push(slice.call(arguments)); }};
@@ -703,7 +715,11 @@ describe('browser', function() {
describe('integration tests with $location', function() {

function setup(options) {
fakeWindow = new MockWindow(options);
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);

module(function($provide, $locationProvider) {

spyOn(fakeWindow.history, 'pushState').andCallFake(function(stateObj, title, newUrl) {
fakeWindow.location.href = newUrl;
});
@@ -827,6 +843,32 @@ describe('browser', function() {
});

});

// issue #12241
it('should not infinite digest if the browser does not synchronously update the location properties', function() {
setup({
history: true,
html5Mode: true,
updateAsync: true // Simulate a browser that doesn't update the href synchronously
});

inject(function($location, $rootScope) {

// Change the hash within Angular and check that we don't infinitely digest
$location.hash('newHash');
expect(function() { $rootScope.$digest(); }).not.toThrow();
expect($location.absUrl()).toEqual('http://server/#newHash');

// Now change the hash from outside Angular and check that $location updates correctly
fakeWindow.location.hash = '#otherHash';

// simulate next tick - since this browser doesn't update synchronously
fakeWindow.location.flushHref();
fakeWindow.fire('hashchange');

expect($location.absUrl()).toEqual('http://server/#otherHash');
});
});
});

describe('integration test with $rootScope', function() {