fix(*): detect external changes in history.state
Previously, `$browser.$$checkUrlChange()` (which was run before each `$digest`) would only detect an external change (i.e. not via `$location`) to the browser URL. External changes to `history.state` would not be detected and propagated to `$location`. This would not be a problem if changes were followed by a `popstate` or `hashchange` event (which would call `cacheStateAndFireUrlChange()`). But since `history.pushState()/replaceState()` do not fire any events, calling these methods manually would result in `$location` getting out-of-sync with the actual history state. This was not detected in tests, because the mocked `window.history` would incorrectly trigger `popstate` when calling `pushState()/replaceState()`, which "covered" the bug. This commit fixes it by always calling `cacheState()`, before looking for and propagating a URL/state change.
This commit is contained in:
+9
-8
@@ -96,7 +96,6 @@ function Browser(window, document, $log, $sniffer) {
|
||||
};
|
||||
|
||||
cacheState();
|
||||
lastHistoryState = cachedState;
|
||||
|
||||
/**
|
||||
* @name $browser#url
|
||||
@@ -150,8 +149,6 @@ function Browser(window, document, $log, $sniffer) {
|
||||
if ($sniffer.history && (!sameBase || !sameState)) {
|
||||
history[replace ? 'replaceState' : 'pushState'](state, '', url);
|
||||
cacheState();
|
||||
// Do the assignment again so that those two variables are referentially identical.
|
||||
lastHistoryState = cachedState;
|
||||
} else {
|
||||
if (!sameBase) {
|
||||
pendingLocation = url;
|
||||
@@ -200,8 +197,7 @@ function Browser(window, document, $log, $sniffer) {
|
||||
|
||||
function cacheStateAndFireUrlChange() {
|
||||
pendingLocation = null;
|
||||
cacheState();
|
||||
fireUrlChange();
|
||||
fireStateOrUrlChange();
|
||||
}
|
||||
|
||||
// This variable should be used *only* inside the cacheState function.
|
||||
@@ -215,11 +211,16 @@ function Browser(window, document, $log, $sniffer) {
|
||||
if (equals(cachedState, lastCachedState)) {
|
||||
cachedState = lastCachedState;
|
||||
}
|
||||
|
||||
lastCachedState = cachedState;
|
||||
lastHistoryState = cachedState;
|
||||
}
|
||||
|
||||
function fireUrlChange() {
|
||||
if (lastBrowserUrl === self.url() && lastHistoryState === cachedState) {
|
||||
function fireStateOrUrlChange() {
|
||||
var prevLastHistoryState = lastHistoryState;
|
||||
cacheState();
|
||||
|
||||
if (lastBrowserUrl === self.url() && prevLastHistoryState === cachedState) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -285,7 +286,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 = fireUrlChange;
|
||||
self.$$checkUrlChange = fireStateOrUrlChange;
|
||||
|
||||
//////////////////////////////////////////////////////////////
|
||||
// Misc API
|
||||
|
||||
+13
-5
@@ -1039,11 +1039,21 @@ describe('$location', function() {
|
||||
});
|
||||
|
||||
it('should update $location when browser state changes', function() {
|
||||
initService({html5Mode:true, supportHistory: true});
|
||||
mockUpBrowser({initialUrl:'http://new.com/a/b/', baseHref:'/a/b/'});
|
||||
inject(function($location, $window) {
|
||||
initService({html5Mode: true, supportHistory: true});
|
||||
mockUpBrowser({initialUrl: 'http://new.com/a/b/', baseHref: '/a/b/'});
|
||||
inject(function($location, $rootScope, $window) {
|
||||
$window.history.pushState({b: 3});
|
||||
$rootScope.$digest();
|
||||
|
||||
expect($location.state()).toEqual({b: 3});
|
||||
|
||||
$window.history.pushState({b: 4}, null, $window.location.href + 'c?d=e#f');
|
||||
$rootScope.$digest();
|
||||
|
||||
expect($location.path()).toBe('/c');
|
||||
expect($location.search()).toEqual({d: 'e'});
|
||||
expect($location.hash()).toBe('f');
|
||||
expect($location.state()).toEqual({b: 4});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2666,12 +2676,10 @@ describe('$location', function() {
|
||||
replaceState: function(state, title, url) {
|
||||
win.history.state = copy(state);
|
||||
if (url) win.location.href = url;
|
||||
jqLite(win).triggerHandler('popstate');
|
||||
},
|
||||
pushState: function(state, title, url) {
|
||||
win.history.state = copy(state);
|
||||
if (url) win.location.href = url;
|
||||
jqLite(win).triggerHandler('popstate');
|
||||
}
|
||||
};
|
||||
win.addEventListener = angular.noop;
|
||||
|
||||
Reference in New Issue
Block a user