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

Commit 2e7a128

Browse files
committed
fix($location): correctly handle external URL change during $digest
Previously, when the URL was changed directly (e.g. via `location.href`) during a `$digest` (e.g. via `scope.$evalAsync()` or `promise.then()`) the change was not handled correctly, unless a `popstate` or `hashchange` event was fired synchronously. This wasn't an issue in most browsers, where the `popstate` event was fired synchronously. In IE11 though, the `popstate` event is not fired at all for hash-only changes ([known bug][1]) and the `hashchange` event is fired asynchronously (which is too late). This commit fixes it by keeping track of `$location` setter methods being called and only processing a URL change if it originated from such a call. If there is a URL difference but no setter method has been called, this means that the browser URL has been updated directly and the change hasn't yet been propagated to `$location` (e.g. due to no synchronous `popstate` event). In that case, the change will be propagated later by the (asynchronous) `hashchange` event. [1]: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/3740423/ Fixes #15556
1 parent a24777a commit 2e7a128

File tree

2 files changed

+71
-29
lines changed

2 files changed

+71
-29
lines changed

src/ng/location.js

+40-29
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ function LocationHtml5Url(appBase, appBaseNoFile, basePrefix) {
137137

138138
this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
139139
this.$$absUrl = appBaseNoFile + this.$$url.substr(1); // first char is always '/'
140+
141+
this.$$urlUpdatedByLocation = true;
140142
};
141143

142144
this.$$parseLinkUrl = function(url, relHref) {
@@ -270,6 +272,8 @@ function LocationHashbangUrl(appBase, appBaseNoFile, hashPrefix) {
270272

271273
this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
272274
this.$$absUrl = appBase + (this.$$url ? hashPrefix + this.$$url : '');
275+
276+
this.$$urlUpdatedByLocation = true;
273277
};
274278

275279
this.$$parseLinkUrl = function(url, relHref) {
@@ -327,6 +331,8 @@ function LocationHashbangInHtml5Url(appBase, appBaseNoFile, hashPrefix) {
327331
this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
328332
// include hashPrefix in $$absUrl when $$url is empty so IE9 does not reload page because of removal of '#'
329333
this.$$absUrl = appBase + hashPrefix + this.$$url;
334+
335+
this.$$urlUpdatedByLocation = true;
330336
};
331337

332338
}
@@ -656,6 +662,7 @@ forEach([LocationHashbangInHtml5Url, LocationHashbangUrl, LocationHtml5Url], fun
656662
// but we're changing the $$state reference to $browser.state() during the $digest
657663
// so the modification window is narrow.
658664
this.$$state = isUndefined(state) ? null : state;
665+
this.$$urlUpdatedByLocation = true;
659666

660667
return this;
661668
};
@@ -968,36 +975,40 @@ function $LocationProvider() {
968975

969976
// update browser
970977
$rootScope.$watch(function $locationWatch() {
971-
var oldUrl = trimEmptyHash($browser.url());
972-
var newUrl = trimEmptyHash($location.absUrl());
973-
var oldState = $browser.state();
974-
var currentReplace = $location.$$replace;
975-
var urlOrStateChanged = oldUrl !== newUrl ||
976-
($location.$$html5 && $sniffer.history && oldState !== $location.$$state);
977-
978-
if (initializing || urlOrStateChanged) {
979-
initializing = false;
980-
981-
$rootScope.$evalAsync(function() {
982-
var newUrl = $location.absUrl();
983-
var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
984-
$location.$$state, oldState).defaultPrevented;
985-
986-
// if the location was changed by a `$locationChangeStart` handler then stop
987-
// processing this location change
988-
if ($location.absUrl() !== newUrl) return;
989-
990-
if (defaultPrevented) {
991-
$location.$$parse(oldUrl);
992-
$location.$$state = oldState;
993-
} else {
994-
if (urlOrStateChanged) {
995-
setBrowserUrlWithFallback(newUrl, currentReplace,
996-
oldState === $location.$$state ? null : $location.$$state);
978+
if (initializing || $location.$$urlUpdatedByLocation) {
979+
$location.$$urlUpdatedByLocation = false;
980+
981+
var oldUrl = trimEmptyHash($browser.url());
982+
var newUrl = trimEmptyHash($location.absUrl());
983+
var oldState = $browser.state();
984+
var currentReplace = $location.$$replace;
985+
var urlOrStateChanged = oldUrl !== newUrl ||
986+
($location.$$html5 && $sniffer.history && oldState !== $location.$$state);
987+
988+
if (initializing || urlOrStateChanged) {
989+
initializing = false;
990+
991+
$rootScope.$evalAsync(function() {
992+
var newUrl = $location.absUrl();
993+
var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
994+
$location.$$state, oldState).defaultPrevented;
995+
996+
// if the location was changed by a `$locationChangeStart` handler then stop
997+
// processing this location change
998+
if ($location.absUrl() !== newUrl) return;
999+
1000+
if (defaultPrevented) {
1001+
$location.$$parse(oldUrl);
1002+
$location.$$state = oldState;
1003+
} else {
1004+
if (urlOrStateChanged) {
1005+
setBrowserUrlWithFallback(newUrl, currentReplace,
1006+
oldState === $location.$$state ? null : $location.$$state);
1007+
}
1008+
afterLocationChange(oldUrl, oldState);
9971009
}
998-
afterLocationChange(oldUrl, oldState);
999-
}
1000-
});
1010+
});
1011+
}
10011012
}
10021013

10031014
$location.$$replace = false;

test/ng/locationSpec.js

+31
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,7 @@ describe('$location', function() {
710710
);
711711
});
712712

713+
713714
it('should not infinitely digest when using a semicolon in initial path', function() {
714715
initService({html5Mode:true,supportHistory:true});
715716
mockUpBrowser({initialUrl:'http://localhost:9876/;jsessionid=foo', baseHref:'/'});
@@ -721,6 +722,36 @@ describe('$location', function() {
721722
});
722723

723724

725+
// Support: IE11
726+
describe('when changing the browser URL directly during a `$digest`', function() {
727+
728+
beforeEach(function() {
729+
initService({supportHistory: true});
730+
mockUpBrowser({initialUrl: 'http://foo.bar/', baseHref: '/'});
731+
});
732+
733+
734+
it('should correctly update `$location` and not digest infinitely', inject(
735+
function($browser, $location, $rootScope, $window) {
736+
$location.url('baz');
737+
$rootScope.$digest();
738+
739+
$rootScope.$apply(function() {
740+
$rootScope.$evalAsync(function() {
741+
$window.location.href += '/qux';
742+
});
743+
});
744+
745+
jqLite($window).triggerHandler('hashchange');
746+
747+
expect($browser.url()).toBe('http://foo.bar/#!/baz/qux');
748+
expect($location.absUrl()).toBe('http://foo.bar/#!/baz/qux');
749+
})
750+
);
751+
752+
});
753+
754+
724755
function updatePathOnLocationChangeSuccessTo(newPath) {
725756
inject(function($rootScope, $location) {
726757
$rootScope.$on('$locationChangeSuccess', function(event, newUrl, oldUrl) {

0 commit comments

Comments
 (0)