Skip to content
This repository has been archived by the owner on Jul 15, 2019. It is now read-only.

Commit

Permalink
Fix issue with unicode url causing 2 state entries pushed on navigation
Browse files Browse the repository at this point in the history
  • Loading branch information
@lingyan
lingyan committed Sep 1, 2015
1 parent 5786680 commit f27482f
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 16 deletions.
26 changes: 24 additions & 2 deletions lib/History.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
/*global window */
'use strict';

var objectAssign = require('object-assign');

var EVENT_POPSTATE = 'popstate';

function isUndefined(v) {
Expand Down Expand Up @@ -64,6 +66,20 @@ History.prototype = {
* @return {String} The url string that denotes current route path and query
*/
getUrl: function () {
// Use origUrl in the history state object first. This is to fix the unicode
// url issue (for browsers supporting history state):
// For urls containing unicode chars, window.location will automatically encode
// these unicode chars. Therefore url comparison logic in handleHistory.js will
// break, because url in the currentNavigation of RouteStore is usually un-encoded.
// "origUrl" saved in the state object is in the same form as in RouteStore. So
// it is safer to use "origUrl" for comparison.
var state = this.getState();
var urlFromState = state && state.origUrl;
if (urlFromState) {
return urlFromState;
}

// fallback to what is the window.location
var location = this.win.location;
return location.pathname + location.search;
},
Expand All @@ -80,7 +96,10 @@ History.prototype = {
if (this._hasPushState) {
title = isUndefined(title) ? win.document.title : title;
url = isUndefined(url) ? win.location.href : url;
win.history.pushState(state, title, url);

// remember the original url in state, so that it can be used by getUrl()
var _state = objectAssign({origUrl: url}, state);
win.history.pushState(_state, title, url);
this.setTitle(title);
} else if (url) {
win.location.href = url;
Expand All @@ -99,7 +118,10 @@ History.prototype = {
if (this._hasPushState) {
title = isUndefined(title) ? win.document.title : title;
url = isUndefined(url) ? win.location.href : url;
win.history.replaceState(state, title, url);

// remember the original url in state, so that it can be used by getUrl()
var _state = objectAssign({origUrl: url}, state);
win.history.replaceState(_state, title, url);
this.setTitle(title);
} else if (url) {
win.location.replace(url);
Expand Down
52 changes: 38 additions & 14 deletions tests/unit/lib/History-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,23 @@ describe('History', function () {
var url = history.getUrl();
expect(url).to.equal('/path/to/page');
});
it ('has pushState, should use history.state.origUrl over location', function () {
var win = _.extend(windowMock.HTML5, {
history: {
state: {
origUrl: '/_url'
}
},
location: {
pathname: '/path/to/page',
search: '',
hash: '#/path/to/abc'
}
});
var history = new History({win: win});
var url = history.getUrl();
expect(url).to.equal('/_url');
});
it ('has pushState with query', function () {
var win = _.extend(windowMock.HTML5, {
location: {
Expand Down Expand Up @@ -158,26 +175,33 @@ describe('History', function () {
var history = new History({win: win});

history.pushState({foo: 'bar'});
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.pushState.title).to.equal('current title');
expect(testResult.pushState.url).to.equal('/currentUrl');

history.pushState({foo: 'bar'}, 't');
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.pushState.title).to.equal('t');
expect(testResult.pushState.url).to.equal('/currentUrl');

history.pushState({foo: 'bar'}, 't', '/url');
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/url", foo: 'bar'});
expect(testResult.pushState.title).to.equal('t');
expect(testResult.pushState.url).to.equal('/url');
expect(windowMock.HTML5.document.title).to.equal('t');

history.pushState({foo: 'bar'}, 'tt', '/url?a=b&x=y');
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/url?a=b&x=y", foo: 'bar'});
expect(testResult.pushState.title).to.equal('tt');
expect(testResult.pushState.url).to.equal('/url?a=b&x=y');
expect(windowMock.HTML5.document.title).to.equal('tt');

var unicodeUrl = '/post/128097060420/2015-fno-vogue全球購物夜-眾藝人名人共襄盛舉';
history.pushState({foo: 'bar'}, 'tt', unicodeUrl);
expect(testResult.pushState.state).to.eql({origUrl: unicodeUrl, foo: 'bar'});
expect(testResult.pushState.title).to.equal('tt');
expect(testResult.pushState.url).to.equal(unicodeUrl);
expect(windowMock.HTML5.document.title).to.equal('tt');
});
it ('has pushState, Firefox', function () {
var win = _.extend(windowMock.Firefox, {
Expand All @@ -191,17 +215,17 @@ describe('History', function () {
var history = new History({win: win});

history.pushState({foo: 'bar'});
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.pushState.title).to.equal('current title');
expect(testResult.pushState.url).to.equal('/currentUrl');

history.pushState({foo: 'bar'}, 't');
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.pushState.title).to.equal('t');
expect(testResult.pushState.url).to.equal('/currentUrl');

history.pushState({foo: 'bar'}, 't', '/url');
expect(testResult.pushState.state).to.eql({foo: 'bar'});
expect(testResult.pushState.state).to.eql({origUrl: "/url", foo: 'bar'});
expect(testResult.pushState.title).to.equal('t');
expect(testResult.pushState.url).to.equal('/url');
});
Expand Down Expand Up @@ -235,23 +259,23 @@ describe('History', function () {
var history = new History({win: win});

history.replaceState({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('current title');
expect(testResult.replaceState.url).to.equal('/currentUrl');

history.replaceState({foo: 'bar'}, 't');
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('t');
expect(testResult.replaceState.url).to.equal('/currentUrl');

history.replaceState({foo: 'bar'}, 't', '/url');
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/url", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('t');
expect(testResult.replaceState.url).to.equal('/url');
expect(windowMock.HTML5.document.title).to.equal('t');

history.replaceState({foo: 'bar'}, 'tt', '/url?a=b&x=y');
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/url?a=b&x=y", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('tt');
expect(testResult.replaceState.url).to.equal('/url?a=b&x=y', 'url has query');
expect(windowMock.HTML5.document.title).to.equal('tt');
Expand All @@ -268,17 +292,17 @@ describe('History', function () {
var history = new History({win: win});

history.replaceState({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('current title');
expect(testResult.replaceState.url).to.equal('/currentUrl');

history.replaceState({foo: 'bar'}, 't');
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/currentUrl", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('t');
expect(testResult.replaceState.url).to.equal('/currentUrl');

history.replaceState({foo: 'bar'}, 't', '/url');
expect(testResult.replaceState.state).to.eql({foo: 'bar'});
expect(testResult.replaceState.state).to.eql({origUrl: "/url", foo: 'bar'});
expect(testResult.replaceState.title).to.equal('t');
expect(testResult.replaceState.url).to.equal('/url');
});
Expand Down

0 comments on commit f27482f

Please sign in to comment.