fix(vanilla): fix base path handling for vanilla push state

christopherthielen · christopherthielen · commit ad61d74e880f · 2017-09-23T13:13:57.000-07:00
This fixes the vanilla PushState location service handling of `base` tags. This change attempts to match the behavior of https://html.spec.whatwg.org/dev/semantics.html#the-base-element If a base tag is '/base/index.html' and a state with a URL of '/foo' is activated, the URL will be '/base/foo'. If the url exactly matches the base tag, it will route to the state matching '/'. This also addresses a longstanding bug where base tags which didn't end in a slash (such as '/base') had their last character stripped (i.e., '/bas'), and therefore didn't work. Now base tags like that should work as described above. Closes #54 Closes angular-ui/ui-router#2357
diff --git a/karma.conf.js b/karma.conf.js
@@ -26,11 +26,12 @@ module.exports = function (karma) {
     frameworks: ['jasmine'],
 
     plugins: [
-      require('karma-webpack'),
-      require('karma-sourcemap-loader'),
+      require('karma-chrome-launcher'),
+      require('karma-firefox-launcher'),
       require('karma-jasmine'),
       require('karma-phantomjs-launcher'),
-      require('karma-chrome-launcher')
+      require('karma-sourcemap-loader'),
+      require('karma-webpack'),
     ],
 
     webpack: {
diff --git a/package.json b/package.json
@@ -74,6 +74,7 @@
     "karma": "^1.2.0",
     "karma-chrome-launcher": "~0.1.0",
     "karma-coverage": "^0.5.3",
+    "karma-firefox-launcher": "^1.0.1",
     "karma-jasmine": "^1.0.2",
     "karma-phantomjs-launcher": "^1.0.4",
     "karma-script-launcher": "~0.1.0",
diff --git a/src/common/strings.ts b/src/common/strings.ts
@@ -111,13 +111,20 @@ export function stringify(o: any) {
 }
 
 /** Returns a function that splits a string on a character or substring */
-export const beforeAfterSubstr = (char: string) => (str: string) => {
+export const beforeAfterSubstr = (char: string) => (str: string): string[] => {
   if (!str) return ["", ""];
   let idx = str.indexOf(char);
   if (idx === -1) return [str, ""];
   return [str.substr(0, idx), str.substr(idx + 1)];
 };
 
+export const hostRegex = new RegExp('^(?:[a-z]+:)?//[^/]+/');
+export const stripFile = (str: string) => str.replace(/\/[^/]*$/, '');
+export const splitHash = beforeAfterSubstr("#");
+export const splitQuery = beforeAfterSubstr("?");
+export const splitEqual = beforeAfterSubstr("=");
+export const trimHashVal = (str: string) => str ? str.replace(/^#/, "") : "";
+
 /**
  * Splits on a delimiter, but returns the delimiters in the array
  *
diff --git a/src/url/urlRouter.ts b/src/url/urlRouter.ts
@@ -14,11 +14,12 @@ import { UrlRuleFactory } from './urlRule';
 import { TargetState } from '../state/targetState';
 import { MatcherUrlRule, MatchResult, UrlParts, UrlRule, UrlRuleHandlerFn, UrlRuleMatchFn, UrlRulesApi, UrlSyncApi, } from './interface';
 import { TargetStateDef } from '../state/interface';
+import { stripFile } from '../common';
 
 /** @hidden */
 function appendBasePath(url: string, isHtml5: boolean, absolute: boolean, baseHref: string): string {
   if (baseHref === '/') return url;
-  if (isHtml5) return baseHref.slice(0, -1) + url;
+  if (isHtml5) return stripFile(baseHref) + url;
   if (absolute) return baseHref.slice(1) + url;
   return url;
 }
diff --git a/src/vanilla/browserLocationConfig.ts b/src/vanilla/browserLocationConfig.ts
@@ -39,12 +39,13 @@ export class BrowserLocationConfig implements LocationConfig {
   };
 
   baseHref(href?: string): string {
-    return isDefined(href) ? this._baseHref = href : this._baseHref || this.applyDocumentBaseHref();
+    return isDefined(href) ? this._baseHref = href :
+      isDefined(this._baseHref) ? this._baseHref : this.applyDocumentBaseHref();
   }
 
   applyDocumentBaseHref() {
-    let baseTags = document.getElementsByTagName("base");
-    return this._baseHref = baseTags.length ? baseTags[0].href.substr(location.origin.length) : "";
+    let baseTag: HTMLBaseElement = document.getElementsByTagName("base")[0];
+    return this._baseHref = baseTag ? baseTag.href.substr(location.origin.length) : "";
   }
 
   dispose() {}
diff --git a/src/vanilla/hashLocationService.ts b/src/vanilla/hashLocationService.ts
@@ -3,7 +3,7 @@
  * @module vanilla
  */
 /** */
-import { trimHashVal } from "./utils";
+import { trimHashVal } from "../common/strings";
 import { UIRouter } from "../router";
 import { BaseLocationServices } from "./baseLocationService";
 
diff --git a/src/vanilla/pushStateLocationService.ts b/src/vanilla/pushStateLocationService.ts
@@ -4,9 +4,9 @@
  */
 /** */
 import { LocationConfig } from "../common/coreservices";
-import { splitQuery, splitHash } from "./utils";
 import { UIRouter } from "../router";
 import { BaseLocationServices } from "./baseLocationService";
+import { splitQuery, splitHash, stripFile } from "../common/strings";
 
 /**
  * A `LocationServices` that gets/sets the current location using the browser's `location` and `history` apis
@@ -22,21 +22,41 @@ export class PushStateLocationService extends BaseLocationServices {
     self.addEventListener("popstate", this._listener, false);
   };
 
+  /**
+   * Gets the base prefix without:
+   * - trailing slash
+   * - trailing filename
+   * - protocol and hostname
+   *
+   * If <base href='/base/index.html'>, this returns '/base'.
+   * If <base href='http://localhost:8080/base/index.html'>, this returns '/base'.
+   *
+   * See: https://html.spec.whatwg.org/dev/semantics.html#the-base-element
+   */
+  _getBasePrefix() {
+    return stripFile(this._config.baseHref());
+  }
+
   _get() {
     let { pathname, hash, search } = this._location;
     search = splitQuery(search)[1]; // strip ? if found
     hash = splitHash(hash)[1]; // strip # if found
-    return pathname + (search ? "?" + search : "") + (hash ? "$" + search : "");
+
+    const basePrefix = this._getBasePrefix();
+    let exactMatch = pathname === this._config.baseHref();
+    let startsWith = pathname.startsWith(basePrefix);
+    pathname = exactMatch ? '/' : startsWith ? pathname.substring(basePrefix.length) : pathname;
+
+    return pathname + (search ? "?" + search : "") + (hash ? "#" + hash : "");
   }
 
   _set(state: any, title: string, url: string, replace: boolean) {
-    let { _config, _history } = this;
-    let fullUrl = _config.baseHref() + url;
+    let fullUrl = this._getBasePrefix() + url;
 
     if (replace) {
-      _history.replaceState(state, title, fullUrl);
+      this._history.replaceState(state, title, fullUrl);
     } else {
-      _history.pushState(state, title, fullUrl);
+      this._history.pushState(state, title, fullUrl);
     }
   }
 
diff --git a/src/vanilla/utils.ts b/src/vanilla/utils.ts
@@ -3,25 +3,10 @@
  * @module vanilla
  */
 /** */
-import { isArray } from "../common/index";
-import { LocationServices, LocationConfig } from "../common/coreservices";
+import {
+  LocationConfig, LocationServices, identity, unnestR, isArray, splitEqual, splitHash, splitQuery
+} from "../common";
 import { UIRouter } from "../router";
-import { identity, unnestR, removeFrom, deregAll, extend } from "../common/common";
-import { LocationLike, HistoryLike } from "./interface";
-import { isDefined } from "../common/predicates";
-import { Disposable } from "../interface";
-
-const beforeAfterSubstr = (char: string) => (str: string): string[] => {
-  if (!str) return ["", ""];
-  let idx = str.indexOf(char);
-  if (idx === -1) return [str, ""];
-  return [str.substr(0, idx), str.substr(idx + 1)];
-};
-
-export const splitHash = beforeAfterSubstr("#");
-export const splitQuery = beforeAfterSubstr("?");
-export const splitEqual = beforeAfterSubstr("=");
-export const trimHashVal = (str) => str ? str.replace(/^#/, "") : "";
 
 export const keyValsToObjectR = (accum, [key, val]) => {
   if (!accum.hasOwnProperty(key)) {
diff --git a/test/_testUtils.ts b/test/_testUtils.ts
@@ -3,6 +3,10 @@ import { map } from "../src/common/common";
 
 let stateProps = ["resolve", "resolvePolicy", "data", "template", "templateUrl", "url", "name", "params"];
 
+const initialUrl = document.location.href;
+export const resetBrowserUrl = () =>
+  history.replaceState(null, null, initialUrl);
+
 export const delay = (ms) =>
     new Promise<any>(resolve => setTimeout(resolve, ms));
 export const _delay = (ms) => () => delay(ms);
diff --git a/test/index.js b/test/index.js
@@ -4,6 +4,9 @@
 require('core-js');
 require('../src/index');
 require('./_matchers');
+var utils = require('./_testUtils');
 
 var testsContext = require.context(".", true, /Spec$/);
 testsContext.keys().forEach(testsContext);
+
+afterAll(utils.resetBrowserUrl);
diff --git a/test/lazyLoadSpec.ts b/test/lazyLoadSpec.ts
@@ -6,6 +6,7 @@ import { UrlRouter } from "../src/url/urlRouter";
 import {StateDeclaration} from "../src/state/interface";
 import {tail} from "../src/common/common";
 import {Transition} from "../src/transition/transition";
+import { TestingPlugin } from './_testingPlugin';
 
 describe('future state', function () {
   let router: UIRouter;
@@ -16,8 +17,7 @@ describe('future state', function () {
 
   beforeEach(() => {
     router = new UIRouter();
-    router.plugin(vanilla.servicesPlugin);
-    router.plugin(vanilla.hashLocationPlugin);
+    router.plugin(TestingPlugin);
     $registry = router.stateRegistry;
     $state = router.stateService;
     $transitions = router.transitionService;
diff --git a/test/urlRouterSpec.ts b/test/urlRouterSpec.ts
@@ -5,6 +5,7 @@ import { UrlService } from "../src/url/urlService";
 import { StateRegistry } from "../src/state/stateRegistry";
 import { noop } from "../src/common/common";
 import { UrlRule, MatchResult } from "../src/url/interface";
+import { pushStateLocationPlugin } from '../src/vanilla';
 
 declare let jasmine;
 let _anything = jasmine.anything();
@@ -240,6 +241,134 @@ describe("UrlRouter", function () {
       let actual = urlRouter.href(matcher('/hello/:name'), { name: 'world', '#': 'frag' }, { absolute: true });
       expect(actual).toBe('http://localhost/#/hello/world#frag');
     });
+
+    describe('in html5mode', () => {
+      let baseTag: HTMLBaseElement;
+      const applyBaseTag = (href: string) => {
+        baseTag = document.createElement('base');
+        baseTag.href = href;
+        document.head.appendChild(baseTag);
+      };
+
+      afterEach(() => baseTag.parentElement.removeChild(baseTag));
+
+      beforeEach(() => {
+        router.dispose(router.getPlugin('vanilla.memoryLocation'));
+        router.plugin(pushStateLocationPlugin);
+        router.urlService = new UrlService(router, false);
+      });
+
+      describe('with base="/base/"', () => {
+        beforeEach(() => applyBaseTag('/base/'));
+
+        it("should prefix the href with /base/", function () {
+          expect(urlRouter.href(matcher("/foo"))).toBe('/base/foo');
+        });
+
+        it('should include #fragments', function () {
+          expect(urlRouter.href(matcher("/foo"), { '#': "hello"})).toBe('/base/foo#hello');
+        });
+
+        it('should return absolute URLs', function () {
+          // don't use urlService var
+          const cfg = router.urlService.config;
+          const href = urlRouter.href(matcher('/hello/:name'), { name: 'world', '#': 'frag' }, { absolute: true });
+          const prot = cfg.protocol();
+          const host = cfg.host();
+          const port = cfg.port();
+          let portStr = (port === 80 || port === 443) ? '' : `:${port}`;
+          expect(href).toBe(`${prot}://${host}${portStr}/base/hello/world#frag`);
+        });
+      });
+
+      describe('with base="/base/index.html"', () => {
+        beforeEach(() => applyBaseTag('/base/index.html'));
+
+        it("should prefix the href with /base/ but not with index.html", function () {
+          expect(urlRouter.href(matcher("/foo"))).toBe('/base/foo');
+        });
+
+        it('should include #fragments', function () {
+          expect(urlRouter.href(matcher("/foo"), { '#': "hello"})).toBe('/base/foo#hello');
+        });
+
+        it('should return absolute URLs', function () {
+          // don't use urlService var
+          const cfg = router.urlService.config;
+          const href = urlRouter.href(matcher('/hello/:name'), { name: 'world', '#': 'frag' }, { absolute: true });
+          const prot = cfg.protocol();
+          const host = cfg.host();
+          const port = cfg.port();
+          let portStr = (port === 80 || port === 443) ? '' : `:${port}`;
+          expect(href).toBe(`${prot}://${host}${portStr}/base/hello/world#frag`);
+        });
+      });
+
+      describe('with base="http://localhost:8080/base/"', () => {
+        beforeEach(() => applyBaseTag('http://localhost:8080/base/'));
+
+        it("should prefix the href with /base/", function () {
+          expect(urlRouter.href(matcher("/foo"))).toBe('/base/foo');
+        });
+
+        it('should include #fragments', function () {
+          expect(urlRouter.href(matcher("/foo"), { '#': "hello"})).toBe('/base/foo#hello');
+        });
+
+        it('should return absolute URLs', function () {
+          // don't use urlService var
+          const cfg = router.urlService.config;
+          const href = urlRouter.href(matcher('/hello/:name'), { name: 'world', '#': 'frag' }, { absolute: true });
+          const prot = cfg.protocol();
+          const host = cfg.host();
+          const port = cfg.port();
+          let portStr = (port === 80 || port === 443) ? '' : `:${port}`;
+          expect(href).toBe(`${prot}://${host}${portStr}/base/hello/world#frag`);
+        });
+      });
+
+      describe('with base="http://localhost:8080/base"', () => {
+        beforeEach(() => applyBaseTag('http://localhost:8080/base'));
+
+        it("should not prefix the href with /base", function () {
+          expect(urlRouter.href(matcher("/foo"))).toBe('/foo');
+        });
+
+        it('should return absolute URLs', function () {
+          // don't use urlService var
+          const cfg = router.urlService.config;
+          const href = urlRouter.href(matcher('/hello/:name'), { name: 'world', '#': 'frag' }, { absolute: true });
+          const prot = cfg.protocol();
+          const host = cfg.host();
+          const port = cfg.port();
+          let portStr = (port === 80 || port === 443) ? '' : `:${port}`;
+          expect(href).toBe(`${prot}://${host}${portStr}/hello/world#frag`);
+        });
+      });
+
+      describe('with base="http://localhost:8080/base/index.html"', () => {
+        beforeEach(() => applyBaseTag('http://localhost:8080/base/index.html'));
+
+        it("should prefix the href with /base/", function () {
+          expect(urlRouter.href(matcher("/foo"))).toBe('/base/foo');
+        });
+
+        it('should include #fragments', function () {
+          expect(urlRouter.href(matcher("/foo"), { '#': "hello"})).toBe('/base/foo#hello');
+        });
+
+        it('should return absolute URLs', function () {
+          // don't use urlService var
+          const cfg = router.urlService.config;
+          const href = urlRouter.href(matcher('/hello/:name'), { name: 'world', '#': 'frag' }, { absolute: true });
+          const prot = cfg.protocol();
+          const host = cfg.host();
+          const port = cfg.port();
+          let portStr = (port === 80 || port === 443) ? '' : `:${port}`;
+          expect(href).toBe(`${prot}://${host}${portStr}/base/hello/world#frag`);
+        });
+      });
+    });
   });
 
   describe('Url Rule priority', () => {
diff --git a/test/vanilla.browserLocationConfigSpec.ts b/test/vanilla.browserLocationConfigSpec.ts
@@ -3,8 +3,10 @@ import { UrlService } from "../src/url/urlService";
 import * as vanilla from "../src/vanilla";
 import { UrlMatcherFactory } from "../src/url/urlMatcherFactory";
 import { BrowserLocationConfig } from '../src/vanilla';
+import { resetBrowserUrl } from './_testUtils';
 
-describe('browserHistory implementation', () => {
+describe('BrowserLocationConfig implementation', () => {
+  afterAll(() => resetBrowserUrl())
 
   let router: UIRouter;
   let $url: UrlService;
diff --git a/test/vanilla.hashLocationServiceSpec.ts b/test/vanilla.hashLocationServiceSpec.ts
diff --git a/test/vanilla.pushStateLocationServiceSpec.ts b/test/vanilla.pushStateLocationServiceSpec.ts
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -39,12 +39,13 @@ export class BrowserLocationConfig implements LocationConfig {`
`39`	`39`	`};`
`40`	`40`
`41`	`41`	`baseHref(href?: string): string {`
`42`		`- return isDefined(href) ? this._baseHref = href : this._baseHref \|\| this.applyDocumentBaseHref();`
	`42`	`+ return isDefined(href) ? this._baseHref = href :`
	`43`	`+ isDefined(this._baseHref) ? this._baseHref : this.applyDocumentBaseHref();`
`43`	`44`	`}`
`44`	`45`
`45`	`46`	`applyDocumentBaseHref() {`
`46`		`- let baseTags = document.getElementsByTagName("base");`
`47`		`- return this._baseHref = baseTags.length ? baseTags[0].href.substr(location.origin.length) : "";`
	`47`	`+ let baseTag: HTMLBaseElement = document.getElementsByTagName("base")[0];`
	`48`	`+ return this._baseHref = baseTag ? baseTag.href.substr(location.origin.length) : "";`
`48`	`49`	`}`
`49`	`50`
`50`	`51`	`dispose() {}`