Skip to content
Permalink
Browse files

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
  • Loading branch information
bedag-moo authored and petebacondarwin committed Apr 26, 2016
1 parent 9355aea commit 51cfb61e46c18a4aac27340120a276dda1112637
Showing with 43 additions and 1 deletion.
  1. +4 −1 src/ng/browser.js
  2. +39 −0 test/ng/browserSpecs.js
@@ -153,7 +153,7 @@ function Browser(window, document, $log, $sniffer) {
// Do the assignment again so that those two variables are referentially identical.
lastHistoryState = cachedState;
} else {
if (!sameBase || pendingLocation) {
if (!sameBase) {
pendingLocation = url;
}
if (replace) {
@@ -167,6 +167,9 @@ function Browser(window, document, $log, $sniffer) {
pendingLocation = url;
}
}
if (pendingLocation) {
pendingLocation = url;
}
return self;
// getter
} else {
@@ -92,7 +92,12 @@ function MockWindow(options) {
},
replaceState: function(state, title, url) {
locationHref = url;
if (!options.updateAsync) committedHref = locationHref;
mockWindow.history.state = copy(state);
if (!options.updateAsync) this.flushHref();
},
flushHref: function() {
committedHref = locationHref;
}
};
// IE 10-11 deserialize history.state on each read making subsequent reads
@@ -443,6 +448,40 @@ describe('browser', function() {

});

describe('url (with ie 11 weirdnesses)', function() {

it('url() should actually set the url, even if IE 11 is weird and replaces HTML entities in the URL', function() {
// this test can not be expressed with the Jasmine spies in the previous describe block, because $browser.url()
// needs to observe the change to location.href during its invocation to enter the failing code path, but the spies
// are not callThrough

sniffer.history = true;
var originalReplace = fakeWindow.location.replace;
fakeWindow.location.replace = function(url) {
url = url.replace('&not', '¬');
// I really don't know why IE 11 (sometimes) does this, but I am not the only one to notice:
// https://connect.microsoft.com/IE/feedback/details/1040980/bug-in-ie-which-interprets-document-location-href-as-html
originalReplace.call(this, url);
};

// the initial URL contains a lengthy oauth token in the hash
var initialUrl = 'http://test.com/oauthcallback#state=xxx%3D&not-before-policy=0';
fakeWindow.location.href = initialUrl;
browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);

// somehow, $location gets a version of this url where the = is no longer escaped, and tells the browser:
var initialUrlFixedByLocation = initialUrl.replace('%3D', '=');
browser.url(initialUrlFixedByLocation, true, null);
expect(browser.url()).toEqual(initialUrlFixedByLocation);

// a little later (but in the same digest cycle) the view asks $location to replace the url, which tells $browser
var secondUrl = 'http://test.com/otherView';
browser.url(secondUrl, true, null);
expect(browser.url()).toEqual(secondUrl);
});

});

describe('url (when state passed)', function() {
var currentHref, pushState, replaceState, locationReplace;

0 comments on commit 51cfb61

Please sign in to comment.
You can’t perform that action at this time.