fix(router): take base uri into account in setUpLocationSync() (#20244)

Normalize the full URL (including the base uri) before passing it to
`router.navigateByUrl()`.

Fixes #20061

PR Close #20244

karma-js.conf.js

@@ -84,7 +84,7 @@ module.exports = function(config) {
       'dist/all/@angular/elements/schematics/**',
       'dist/all/@angular/examples/**/e2e_test/*',
       'dist/all/@angular/language-service/**',
-      'dist/all/@angular/router/test/**',
+      'dist/all/@angular/router/**/test/**',
       'dist/all/@angular/platform-browser/testing/e2e_util.js',
       'dist/all/angular1_router.js',
       'dist/examples/**/e2e_test/**',

packages/router/BUILD.bazel

@@ -15,8 +15,8 @@ ng_module(
         "//packages/common",
         "//packages/core",
         "//packages/platform-browser",
-        "//packages/upgrade/static",
         "@rxjs",
+        "@rxjs//operators",
     ],
 )
 

packages/router/karma-test-shim.js

@@ -51,6 +51,8 @@ System.config({
     '@angular/platform-browser': {main: 'index.js', defaultExtension: 'js'},
     '@angular/platform-browser-dynamic/testing': {main: 'index.js', defaultExtension: 'js'},
     '@angular/platform-browser-dynamic': {main: 'index.js', defaultExtension: 'js'},
+    '@angular/upgrade/static': {main: 'index.js', defaultExtension: 'js'},
+    '@angular/router/upgrade': {main: 'index.js', defaultExtension: 'js'},
     '@angular/router/testing': {main: 'index.js', defaultExtension: 'js'},
     '@angular/router': {main: 'index.js', defaultExtension: 'js'},
     'rxjs/ajax': {main: 'index.js', defaultExtension: 'js'},

packages/router/karma.conf.js

@@ -61,21 +61,24 @@ module.exports = function(config) {
       {
         pattern: 'dist/all/@angular/platform-browser/testing/**/*.js',
         included: false,
-        watched: false,
+        watched: false
       },
 
       {pattern: 'dist/all/@angular/platform-browser-dynamic/*.js', included: false, watched: false},
       {
         pattern: 'dist/all/@angular/platform-browser-dynamic/src/**/*.js',
         included: false,
-        watched: false,
+        watched: false
       },
       {
         pattern: 'dist/all/@angular/platform-browser-dynamic/testing/**/*.js',
         included: false,
-        watched: false,
+        watched: false
       },
 
+      {pattern: 'dist/all/@angular/upgrade/static/*.js', included: false, watched: false},
+      {pattern: 'dist/all/@angular/upgrade/static/src/**/*.js', included: false, watched: false},
+
       // Router
       {pattern: 'dist/all/@angular/router/**/*.js', included: false, watched: true}
     ],

packages/router/upgrade/BUILD.bazel

@@ -6,10 +6,17 @@ load("//tools:defaults.bzl", "ng_module")
 
 ng_module(
     name = "upgrade",
-    srcs = glob(["**/*.ts"]),
+    srcs = glob(
+        [
+            "*.ts",
+            "src/**/*.ts",
+        ],
+    ),
     module_name = "@angular/router/upgrade",
     deps = [
+        "//packages/common",
         "//packages/core",
         "//packages/router",
+        "//packages/upgrade/static",
     ],
 )

packages/router/upgrade/src/upgrade.ts

@@ -6,12 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {Location} from '@angular/common';
 import {APP_BOOTSTRAP_LISTENER, ComponentRef, InjectionToken} from '@angular/core';
 import {Router} from '@angular/router';
 import {UpgradeModule} from '@angular/upgrade/static';
 
-
-
 /**
  * @description
  *
@@ -68,11 +67,47 @@ export function setUpLocationSync(ngUpgrade: UpgradeModule) {
   }
 
   const router: Router = ngUpgrade.injector.get(Router);
-  const url = document.createElement('a');
+  const location: Location = ngUpgrade.injector.get(Location);
 
   ngUpgrade.$injector.get('$rootScope')
       .$on('$locationChangeStart', (_: any, next: string, __: string) => {
-        url.href = next;
+        const url = resolveUrl(next);
-        router.navigateByUrl(url.pathname + url.search + url.hash);
+        const path = location.normalize(url.pathname);
+        router.navigateByUrl(path + url.search + url.hash);
       });
 }
+
+/**
+ * Normalize and parse a URL.
+ *
+ * - Normalizing means that a relative URL will be resolved into an absolute URL in the context of
+ *   the application document.
+ * - Parsing means that the anchor's `protocol`, `hostname`, `port`, `pathname` and related
+ *   properties are all populated to reflect the normalized URL.
+ *
+ * While this approach has wide compatibility, it doesn't work as expected on IE. On IE, normalizing
+ * happens similar to other browsers, but the parsed components will not be set. (E.g. if you assign
+ * `a.href = 'foo'`, then `a.protocol`, `a.host`, etc. will not be correctly updated.)
+ * We work around that by performing the parsing in a 2nd step by taking a previously normalized URL
+ * and assigning it again. This correctly populates all properties.
+ *
+ * See
+ * https://github.com/angular/angular.js/blob/2c7400e7d07b0f6cec1817dab40b9250ce8ebce6/src/ng/urlUtils.js#L26-L33
+ * for more info.
+ */
+let anchor: HTMLAnchorElement|undefined;
+function resolveUrl(url: string): {pathname: string, search: string, hash: string} {
+  if (!anchor) {
+    anchor = document.createElement('a');
+  }
+
+  anchor.setAttribute('href', url);
+  anchor.setAttribute('href', anchor.href);
+
+  return {
+    // IE does not start `pathname` with `/` like other browsers.
+    pathname: `/${anchor.pathname.replace(/^\//, '')}`,
+    search: anchor.search,
+    hash: anchor.hash
+  };
+}

packages/router/upgrade/test/BUILD.bazel

@@ -0,0 +1,21 @@
+load("//tools:defaults.bzl", "ts_library", "ts_web_test_suite")
+
+ts_library(
+    name = "test_lib",
+    testonly = 1,
+    srcs = glob(["**/*.ts"]),
+    deps = [
+        "//packages/common",
+        "//packages/core/testing",
+        "//packages/router",
+        "//packages/router/upgrade",
+        "//packages/upgrade/static",
+    ],
+)
+
+ts_web_test_suite(
+    name = "test_web",
+    deps = [
+        ":test_lib",
+    ],
+)

packages/router/upgrade/test/upgrade.spec.ts

@@ -0,0 +1,101 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {Location} from '@angular/common';
+import {TestBed} from '@angular/core/testing';
+import {Router} from '@angular/router';
+import {setUpLocationSync} from '@angular/router/upgrade';
+import {UpgradeModule} from '@angular/upgrade/static';
+
+describe('setUpLocationSync', () => {
+  let upgradeModule: UpgradeModule;
+  let RouterMock: any;
+  let LocationMock: any;
+
+  beforeEach(() => {
+    RouterMock = jasmine.createSpyObj('Router', ['navigateByUrl']);
+    LocationMock = jasmine.createSpyObj('Location', ['normalize']);
+
+    TestBed.configureTestingModule({
+      providers: [
+        UpgradeModule, {provide: Router, useValue: RouterMock},
+        {provide: Location, useValue: LocationMock}
+      ],
+    });
+
+    upgradeModule = TestBed.get(UpgradeModule);
+    upgradeModule.$injector = {
+      get: jasmine.createSpy('$injector.get').and.returnValue({'$on': () => undefined})
+    };
+  });
+
+  it('should throw an error if the UpgradeModule.bootstrap has not been called', () => {
+    upgradeModule.$injector = null;
+
+    expect(() => setUpLocationSync(upgradeModule)).toThrowError(`
+        RouterUpgradeInitializer can be used only after UpgradeModule.bootstrap has been called.
+        Remove RouterUpgradeInitializer and call setUpLocationSync after UpgradeModule.bootstrap.
+      `);
+  });
+
+  it('should get the $rootScope from AngularJS and set an $on watch on $locationChangeStart',
+     () => {
+       const $rootScope = jasmine.createSpyObj('$rootScope', ['$on']);
+
+       upgradeModule.$injector.get.and.callFake(
+           (name: string) => (name === '$rootScope') && $rootScope);
+
+       setUpLocationSync(upgradeModule);
+
+       expect($rootScope.$on).toHaveBeenCalledTimes(1);
+       expect($rootScope.$on).toHaveBeenCalledWith('$locationChangeStart', jasmine.any(Function));
+     });
+
+  it('should navigate by url every time $locationChangeStart is broadcasted', () => {
+    const url = 'https://google.com';
+    const pathname = '/custom/route';
+    const normalizedPathname = 'foo';
+    const query = '?query=1&query2=3';
+    const hash = '#new/hash';
+    const $rootScope = jasmine.createSpyObj('$rootScope', ['$on']);
+
+    upgradeModule.$injector.get.and.returnValue($rootScope);
+    LocationMock.normalize.and.returnValue(normalizedPathname);
+
+    setUpLocationSync(upgradeModule);
+
+    const callback = $rootScope.$on.calls.argsFor(0)[1];
+    callback({}, url + pathname + query + hash, '');
+
+    expect(LocationMock.normalize).toHaveBeenCalledTimes(1);
+    expect(LocationMock.normalize).toHaveBeenCalledWith(pathname);
+
+    expect(RouterMock.navigateByUrl).toHaveBeenCalledTimes(1);
+    expect(RouterMock.navigateByUrl).toHaveBeenCalledWith(normalizedPathname + query + hash);
+  });
+
+  it('should work correctly on browsers that do not start pathname with `/`', () => {
+    const anchorProto = HTMLAnchorElement.prototype;
+    const originalDescriptor = Object.getOwnPropertyDescriptor(anchorProto, 'pathname');
+    Object.defineProperty(anchorProto, 'pathname', {get: () => 'foo/bar'});
+
+    try {
+      const $rootScope = jasmine.createSpyObj('$rootScope', ['$on']);
+      upgradeModule.$injector.get.and.returnValue($rootScope);
+
+      setUpLocationSync(upgradeModule);
+
+      const callback = $rootScope.$on.calls.argsFor(0)[1];
+      callback({}, '', '');
+
+      expect(LocationMock.normalize).toHaveBeenCalledWith('/foo/bar');
+    } finally {
+      Object.defineProperty(anchorProto, 'pathname', originalDescriptor !);
+    }
+  });
+});