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

Commit

Permalink
fix($location): correctly handle external URL change during $digest
Browse files Browse the repository at this point in the history
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 was an issue when calling `history.pushState()/replaceState()` in all
browsers, since these methods do not emit any event. This was also an issue when
setting `location.href` in IE11, where (unlike other browsers) no `popstate`
event is fired at all for hash-only changes ([known bug][1]) and the
`hashchange` event is fired asynchronously (which is too late).

This commit fixes both usecases by:

1. 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/history
   has been updated directly and the change hasn't yet been propagated to
   `$location` (e.g. due to no event being fired synchronously or at all).
2. Checking for URL/state changes at the end of the `$digest`, in order to
   detect changes via `history` methods (that took place during the `$digest`).

[1]: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/3740423/

Fixes #11075
Fixes #12571
Fixes #15556

Closes #15561
  • Loading branch information
gkalpak committed Jan 9, 2017
1 parent 2b360bf commit 752f411
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 29 deletions.
69 changes: 40 additions & 29 deletions src/ng/location.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ function LocationHtml5Url(appBase, appBaseNoFile, basePrefix) {

this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
this.$$absUrl = appBaseNoFile + this.$$url.substr(1); // first char is always '/'

this.$$urlUpdatedByLocation = true;
};

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

this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
this.$$absUrl = appBase + (this.$$url ? hashPrefix + this.$$url : '');

this.$$urlUpdatedByLocation = true;
};

this.$$parseLinkUrl = function(url, relHref) {
Expand Down Expand Up @@ -327,6 +331,8 @@ function LocationHashbangInHtml5Url(appBase, appBaseNoFile, hashPrefix) {
this.$$url = encodePath(this.$$path) + (search ? '?' + search : '') + hash;
// include hashPrefix in $$absUrl when $$url is empty so IE9 does not reload page because of removal of '#'
this.$$absUrl = appBase + hashPrefix + this.$$url;

this.$$urlUpdatedByLocation = true;
};

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

return this;
};
Expand Down Expand Up @@ -968,36 +975,40 @@ function $LocationProvider() {

// update browser
$rootScope.$watch(function $locationWatch() {
var oldUrl = trimEmptyHash($browser.url());
var newUrl = trimEmptyHash($location.absUrl());
var oldState = $browser.state();
var currentReplace = $location.$$replace;
var urlOrStateChanged = oldUrl !== newUrl ||
($location.$$html5 && $sniffer.history && oldState !== $location.$$state);

if (initializing || urlOrStateChanged) {
initializing = false;

$rootScope.$evalAsync(function() {
var newUrl = $location.absUrl();
var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
$location.$$state, oldState).defaultPrevented;

// if the location was changed by a `$locationChangeStart` handler then stop
// processing this location change
if ($location.absUrl() !== newUrl) return;

if (defaultPrevented) {
$location.$$parse(oldUrl);
$location.$$state = oldState;
} else {
if (urlOrStateChanged) {
setBrowserUrlWithFallback(newUrl, currentReplace,
oldState === $location.$$state ? null : $location.$$state);
if (initializing || $location.$$urlUpdatedByLocation) {
$location.$$urlUpdatedByLocation = false;

var oldUrl = trimEmptyHash($browser.url());
var newUrl = trimEmptyHash($location.absUrl());
var oldState = $browser.state();
var currentReplace = $location.$$replace;
var urlOrStateChanged = oldUrl !== newUrl ||
($location.$$html5 && $sniffer.history && oldState !== $location.$$state);

if (initializing || urlOrStateChanged) {
initializing = false;

$rootScope.$evalAsync(function() {
var newUrl = $location.absUrl();
var defaultPrevented = $rootScope.$broadcast('$locationChangeStart', newUrl, oldUrl,
$location.$$state, oldState).defaultPrevented;

// if the location was changed by a `$locationChangeStart` handler then stop
// processing this location change
if ($location.absUrl() !== newUrl) return;

if (defaultPrevented) {
$location.$$parse(oldUrl);
$location.$$state = oldState;
} else {
if (urlOrStateChanged) {
setBrowserUrlWithFallback(newUrl, currentReplace,
oldState === $location.$$state ? null : $location.$$state);
}
afterLocationChange(oldUrl, oldState);
}
afterLocationChange(oldUrl, oldState);
}
});
});
}
}

$location.$$replace = false;
Expand Down
4 changes: 4 additions & 0 deletions src/ng/rootScope.js
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,10 @@ function $RootScopeProvider() {
}
}
postDigestQueue.length = postDigestQueuePosition = 0;

// Check for changes to browser url that happened during the $digest
// (for which no event is fired; e.g. via `history.pushState()`)
$browser.$$checkUrlChange();
},


Expand Down
58 changes: 58 additions & 0 deletions test/ng/locationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ describe('$location', function() {
);
});


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


describe('when changing the browser URL/history directly during a `$digest`', function() {

beforeEach(function() {
initService({supportHistory: true});
mockUpBrowser({initialUrl: 'http://foo.bar/', baseHref: '/'});
});


it('should correctly update `$location` from history and not digest infinitely', inject(
function($browser, $location, $rootScope, $window) {
$location.url('baz');
$rootScope.$digest();

var originalUrl = $window.location.href;

$rootScope.$apply(function() {
$rootScope.$evalAsync(function() {
$window.history.pushState({}, null, originalUrl + '/qux');
});
});

expect($browser.url()).toBe('http://foo.bar/#!/baz/qux');
expect($location.absUrl()).toBe('http://foo.bar/#!/baz/qux');

$rootScope.$apply(function() {
$rootScope.$evalAsync(function() {
$window.history.replaceState({}, null, originalUrl + '/quux');
});
});

expect($browser.url()).toBe('http://foo.bar/#!/baz/quux');
expect($location.absUrl()).toBe('http://foo.bar/#!/baz/quux');
})
);


it('should correctly update `$location` from URL and not digest infinitely', inject(
function($browser, $location, $rootScope, $window) {
$location.url('baz');
$rootScope.$digest();

$rootScope.$apply(function() {
$rootScope.$evalAsync(function() {
$window.location.href += '/qux';
});
});

jqLite($window).triggerHandler('hashchange');

expect($browser.url()).toBe('http://foo.bar/#!/baz/qux');
expect($location.absUrl()).toBe('http://foo.bar/#!/baz/qux');
})
);

});


function updatePathOnLocationChangeSuccessTo(newPath) {
inject(function($rootScope, $location) {
$rootScope.$on('$locationChangeSuccess', function(event, newUrl, oldUrl) {
Expand Down

0 comments on commit 752f411

Please sign in to comment.