fix(progress-bar): incorrectly handling current path when using hash location strategy (#12713)

Fixes the progress bar prefixing the references incorrectly when the consumer is using the router's hash location strategy. The issue comes from the fact that the router normalizes the `location.pathname` between the regular strategy and the hash one, whereas we need to use the exact same path as the `window.location`.

Fixes #12710.

Author: crisbeto

src/lib/progress-bar/progress-bar.spec.ts

@@ -1,52 +1,49 @@
-import {TestBed, async, ComponentFixture} from '@angular/core/testing';
-import {Component} from '@angular/core';
+import {TestBed, ComponentFixture} from '@angular/core/testing';
+import {Component, Type} from '@angular/core';
 import {By} from '@angular/platform-browser';
-import {Location} from '@angular/common';
-import {MatProgressBarModule} from './index';
+import {MatProgressBarModule, MAT_PROGRESS_BAR_LOCATION} from './index';
 
 
 describe('MatProgressBar', () => {
   let fakePath = '/fake-path';
 
-  beforeEach(async(() => {
+  function createComponent<T>(componentType: Type<T>): ComponentFixture<T> {
     TestBed.configureTestingModule({
       imports: [MatProgressBarModule],
-      declarations: [
-        BasicProgressBar,
-        BufferProgressBar,
-      ],
+      declarations: [componentType],
       providers: [{
-        provide: Location,
-        useValue: {path: () => fakePath}
+        provide: MAT_PROGRESS_BAR_LOCATION,
+        useValue: {pathname: fakePath}
       }]
-    });
-
-    TestBed.compileComponents();
-  }));
+    }).compileComponents();
 
+    return TestBed.createComponent<T>(componentType);
+  }
 
   describe('basic progress-bar', () => {
-    let fixture: ComponentFixture<BasicProgressBar>;
-
-    beforeEach(() => {
-      fixture = TestBed.createComponent(BasicProgressBar);
+    it('should apply a mode of "determinate" if no mode is provided.', () => {
+      const fixture = createComponent(BasicProgressBar);
       fixture.detectChanges();
-    });
 
-    it('should apply a mode of "determinate" if no mode is provided.', () => {
-      let progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
+      const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
       expect(progressElement.componentInstance.mode).toBe('determinate');
     });
 
     it('should define default values for value and bufferValue attributes', () => {
-      let progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
+      const fixture = createComponent(BasicProgressBar);
+      fixture.detectChanges();
+
+      const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
       expect(progressElement.componentInstance.value).toBe(0);
       expect(progressElement.componentInstance.bufferValue).toBe(0);
     });
 
     it('should clamp value and bufferValue between 0 and 100', () => {
-      let progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
-      let progressComponent = progressElement.componentInstance;
+      const fixture = createComponent(BasicProgressBar);
+      fixture.detectChanges();
+
+      const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
+      const progressComponent = progressElement.componentInstance;
 
       progressComponent.value = 50;
       expect(progressComponent.value).toBe(50);
@@ -68,8 +65,11 @@ describe('MatProgressBar', () => {
     });
 
     it('should return the transform attribute for bufferValue and mode', () => {
-      let progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
-      let progressComponent = progressElement.componentInstance;
+      const fixture = createComponent(BasicProgressBar);
+      fixture.detectChanges();
+
+      const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
+      const progressComponent = progressElement.componentInstance;
 
       expect(progressComponent._primaryTransform()).toEqual({transform: 'scaleX(0)'});
       expect(progressComponent._bufferTransform()).toBe(undefined);
@@ -95,26 +95,38 @@ describe('MatProgressBar', () => {
     });
 
     it('should prefix SVG references with the current path', () => {
+      const fixture = createComponent(BasicProgressBar);
+      fixture.detectChanges();
+
       const rect = fixture.debugElement.query(By.css('rect')).nativeElement;
       expect(rect.getAttribute('fill')).toMatch(/^url\(['"]?\/fake-path#.*['"]?\)$/);
     });
 
+    it('should account for location hash when prefixing the SVG references', () => {
+      fakePath = '/fake-path#anchor';
+
+      const fixture = createComponent(BasicProgressBar);
+      fixture.detectChanges();
+
+      const rect = fixture.debugElement.query(By.css('rect')).nativeElement;
+      expect(rect.getAttribute('fill')).not.toContain('#anchor#');
+    });
+
     it('should not be able to tab into the underlying SVG element', () => {
+      const fixture = createComponent(BasicProgressBar);
+      fixture.detectChanges();
+
       const svg = fixture.debugElement.query(By.css('svg')).nativeElement;
       expect(svg.getAttribute('focusable')).toBe('false');
     });
   });
 
   describe('buffer progress-bar', () => {
-    let fixture: ComponentFixture<BufferProgressBar>;
-
-    beforeEach(() => {
-      fixture = TestBed.createComponent(BufferProgressBar);
+    it('should not modify the mode if a valid mode is provided.', () => {
+      const fixture = createComponent(BufferProgressBar);
       fixture.detectChanges();
-    });
 
-    it('should not modify the mode if a valid mode is provided.', () => {
-      let progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
+      const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'));
       expect(progressElement.componentInstance.mode).toBe('buffer');
     });
   });

src/lib/progress-bar/progress-bar.ts

@@ -12,9 +12,9 @@ import {
   Inject,
   Input,
   Optional,
-  ViewEncapsulation
+  ViewEncapsulation,
+  InjectionToken
 } from '@angular/core';
-import {Location} from '@angular/common';
 import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
 import {CanColor, mixinColor} from '@angular/material/core';
 
@@ -29,6 +29,30 @@ export class MatProgressBarBase {
 
 export const _MatProgressBarMixinBase = mixinColor(MatProgressBarBase, 'primary');
 
+/**
+ * Injection token used to provide the current location to `MatProgressBar`.
+ * Used to handle server-side rendering and to stub out during unit tests.
+ * @docs-private
+ */
+export const MAT_PROGRESS_BAR_LOCATION = new InjectionToken<MatProgressBarLocation>(
+  'mat-progress-bar-location',
+  {providedIn: 'root', factory: MAT_PROGRESS_BAR_LOCATION_FACTORY}
+);
+
+/**
+ * Stubbed out location for `MatProgressBar`.
+ * @docs-private
+ */
+export interface MatProgressBarLocation {
+  pathname: string;
+}
+
+/** @docs-private */
+export function MAT_PROGRESS_BAR_LOCATION_FACTORY(): MatProgressBarLocation {
+  return typeof window !== 'undefined' ? window.location : {pathname: ''};
+}
+
+
 /** Counter used to generate unique IDs for progress bars. */
 let progressbarId = 0;
 
@@ -61,13 +85,17 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor
                * @deprecated `location` parameter to be made required.
                * @breaking-change 8.0.0
                */
-              @Optional() location?: Location) {
+              @Optional() @Inject(MAT_PROGRESS_BAR_LOCATION) location?: MatProgressBarLocation) {
     super(_elementRef);
 
     // We need to prefix the SVG reference with the current path, otherwise they won't work
     // in Safari if the page has a `<base>` tag. Note that we need quotes inside the `url()`,
-    // because named route URLs can contain parentheses (see #12338).
-    this._rectangleFillValue = `url('${location ? location.path() : ''}#${this.progressbarId}')`;
+    // because named route URLs can contain parentheses (see #12338). Also we don't use
+    // `Location` from `@angular/common` since we can't tell the difference between whether
+    // the consumer is using the hash location strategy or not, because `Location` normalizes
+    // both `/#/foo/bar` and `/foo/bar` to the same thing.
+    const path = location ? location.pathname.split('#')[0] : '';
+    this._rectangleFillValue = `url('${path}#${this.progressbarId}')`;
   }
 
   /** Value of the progress bar. Defaults to zero. Mirrored to aria-valuenow. */