8000 fix(*): detect external changes in `history.state` · angular/angular.js@fef4ad2 · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit fef4ad2

Browse files
gkalpakgkalpak
committed
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.
1 parent a24777a commit fef4ad2

File tree

2 files changed

+22
-13
lines changed
< 8000 /span>

2 files changed

+22
-13
lines changed

‎src/ng/browser.js

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ function Browser(window, document, $log, $sniffer) {
9696
};
9797

9898
cacheState();
99-
lastHistoryState = cachedState;
10099

101100
/**
102101
* @name $browser#url
@@ -150,8 +149,6 @@ function Browser(window, document, $log, $sniffer) {
150149
if ($sniffer.history && (!sameBase || !sameState)) {
151150
history[replace ? 'replaceState' : 'pushState'](state, '', url);
152151
cacheState();
153-
// Do the assignment again so that those two variables are referentially identical.
154-
lastHistoryState = cachedState;
155152
} else {
156153
if (!sameBase) {
157154
pendingLocation = url;
@@ -200,8 +197,7 @@ function Browser(window, document, $log, $sniffer) {
200197

201198
function cacheStateAndFireUrlChange() {
202199
pendingLocation = null;
203-
cacheState();
204-
fireUrlChange();
200+
fireStateOrUrlChange();
205201
}
206202

207203
// This variable should be used *only* inside the cacheState function.
@@ -215,11 +211,16 @@ function Browser(window, document, $log, $sniffer) {
215211
if (equals(cachedState, lastCachedState)) {
216212
cachedState = lastCachedState;
217213
}
214+
218215
lastCachedState = cachedState;
216+
lastHistoryState = cachedState;
219217
}
220218

221-
function fireUrlChange() {
222-
if (lastBrowserUrl === self.url() && lastHistoryState === cachedState) {
219+
function fireStateOrUrlChange() {
220+
var prevLastHistoryState = lastHistoryState;
221+
cacheState();
222+
223+
if (lastBrowserUrl === self.url() && prevLastHistoryState === cachedState) {
223224
return;
224225
}
225226

@@ -285,7 +286,7 @@ function Browser(window, document, $log, $sniffer) {
285286
* Needs to be exported to be able to check for changes that have been done in sync,
286287
* as hashchange/popstate events fire in async.
287288
*/
288-
self.$$checkUrlChange = fireUrlChange;
289+
self.$$checkUrlChange = fireStateOrUrlChange;
289290

290291
//////////////////////////////////////////////////////////////
291292
// Misc API

‎test/ng/locationSpec.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,11 +1039,21 @@ describe('$location', function() {
10391039
});
10401040

10411041
it('should update $location when browser state changes', function() {
1042-
initService({html5Mode:true, supportHistory: true});
1043-
mockUpBrowser({initialUrl:'http://new.com/a/b/', baseHref:'/a/b/'});
1044-
inject(function($location, $window) {
1042+
initService({html5Mode: true, supportHistory: true});
1043+
mockUpBrowser({initialUrl: 'http://new.com/a/b/', baseHref: '/a/b/'});
1044+
inject(function($location, $rootScope, $window) {
10451045
$window.history.pushState({b: 3});
1046+
$rootScope.$digest();
1047+
10461048
expect($location.state()).toEqual({b: 3});
1049+
1050+
$window.history.pushState({b: 4}, null, $window.location.href + 'c?d=e#f');
1051+
$rootScope.$digest();
1052+
1053+
expect($location.path()).toBe('/c');
1054+
expect($location.search()).toEqual({d: 'e'});
1055+
expect($location.hash()).toBe('f');
1056+
expect($location.state()).toEqual({b: 4});
10471057
});
10481058
});
10491059

@@ -2666,12 +2676,10 @@ describe('$location', function() {
26662676
replaceState: function(state, title, url) {
26672677
win.history.state = copy(state);
26682678
if (url) win.location.href = url;
2669-
jqLite(win).triggerHandler('popstate');
26702679
},
26712680
pushState: function(state, title, url) {
26722681
win.history.state = copy(state);
26732682
if (url) win.location.href = url;
2674-
jqLite(win).triggerHandler('popstate');
26752683
}
26762684
};
26772685
win.addEventListener = angular.noop;

0 commit comments

Comments
 (0)
0