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

Commit 743b33a

Browse files
bedag-moopetebacondarwin
authored andcommitted
fix($brower): set the url even if the browser transforms it
While $location expects that $browser stores the URL unchanged, "some browsers" transform the URL when setting or defer the acutal update. To work around this, $browser.url() kept the unchanged URL in pendingLocation. However, it failed to update pendingLocation in all code paths, causing $browser.url() to sometimes incorrectly report previous URLs, which horribly confused $location. This fix ensures that pendingLocation is always updated if set, causing url() to report the current url. Fixes #14427 Closes #14499
1 parent 9f8fa79 commit 743b33a

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

src/ng/browser.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ function Browser(window, document, $log, $sniffer) {
153153
// Do the assignment again so that those two variables are referentially identical.
154154
lastHistoryState = cachedState;
155155
} else {
156-
if (!sameBase || pendingLocation) {
156+
if (!sameBase) {
157157
pendingLocation = url;
158158
}
159159
if (replace) {
@@ -167,6 +167,9 @@ function Browser(window, document, $log, $sniffer) {
167167
pendingLocation = url;
168168
}
169169
}
170+
if (pendingLocation) {
171+
pendingLocation = url;
172+
}
170173
return self;
171174
// getter
172175
} else {

test/ng/browserSpecs.js

+39
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,12 @@ function MockWindow(options) {
9292
},
9393
replaceState: function(state, title, url) {
9494
locationHref = url;
95+
if (!options.updateAsync) committedHref = locationHref;
9596
mockWindow.history.state = copy(state);
97+
if (!options.updateAsync) this.flushHref();
98+
},
99+
flushHref: function() {
100+
committedHref = locationHref;
96101
}
97102
};
98103
// IE 10-11 deserialize history.state on each read making subsequent reads
@@ -443,6 +448,40 @@ describe('browser', function() {
443448

444449
});
445450

451+
describe('url (with ie 11 weirdnesses)', function() {
452+
453+
it('url() should actually set the url, even if IE 11 is weird and replaces HTML entities in the URL', function() {
454+
// this test can not be expressed with the Jasmine spies in the previous describe block, because $browser.url()
455+
// needs to observe the change to location.href during its invocation to enter the failing code path, but the spies
456+
// are not callThrough
457+
458+
sniffer.history = true;
459+
var originalReplace = fakeWindow.location.replace;
460+
fakeWindow.location.replace = function(url) {
461+
url = url.replace('&not', '¬');
462+
// I really don't know why IE 11 (sometimes) does this, but I am not the only one to notice:
463+
// https://connect.microsoft.com/IE/feedback/details/1040980/bug-in-ie-which-interprets-document-location-href-as-html
464+
originalReplace.call(this, url);
465+
};
466+
467+
// the initial URL contains a lengthy oauth token in the hash
468+
var initialUrl = 'http://test.com/oauthcallback#state=xxx%3D&not-before-policy=0';
469+
fakeWindow.location.href = initialUrl;
470+
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
471+
472+
// somehow, $location gets a version of this url where the = is no longer escaped, and tells the browser:
473+
var initialUrlFixedByLocation = initialUrl.replace('%3D', '=');
474+
browser.url(initialUrlFixedByLocation, true, null);
475+
expect(browser.url()).toEqual(initialUrlFixedByLocation);
476+
477+
// a little later (but in the same digest cycle) the view asks $location to replace the url, which tells $browser
478+
var secondUrl = 'http://test.com/otherView';
479+
browser.url(secondUrl, true, null);
480+
expect(browser.url()).toEqual(secondUrl);
481+
});
482+
483+
});
484+
446485
describe('url (when state passed)', function() {
447486
var currentHref, pushState, replaceState, locationReplace;
448487

0 commit comments

Comments
 (0)