Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit f8a4232

Browse files
mgolmgol
committedOct 13, 2014
fix($browser): account for IE deserializing history.state on each read
IE 10-11+ deserialize history.state on every read, causing simple comparisons against history.state always return false. Account for that caching `history.state` on every hashchange or popstate event. Also: 1. Prevent firing onUrlChange callbacks twice if both popstate and hashchange event were fired. 2. Fix the issue of routes sometimes not firing the URL change in all browsers. Closes #9587 Fixes #9545
1 parent 874cac8 commit f8a4232

File tree

3 files changed

+230
-79
lines changed

3 files changed

+230
-79
lines changed
 

‎src/ng/browser.js

+35-10
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,14 @@ function Browser(window, document, $log, $sniffer) {
123123
// URL API
124124
//////////////////////////////////////////////////////////////
125125

126-
var lastBrowserUrl = location.href,
127-
lastHistoryState = history.state,
126+
var cachedState, lastHistoryState,
127+
lastBrowserUrl = location.href,
128128
baseElement = document.find('base'),
129129
reloadLocation = null;
130130

131+
cacheState();
132+
lastHistoryState = cachedState;
133+
131134
/**
132135
* @name $browser#url
133136
*
@@ -165,18 +168,20 @@ function Browser(window, document, $log, $sniffer) {
165168
// Don't change anything if previous and current URLs and states match. This also prevents
166169
// IE<10 from getting into redirect loop when in LocationHashbangInHtml5Url mode.
167170
// See https://github.com/angular/angular.js/commit/ffb2701
168-
if (lastBrowserUrl === url && (!$sniffer.history || history.state === state)) {
171+
if (lastBrowserUrl === url && (!$sniffer.history || cachedState === state)) {
169172
return;
170173
}
171174
var sameBase = lastBrowserUrl && stripHash(lastBrowserUrl) === stripHash(url);
172175
lastBrowserUrl = url;
176+
lastHistoryState = state;
173177
// Don't use history API if only the hash changed
174178
// due to a bug in IE10/IE11 which leads
175179
// to not firing a `hashchange` nor `popstate` event
176180
// in some cases (see #9143).
177-
if ($sniffer.history && (!sameBase || history.state !== state)) {
181+
if ($sniffer.history && (!sameBase || cachedState !== state)) {
178182
history[replace ? 'replaceState' : 'pushState'](state, '', url);
179-
lastHistoryState = history.state;
183+
cacheState();
184+
lastHistoryState = cachedState;
180185
} else {
181186
if (!sameBase) {
182187
reloadLocation = url;
@@ -208,20 +213,40 @@ function Browser(window, document, $log, $sniffer) {
208213
* @returns {object} state
209214
*/
210215
self.state = function() {
211-
return isUndefined(history.state) ? null : history.state;
216+
return cachedState;
212217
};
213218

214219
var urlChangeListeners = [],
215220
urlChangeInit = false;
216221

222+
function cacheStateAndFireUrlChange() {
223+
cacheState();
224+
fireUrlChange();
225+
}
226+
227+
// This variable should be used *only* inside the cacheState function.
228+
var lastCachedState = null;
229+
function cacheState() {
230+
// This should be the only place in $browser where `history.state` is read.
231+
cachedState = window.history.state;
232+
cachedState = isUndefined(cachedState) ? null : cachedState;
233+
234+
// Prevent callbacks fo fire twice if both hashchange & popstate were fired.
235+
if (equals(cachedState, lastCachedState)) {
236+
cachedState = lastCachedState;
237+
}
238+
lastCachedState = cachedState;
239+
}
240+
217241
function fireUrlChange() {
218-
if (lastBrowserUrl === self.url() && lastHistoryState === history.state) {
242+
if (lastBrowserUrl === self.url() && lastHistoryState === cachedState) {
219243
return;
220244
}
221245

222246
lastBrowserUrl = self.url();
247+
lastHistoryState = cachedState;
223248
forEach(urlChangeListeners, function(listener) {
224-
listener(self.url(), history.state);
249+
listener(self.url(), cachedState);
225250
});
226251
}
227252

@@ -254,9 +279,9 @@ function Browser(window, document, $log, $sniffer) {
254279
// changed by push/replaceState
255280

256281
// html5 history api - popstate event
257-
if ($sniffer.history) jqLite(window).on('popstate', fireUrlChange);
282+
if ($sniffer.history) jqLite(window).on('popstate', cacheStateAndFireUrlChange);
258283
// hashchange event
259-
jqLite(window).on('hashchange', fireUrlChange);
284+
jqLite(window).on('hashchange', cacheStateAndFireUrlChange);
260285

261286
urlChangeInit = true;
262287
}

‎src/ng/location.js

+7-4
Original file line numberDiff line numberDiff line change
@@ -850,9 +850,10 @@ function $LocationProvider(){
850850
var oldUrl = $browser.url();
851851
var oldState = $browser.state();
852852
var currentReplace = $location.$$replace;
853+
var urlOrStateChanged = oldUrl !== $location.absUrl() ||
854+
($location.$$html5 && $sniffer.history && oldState !== $location.$$state);
853855

854-
if (initializing || oldUrl !== $location.absUrl() ||
855-
($location.$$html5 && $sniffer.history && oldState !== $location.$$state)) {
856+
if (initializing || urlOrStateChanged) {
856857
initializing = false;
857858

858859
$rootScope.$evalAsync(function() {
@@ -861,8 +862,10 @@ function $LocationProvider(){
861862
$location.$$parse(oldUrl);
862863
$location.$$state = oldState;
863864
} else {
864-
setBrowserUrlWithFallback($location.absUrl(), currentReplace,
865-
oldState === $location.$$state ? null : $location.$$state);
865+
if (urlOrStateChanged) {
866+
setBrowserUrlWithFallback($location.absUrl(), currentReplace,
867+
oldState === $location.$$state ? null : $location.$$state);
868+
}
866869
afterLocationChange(oldUrl, oldState);
867870
}
868871
});

‎test/ng/browserSpecs.js

+188-65
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
var historyEntriesLength;
44
var sniffer = {};
55

6-
function MockWindow() {
6+
function MockWindow(options) {
7+
if (typeof options !== 'object') {
8+
options = {};
9+
}
710
var events = {};
811
var timeouts = this.timeouts = [];
912
var locationHref = 'http://server/';
1013
var mockWindow = this;
14+
var msie = options.msie;
15+
var ieState;
1116

1217
historyEntriesLength = 1;
1318

@@ -58,16 +63,32 @@ function MockWindow() {
5863
};
5964

6065
this.history = {
61-
state: null,
62-
pushState: function() {
66+
pushState: function () {
6367
this.replaceState.apply(this, arguments);
6468
historyEntriesLength++;
6569
},
66-
replaceState: function(state, title, url) {
70+
replaceState: function (state, title, url) {
6771
locationHref = url;
6872
mockWindow.history.state = copy(state);
6973
}
7074
};
75+
// IE 10-11 deserialize history.state on each read making subsequent reads
76+
// different object.
77+
if (!msie) {
78+
this.history.state = null;
79+
} else {
80+
ieState = null;
81+
Object.defineProperty(this.history, 'state', {
82+
get: function() {
83+
return copy(ieState);
84+
},
85+
set: function(value) {
86+
ieState = value;
87+
},
88+
configurable: true,
89+
enumerable: true,
90+
});
91+
}
7192
}
7293

7394
function MockDocument() {
@@ -95,7 +116,7 @@ function MockDocument() {
95116

96117
describe('browser', function() {
97118
/* global Browser: false */
98-
var browser, fakeWindow, fakeDocument, logs, scripts, removedScripts;
119+
var browser, fakeWindow, fakeDocument, fakeLog, logs, scripts, removedScripts;
99120

100121
beforeEach(function() {
101122
scripts = [];
@@ -114,29 +135,49 @@ describe('browser', function() {
114135
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
115136
});
116137

117-
describe('MockBrowser historyEntriesLength', function() {
118-
it('should increment historyEntriesLength when setting location.href', function() {
119-
expect(historyEntriesLength).toBe(1);
120-
fakeWindow.location.href = '/foo';
121-
expect(historyEntriesLength).toBe(2);
122-
});
138+
describe('MockBrowser', function() {
139+
describe('historyEntriesLength', function() {
140+
it('should increment historyEntriesLength when setting location.href', function() {
141+
expect(historyEntriesLength).toBe(1);
142+
fakeWindow.location.href = '/foo';
143+
expect(historyEntriesLength).toBe(2);
144+
});
145+
146+
it('should not increment historyEntriesLength when using location.replace', function() {
147+
expect(historyEntriesLength).toBe(1);
148+
fakeWindow.location.replace('/foo');
149+
expect(historyEntriesLength).toBe(1);
150+
});
123151

124-
it('should not increment historyEntriesLength when using location.replace', function() {
125-
expect(historyEntriesLength).toBe(1);
126-
fakeWindow.location.replace('/foo');
127-
expect(historyEntriesLength).toBe(1);
152+
it('should increment historyEntriesLength when using history.pushState', function() {
153+
expect(historyEntriesLength).toBe(1);
154+
fakeWindow.history.pushState({a: 2}, 'foo', '/bar');
155+
expect(historyEntriesLength).toBe(2);
156+
});
157+
158+
it('should not increment historyEntriesLength when using history.replaceState', function() {
159+
expect(historyEntriesLength).toBe(1);
160+
fakeWindow.history.replaceState({a: 2}, 'foo', '/bar');
161+
expect(historyEntriesLength).toBe(1);
162+
});
128163
});
129164

130-
it('should increment historyEntriesLength when using history.pushState', function() {
131-
expect(historyEntriesLength).toBe(1);
132-
fakeWindow.history.pushState({a: 2}, 'foo', '/bar');
133-
expect(historyEntriesLength).toBe(2);
165+
describe('in IE', function() {
166+
it('should return different state objects on every read', function() {
167+
fakeWindow = new MockWindow({msie: true});
168+
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
169+
170+
browser.url(fakeWindow.location.href, false, {prop: 'val'});
171+
expect(fakeWindow.history.state).not.toBe(fakeWindow.history.state);
172+
expect(fakeWindow.history.state).toEqual(fakeWindow.history.state);
173+
});
134174
});
135175

136-
it('should not increment historyEntriesLength when using history.replaceState', function() {
137-
expect(historyEntriesLength).toBe(1);
138-
fakeWindow.history.replaceState({a: 2}, 'foo', '/bar');
139-
expect(historyEntriesLength).toBe(1);
176+
describe('not in IE', function() {
177+
it('should return the same state object on every read', function() {
178+
browser.url(fakeWindow.location.href, false, {prop: 'val'});
179+
expect(fakeWindow.history.state).toBe(fakeWindow.history.state);
180+
});
140181
});
141182
});
142183

@@ -607,58 +648,112 @@ describe('browser', function() {
607648
currentHref = fakeWindow.location.href;
608649
});
609650

610-
it('should change state', function() {
611-
browser.url(currentHref + '/something', false, {prop: 'val1'});
612-
expect(fakeWindow.history.state).toEqual({prop: 'val1'});
613-
});
651+
describe('in IE', runTests({msie: true}));
652+
describe('not in IE', runTests({msie: false}));
614653

615-
it('should allow to set falsy states (except `undefined`)', function() {
616-
fakeWindow.history.state = {prop: 'val1'};
654+
function runTests(options) {
655+
return function() {
656+
beforeEach(function() {
657+
fakeWindow = new MockWindow({msie: options.msie});
658+
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
659+
});
617660

618-
browser.url(currentHref, false, null);
619-
expect(fakeWindow.history.state).toBe(null);
661+
it('should change state', function() {
662+
browser.url(currentHref, false, {prop: 'val1'});
663+
expect(fakeWindow.history.state).toEqual({prop: 'val1'});
664+
browser.url(currentHref + '/something', false, {prop: 'val2'});
665+
expect(fakeWindow.history.state).toEqual({prop: 'val2'});
666+
});
620667

621-
browser.url(currentHref, false, false);
622-
expect(fakeWindow.history.state).toBe(false);
668+
it('should allow to set falsy states (except `undefined`)', function() {
669+
browser.url(currentHref, false, {prop: 'val1'});
623670

624-
browser.url(currentHref, false, '');
625-
expect(fakeWindow.history.state).toBe('');
671+
browser.url(currentHref, false, null);
672+
expect(fakeWindow.history.state).toBe(null);
626673

627-
browser.url(currentHref, false, 0);
628-
expect(fakeWindow.history.state).toBe(0);
629-
});
674+
browser.url(currentHref, false, false);
675+
expect(fakeWindow.history.state).toBe(false);
630676

631-
it('should treat `undefined` state as `null`', function() {
632-
fakeWindow.history.state = {prop: 'val1'};
677+
browser.url(currentHref, false, '');
678+
expect(fakeWindow.history.state).toBe('');
633679

634-
browser.url(currentHref, false, undefined);
635-
expect(fakeWindow.history.state).toBe(null);
636-
});
680+
browser.url(currentHref, false, 0);
681+
expect(fakeWindow.history.state).toBe(0);
682+
});
637683

638-
it('should do pushState with the same URL and a different state', function() {
639-
browser.url(currentHref, false, {prop: 'val1'});
640-
expect(fakeWindow.history.state).toEqual({prop: 'val1'});
684+
it('should treat `undefined` state as `null`', function() {
685+
browser.url(currentHref, false, {prop: 'val1'});
641686

642-
browser.url(currentHref, false, null);
643-
expect(fakeWindow.history.state).toBe(null);
687+
browser.url(currentHref, false, undefined);
688+
expect(fakeWindow.history.state).toBe(null);
689+
});
644690

645-
browser.url(currentHref, false, {prop: 'val2'});
646-
browser.url(currentHref, false, {prop: 'val3'});
647-
expect(fakeWindow.history.state).toEqual({prop: 'val3'});
648-
});
691+
it('should do pushState with the same URL and a different state', function() {
692+
browser.url(currentHref, false, {prop: 'val1'});
693+
expect(fakeWindow.history.state).toEqual({prop: 'val1'});
649694

650-
it('should do pushState with the same URL and null state', function() {
651-
fakeWindow.history.state = {prop: 'val1'};
652-
browser.url(currentHref, false, null);
653-
expect(fakeWindow.history.state).toEqual(null);
654-
});
695+
browser.url(currentHref, false, null);
696+
expect(fakeWindow.history.state).toBe(null);
697+
698+
browser.url(currentHref, false, {prop: 'val2'});
699+
browser.url(currentHref, false, {prop: 'val3'});
700+
expect(fakeWindow.history.state).toEqual({prop: 'val3'});
701+
});
702+
703+
it('should do pushState with the same URL and null state', function() {
704+
browser.url(currentHref, false, {prop: 'val1'});
705+
browser.url(currentHref, false, null);
706+
expect(fakeWindow.history.state).toEqual(null);
707+
});
708+
709+
it('should do pushState with the same URL and the same non-null state', function() {
710+
browser.url(currentHref, false, {prop: 'val2'});
711+
fakeWindow.history.state = {prop: 'val3'};
712+
browser.url(currentHref, false, {prop: 'val2'});
713+
expect(fakeWindow.history.state).toEqual({prop: 'val2'});
714+
});
715+
};
716+
}
717+
});
655718

656-
it('should do pushState with the same URL and the same non-null state', function() {
657-
browser.url(currentHref, false, {prop: 'val2'});
658-
fakeWindow.history.state = {prop: 'val3'};
659-
browser.url(currentHref, false, {prop: 'val2'});
660-
expect(fakeWindow.history.state).toEqual({prop: 'val2'});
719+
describe('state', function() {
720+
var currentHref;
721+
722+
beforeEach(function() {
723+
sniffer = {history: true};
724+
currentHref = fakeWindow.location.href;
661725
});
726+
727+
describe('in IE', runTests({msie: true}));
728+
describe('not in IE', runTests({msie: false}));
729+
730+
function runTests(options) {
731+
return function() {
732+
beforeEach(function() {
733+
fakeWindow = new MockWindow({msie: options.msie});
734+
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
735+
});
736+
737+
it('should return history.state', function() {
738+
browser.url(currentHref, false, {prop: 'val'});
739+
expect(browser.state()).toEqual({prop: 'val'});
740+
browser.url(currentHref, false, 2);
741+
expect(browser.state()).toEqual(2);
742+
browser.url(currentHref, false, null);
743+
expect(browser.state()).toEqual(null);
744+
});
745+
746+
it('should return null if history.state is undefined', function() {
747+
browser.url(currentHref, false, undefined);
748+
expect(browser.state()).toBe(null);
749+
});
750+
751+
it('should return the same state object in subsequent invocations in IE', function() {
752+
browser.url(currentHref, false, {prop: 'val'});
753+
expect(browser.state()).toBe(browser.state());
754+
});
755+
};
756+
}
662757
});
663758

664759
describe('urlChange', function() {
@@ -723,6 +818,36 @@ describe('browser', function() {
723818
fakeWindow.fire('hashchange');
724819
expect(callback).not.toHaveBeenCalled();
725820
});
821+
822+
describe('state handling', function() {
823+
var currentHref;
824+
825+
beforeEach(function() {
826+
sniffer = {history: true};
827+
currentHref = fakeWindow.location.href;
828+
});
829+
830+
describe('in IE', runTests({msie: true}));
831+
describe('not in IE', runTests({msie: false}));
832+
833+
function runTests(options) {
834+
return function() {
835+
beforeEach(function() {
836+
fakeWindow = new MockWindow({msie: options.msie});
837+
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
838+
});
839+
840+
it('should fire onUrlChange listeners only once if both popstate and hashchange triggered', function() {
841+
fakeWindow.history.state = {prop: 'val'};
842+
browser.onUrlChange(callback);
843+
844+
fakeWindow.fire('hashchange');
845+
fakeWindow.fire('popstate');
846+
expect(callback).toHaveBeenCalledOnce();
847+
});
848+
};
849+
}
850+
});
726851
});
727852

728853

@@ -881,9 +1006,7 @@ describe('browser', function() {
8811006

8821007
// from $location for rewriting the initial url into a hash url
8831008
expect(browser.url).toHaveBeenCalledWith('http://server/#/some/deep/path', true);
884-
// from the initial call to the watch in $location for watching $location
885-
expect(browser.url).toHaveBeenCalledWith('http://server/#/some/deep/path', false, null);
886-
expect(changeUrlCount).toBe(2);
1009+
expect(changeUrlCount).toBe(1);
8871010
});
8881011

8891012
});

0 commit comments

Comments
 (0)
This repository has been archived.