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

refactor($browser): more test coverage around history.state manipulation #9597

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/ng/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,12 @@ function Browser(window, document, $log, $sniffer) {

// setter
if (url) {
var sameState = lastHistoryState === state;

// Don't change anything if previous and current URLs and states match. This also prevents
// IE<10 from getting into redirect loop when in LocationHashbangInHtml5Url mode.
// See https://github.com/angular/angular.js/commit/ffb2701
if (lastBrowserUrl === url && (!$sniffer.history || cachedState === state)) {
if (lastBrowserUrl === url && (!$sniffer.history || sameState)) {
return;
}
var sameBase = lastBrowserUrl && stripHash(lastBrowserUrl) === stripHash(url);
Expand All @@ -178,9 +180,10 @@ function Browser(window, document, $log, $sniffer) {
// due to a bug in IE10/IE11 which leads
// to not firing a `hashchange` nor `popstate` event
// in some cases (see #9143).
if ($sniffer.history && (!sameBase || cachedState !== state)) {
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) {
Expand Down
36 changes: 24 additions & 12 deletions test/ng/browserSpecs.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,11 +641,9 @@ describe('browser', function() {
});

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

beforeEach(function() {
sniffer = {history: true};
currentHref = fakeWindow.location.href;
});

describe('in IE', runTests({msie: true}));
Expand All @@ -654,7 +652,14 @@ describe('browser', function() {
function runTests(options) {
return function() {
beforeEach(function() {
sniffer = {history: true};

fakeWindow = new MockWindow({msie: options.msie});
currentHref = fakeWindow.location.href;
pushState = spyOn(fakeWindow.history, 'pushState').andCallThrough();
replaceState = spyOn(fakeWindow.history, 'replaceState').andCallThrough();
locationReplace = spyOn(fakeWindow.location, 'replace').andCallThrough();

browser = new Browser(fakeWindow, fakeDocument, fakeLog, sniffer);
browser.onUrlChange(function() {});
});
Expand Down Expand Up @@ -703,20 +708,27 @@ describe('browser', function() {
expect(fakeWindow.history.state).toEqual({prop: 'val3'});
});

it('should do pushState with the same URL and null state', function() {
fakeWindow.history.state = {prop: 'val1'};
it('should do pushState with the same URL and deep equal but referentially different state', function() {
fakeWindow.history.state = {prop: 'val'};
fakeWindow.fire('popstate');
expect(historyEntriesLength).toBe(1);

browser.url(currentHref, false, null);
expect(fakeWindow.history.state).toEqual(null);
browser.url(currentHref, false, {prop: 'val'});
expect(fakeWindow.history.state).toEqual({prop: 'val'});
expect(historyEntriesLength).toBe(2);
});

it('should do pushState with the same URL and the same non-null state', function() {
fakeWindow.history.state = null;
fakeWindow.fire('popstate');
it('should not do pushState with the same URL and state from $browser.state()', function() {
browser.url(currentHref, false, {prop: 'val'});

browser.url(currentHref, false, {prop: 'val2'});
expect(fakeWindow.history.state).toEqual({prop: 'val2'});
pushState.reset();
replaceState.reset();
locationReplace.reset();

browser.url(currentHref, false, browser.state());
expect(pushState).not.toHaveBeenCalled();
expect(replaceState).not.toHaveBeenCalled();
expect(locationReplace).not.toHaveBeenCalled();
});
};
}
Expand Down