From bfad456012bc5a16088929cdc268e754fe960802 Mon Sep 17 00:00:00 2001 From: Raphael Luba Date: Fri, 1 Jun 2018 17:06:19 +0200 Subject: [PATCH 1/4] Fix: History did not handle absolute URLs correctly when a root was declared --- src/index.js | 31 ++++++++++++++++--------------- test/history.spec.js | 27 +++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/index.js b/src/index.js index 9ce1ef3..d3b1998 100644 --- a/src/index.js +++ b/src/index.js @@ -119,13 +119,13 @@ export class BrowserHistory extends History { /** * Causes a history navigation to occur. * - * @param fragment The history fragment to navigate to. + * @param url URL to navigate to. * @param options The set of options that specify how the navigation should occur. * @return Promise if triggering navigation, otherwise true/false indicating if navigation occured. */ - navigate(fragment?: string, {trigger = true, replace = false} = {}): Promise|boolean { - if (fragment && absoluteUrl.test(fragment)) { - this.location.href = fragment; + navigate(url?: string, {trigger = true, replace = false} = {}): Promise|boolean { + if (url && absoluteUrl.test(url)) { + this.location.href = url; return true; } @@ -133,7 +133,7 @@ export class BrowserHistory extends History { return false; } - fragment = this._getFragment(fragment || ''); + const fragment = this._getFragment(url || ''); if (this.fragment === fragment && !replace) { return false; @@ -141,14 +141,14 @@ export class BrowserHistory extends History { this.fragment = fragment; - let url = this.root + fragment; + url = this.root + fragment; // Don't include a trailing slash on the root. if (fragment === '' && url !== '/') { url = url.slice(0, -1); } - // If pushState is available, we use it to set the fragment as a real URL. + // If pushState is available, we use it to set the url as a real URL. if (this._hasPushState) { url = url.replace('//', '/'); this.history[replace ? 'replaceState' : 'pushState']({}, DOM.title, url); @@ -163,7 +163,7 @@ export class BrowserHistory extends History { } if (trigger) { - return this._loadUrl(fragment); + return this._loadUrl(url); } return true; @@ -209,20 +209,21 @@ export class BrowserHistory extends History { return this.location.hash.substr(1); } - _getFragment(fragment: string, forcePushState?: boolean): string { - let root; - + _getFragment(url: string, forcePushState?: boolean): string { + let fragment = url; if (!fragment) { if (this._hasPushState || !this._wantsHashChange || forcePushState) { fragment = this.location.pathname + this.location.search; - root = this.root.replace(trailingSlash, ''); - if (!fragment.indexOf(root)) { - fragment = fragment.substr(root.length); - } } else { fragment = this._getHash(); } } + if (this._hasPushState || !this._wantsHashChange || forcePushState) { + const root = this.root.replace(trailingSlash, ''); + if (fragment.indexOf(root) === 0) { + fragment = fragment.substr(root.length); + } + } return '/' + fragment.replace(routeStripper, ''); } diff --git a/test/history.spec.js b/test/history.spec.js index c3f826f..1ab8da5 100644 --- a/test/history.spec.js +++ b/test/history.spec.js @@ -10,16 +10,39 @@ describe('browser history', () => { describe('_getFragment()', () => { - it('should normalize fragment', () => { + it('should normalize fragment from URL', () => { var expected = '/admin/user/123'; - var bh = new BrowserHistory(); + var bh = new BrowserHistory(new LinkHandler()); + bh.activate({}); + + expect(bh._getFragment('admin/user/123')).toBe(expected); + expect(bh._getFragment('admin/user/123 ')).toBe(expected); + expect(bh._getFragment('/admin/user/123')).toBe(expected); + expect(bh._getFragment('/admin/user/123 ')).toBe(expected); + expect(bh._getFragment('///admin/user/123')).toBe(expected); + + expect(bh._getFragment('#admin/user/123')).toBe(expected); + expect(bh._getFragment('#admin/user/123 ')).toBe(expected); + expect(bh._getFragment('#/admin/user/123')).toBe(expected); + expect(bh._getFragment('#/admin/user/123 ')).toBe(expected); + expect(bh._getFragment('#///admin/user/123')).toBe(expected); + }); + + it('should normalize fragment when a root is configured', () => { + const expected = '/admin/user/123'; + const bh = new BrowserHistory(new LinkHandler()); + bh.activate({root: '/root'}); + bh._hasPushState = true; expect(bh._getFragment('admin/user/123')).toBe(expected); expect(bh._getFragment('admin/user/123 ')).toBe(expected); expect(bh._getFragment('/admin/user/123')).toBe(expected); expect(bh._getFragment('/admin/user/123 ')).toBe(expected); expect(bh._getFragment('///admin/user/123')).toBe(expected); + expect(bh._getFragment('/root/admin/user/123')).toBe(expected); + expect(bh._getFragment('/root///admin/user/123')).toBe(expected); + bh._hasPushState = false; expect(bh._getFragment('#admin/user/123')).toBe(expected); expect(bh._getFragment('#admin/user/123 ')).toBe(expected); expect(bh._getFragment('#/admin/user/123')).toBe(expected); From 407959e9c9e53eac3152aae8b1cafec50c507064 Mon Sep 17 00:00:00 2001 From: Raphael Luba Date: Fri, 1 Jun 2018 17:14:03 +0200 Subject: [PATCH 2/4] Fix: History did not handle outbound absolute paths correctly --- src/index.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index d3b1998..102ddad 100644 --- a/src/index.js +++ b/src/index.js @@ -124,9 +124,19 @@ export class BrowserHistory extends History { * @return Promise if triggering navigation, otherwise true/false indicating if navigation occured. */ navigate(url?: string, {trigger = true, replace = false} = {}): Promise|boolean { - if (url && absoluteUrl.test(url)) { - this.location.href = url; - return true; + if (url) { + let isOutbound = false; + if (absoluteUrl.test(url)) { + isOutbound = true; + } else if (this._hasPushState && url.indexOf('/') === 0 && url.indexOf(this.root) !== 0) { + // Absolute path with a different root + isOutbound = true; + } + + if (isOutbound) { + this.location.href = url; + return true; + } } if (!this._isActive) { From d5a84b36c651a93512492504955274b89466fd53 Mon Sep 17 00:00:00 2001 From: Raphael Luba Date: Tue, 5 Jun 2018 20:48:37 +0200 Subject: [PATCH 3/4] Fix: Added root to urls when navigating in in non-pushstate mode --- src/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 102ddad..d86fb18 100644 --- a/src/index.js +++ b/src/index.js @@ -151,7 +151,10 @@ export class BrowserHistory extends History { this.fragment = fragment; - url = this.root + fragment; + url = fragment; + if (this.pushState) { + url = this.root + url; + } // Don't include a trailing slash on the root. if (fragment === '' && url !== '/') { From fed52d3cc5ead37a0c0e3981bd0236f35d52a824 Mon Sep 17 00:00:00 2001 From: Raphael Luba Date: Tue, 5 Jun 2018 21:13:11 +0200 Subject: [PATCH 4/4] Fix typo. --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index d86fb18..91c1c9f 100644 --- a/src/index.js +++ b/src/index.js @@ -152,7 +152,7 @@ export class BrowserHistory extends History { this.fragment = fragment; url = fragment; - if (this.pushState) { + if (this._hasPushState) { url = this.root + url; }