From 03b0bc92cb4f8beb0ea6300bcda40708bb1c3dd0 Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Wed, 12 Feb 2020 16:30:49 +0100
Subject: [PATCH] fix(chips): only add type attribute to button remove icons

In #18095 we made it so that the chip remove button automatically adds `type="button"` so that it doesn't accidentally submit forms. It was made so that it always gets added since it was less code than adding extra checks, but the problem is that it ends up conflicting with some CSS resets. These changes make it so that we only add it for buttons.

Fixes #18464.
---
 src/material-experimental/mdc-chips/chip-icons.ts  | 11 ++++++-----
 .../mdc-chips/chip-remove.spec.ts                  | 11 ++++++++++-
 src/material/chips/chip-remove.spec.ts             | 11 ++++++++++-
 src/material/chips/chip.ts                         | 14 ++++++++++----
 tools/public_api_guard/material/chips.d.ts         |  2 +-
 5 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/src/material-experimental/mdc-chips/chip-icons.ts b/src/material-experimental/mdc-chips/chip-icons.ts
index 10522c78aa23..ec2459d89892 100644
--- a/src/material-experimental/mdc-chips/chip-icons.ts
+++ b/src/material-experimental/mdc-chips/chip-icons.ts
@@ -112,9 +112,6 @@ const _MatChipRemoveMixinBase:
     '(click)': 'interaction.next($event)',
     '(keydown)': 'interaction.next($event)',
 
-    // Prevent accidental form submissions.
-    'type': 'button',
-
     // We need to remove this explicitly, because it gets inherited from MatChipTrailingIcon.
     '[attr.aria-hidden]': 'null',
   }
@@ -126,8 +123,12 @@ export class MatChipRemove extends _MatChipRemoveMixinBase implements CanDisable
    */
   interaction: Subject<MouseEvent | KeyboardEvent> = new Subject<MouseEvent | KeyboardEvent>();
 
-  constructor(_elementRef: ElementRef) {
-    super(_elementRef);
+  constructor(elementRef: ElementRef) {
+    super(elementRef);
+
+    if (elementRef.nativeElement.nodeName === 'BUTTON') {
+      elementRef.nativeElement.setAttribute('type', 'button');
+    }
   }
 
   static ngAcceptInputType_disabled: BooleanInput;
diff --git a/src/material-experimental/mdc-chips/chip-remove.spec.ts b/src/material-experimental/mdc-chips/chip-remove.spec.ts
index ab85e2160fb2..884daeb70bf3 100644
--- a/src/material-experimental/mdc-chips/chip-remove.spec.ts
+++ b/src/material-experimental/mdc-chips/chip-remove.spec.ts
@@ -49,6 +49,12 @@ describe('MDC-based Chip Remove', () => {
       expect(buttonElement.getAttribute('type')).toBe('button');
     });
 
+    it('should not set the `type` attribute on non-button elements', () => {
+      const buttonElement = chipNativeElement.querySelector('span.mat-mdc-chip-remove')!;
+
+      expect(buttonElement.hasAttribute('type')).toBe(false);
+    });
+
     it('should start MDC exit animation on click', () => {
       let buttonElement = chipNativeElement.querySelector('button')!;
 
@@ -158,7 +164,10 @@ describe('MDC-based Chip Remove', () => {
     <mat-chip
       [removable]="removable"
       [disabled]="disabled"
-      (removed)="didRemove()"><button matChipRemove></button></mat-chip>
+      (removed)="didRemove()">
+      <button matChipRemove></button>
+      <span matChipRemove></span>
+    </mat-chip>
   `
 })
 class TestChip {
diff --git a/src/material/chips/chip-remove.spec.ts b/src/material/chips/chip-remove.spec.ts
index 08874145bedf..d4dc3a9f5a3e 100644
--- a/src/material/chips/chip-remove.spec.ts
+++ b/src/material/chips/chip-remove.spec.ts
@@ -42,6 +42,12 @@ describe('Chip Remove', () => {
       expect(buttonElement.getAttribute('type')).toBe('button');
     });
 
+    it('should not set the `type` attribute on non-button elements', () => {
+      const buttonElement = chipNativeElement.querySelector('span.mat-chip-remove')!;
+
+      expect(buttonElement.hasAttribute('type')).toBe(false);
+    });
+
     it('should emits (removed) on click', () => {
       let buttonElement = chipNativeElement.querySelector('button')!;
 
@@ -79,7 +85,10 @@ describe('Chip Remove', () => {
     <mat-chip
       [removable]="removable"
       [disabled]="disabled"
-      (removed)="didRemove()"><button matChipRemove></button></mat-chip>
+      (removed)="didRemove()">
+      <button matChipRemove></button>
+      <span matChipRemove></span>
+    </mat-chip>
   `
 })
 class TestChip {
diff --git a/src/material/chips/chip.ts b/src/material/chips/chip.ts
index c55e267752e2..d6f645baca34 100644
--- a/src/material/chips/chip.ts
+++ b/src/material/chips/chip.ts
@@ -422,13 +422,19 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
   host: {
     'class': 'mat-chip-remove mat-chip-trailing-icon',
     '(click)': '_handleClick($event)',
-
-    // Prevent accidental form submissions.
-    'type': 'button',
   }
 })
 export class MatChipRemove {
-  constructor(protected _parentChip: MatChip) {}
+  constructor(
+    protected _parentChip: MatChip,
+    // @breaking-change 11.0.0 `elementRef` parameter to be made required.
+    elementRef?: ElementRef<HTMLElement>) {
+
+      // @breaking-change 11.0.0 Remove null check for `elementRef`.
+    if (elementRef && elementRef.nativeElement.nodeName === 'BUTTON') {
+      elementRef.nativeElement.setAttribute('type', 'button');
+    }
+   }
 
   /** Calls the parent chip's public `remove()` method if applicable. */
   _handleClick(event: Event): void {
diff --git a/tools/public_api_guard/material/chips.d.ts b/tools/public_api_guard/material/chips.d.ts
index bd91cc143ec4..6cd304a4d739 100644
--- a/tools/public_api_guard/material/chips.d.ts
+++ b/tools/public_api_guard/material/chips.d.ts
@@ -182,7 +182,7 @@ export declare class MatChipListChange {
 
 export declare class MatChipRemove {
     protected _parentChip: MatChip;
-    constructor(_parentChip: MatChip);
+    constructor(_parentChip: MatChip, elementRef?: ElementRef<HTMLElement>);
     _handleClick(event: Event): void;
     static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatChipRemove, "[matChipRemove]", never, {}, {}, never>;
     static ɵfac: i0.ɵɵFactoryDef<MatChipRemove>;