fix(forms): don't prevent default behavior for forms with method="dialog" (#47308)

crisbeto · dylhunn · commit 4a13210ecd69 · 2022-09-09T14:26:48.000-07:00
The forms `submit` event handlers have a `return false` to prevent form submissions from reloading the page, however this also prevents the browser behavior for forms with `method="dialog"`. These changes add an exception since the `method="dialog"` doesn't refresh the page. Fixes #47150. PR Close #47308
diff --git a/packages/forms/src/directives/ng_form.ts b/packages/forms/src/directives/ng_form.ts
@@ -298,7 +298,9 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
     (this as {submitted: boolean}).submitted = true;
     syncPendingControls(this.form, this._directives);
     this.ngSubmit.emit($event);
-    return false;
+    // Forms with `method="dialog"` have some special behavior
+    // that won't reload the page and that shouldn't be prevented.
+    return ($event?.target as HTMLFormElement | null)?.method === 'dialog';
   }
 
   /**
diff --git a/packages/forms/src/directives/reactive_directives/form_group_directive.ts b/packages/forms/src/directives/reactive_directives/form_group_directive.ts
@@ -94,8 +94,8 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
   @Output() ngSubmit = new EventEmitter();
 
   constructor(
-      @Optional() @Self() @Inject(NG_VALIDATORS) private validators: (Validator|ValidatorFn)[],
-      @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) private asyncValidators:
+      @Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator|ValidatorFn)[],
+      @Optional() @Self() @Inject(NG_ASYNC_VALIDATORS) asyncValidators:
           (AsyncValidator|AsyncValidatorFn)[]) {
     super();
     this._setValidators(validators);
@@ -271,7 +271,10 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
     (this as {submitted: boolean}).submitted = true;
     syncPendingControls(this.form, this.directives);
     this.ngSubmit.emit($event);
-    return false;
+    // Forms with `method="dialog"` have some special behavior that won't reload the page and that
+    // shouldn't be prevented. Note that we need to null check the `event` and the `target`, because
+    // some internal apps call this method directly with the wrong arguments.
+    return ($event?.target as HTMLFormElement | null)?.method === 'dialog';
   }
 
   /**
diff --git a/packages/forms/test/reactive_integration_spec.ts b/packages/forms/test/reactive_integration_spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {ɵgetDOM as getDOM} from '@angular/common';
-import {Component, Directive, forwardRef, Input, NgModule, OnDestroy, Type} from '@angular/core';
+import {Component, Directive, ElementRef, forwardRef, Input, NgModule, OnDestroy, Type, ViewChild} from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
 import {AbstractControl, AsyncValidator, AsyncValidatorFn, COMPOSITION_BUFFER_MODE, ControlValueAccessor, DefaultValueAccessor, FormArray, FormBuilder, FormControl, FormControlDirective, FormControlName, FormGroup, FormGroupDirective, FormsModule, MaxValidator, MinLengthValidator, MinValidator, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, ReactiveFormsModule, Validator, Validators} from '@angular/forms';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
@@ -2114,6 +2114,20 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
           expect(passwordControl.value).toEqual('Carson', 'Expected value to change on submit.');
           expect(passwordControl.valid).toBe(true, 'Expected validation to run on submit.');
         });
+
+        it('should not prevent the default action on forms with method="dialog"', fakeAsync(() => {
+             if (typeof HTMLDialogElement === 'undefined') {
+               return;
+             }
+
+             const fixture = initTest(NativeDialogForm);
+             fixture.detectChanges();
+             tick();
+             const event = dispatchEvent(fixture.componentInstance.form.nativeElement, 'submit');
+             fixture.detectChanges();
+
+             expect(event.defaultPrevented).toBe(false);
+           }));
       });
     });
 
@@ -5437,3 +5451,17 @@ class MinMaxFormControlComp {
   min: number|string = 1;
   max: number|string = 10;
 }
+
+@Component({
+  template: `
+    <dialog open>
+      <form #form method="dialog" [formGroup]="formGroup">
+        <button>Submit</button>
+      </form>
+    </dialog>
+  `
+})
+class NativeDialogForm {
+  @ViewChild('form') form!: ElementRef<HTMLFormElement>;
+  formGroup = new FormGroup({});
+}
diff --git a/packages/forms/test/template_integration_spec.ts b/packages/forms/test/template_integration_spec.ts
@@ -7,7 +7,7 @@
  */
 
 import {CommonModule, ɵgetDOM as getDOM} from '@angular/common';
-import {Component, Directive, forwardRef, Input, Type, ViewChild} from '@angular/core';
+import {Component, Directive, ElementRef, forwardRef, Input, Type, ViewChild} from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing';
 import {AbstractControl, AsyncValidator, COMPOSITION_BUFFER_MODE, ControlValueAccessor, FormControl, FormsModule, MaxLengthValidator, MaxValidator, MinLengthValidator, MinValidator, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, NgForm, NgModel, Validator} from '@angular/forms';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
@@ -1062,6 +1062,21 @@ import {NgModelCustomComp, NgModelCustomWrapper} from './value_accessor_integrat
                      ['fired', 'fired'],
                      'Expected ngModelChanges to fire again on submit if value changed.');
            }));
+
+
+        it('should not prevent the default action on forms with method="dialog"', fakeAsync(() => {
+             if (typeof HTMLDialogElement === 'undefined') {
+               return;
+             }
+
+             const fixture = initTest(NativeDialogForm);
+             fixture.detectChanges();
+             tick();
+             const event = dispatchEvent(fixture.componentInstance.form.nativeElement, 'submit');
+             fixture.detectChanges();
+
+             expect(event.defaultPrevented).toBe(false);
+           }));
       });
 
       describe('ngFormOptions', () => {
@@ -2932,3 +2947,17 @@ class NgModelNoMinMaxValidator {
   max!: number;
   @ViewChild('myDir') myDir: any;
 }
+
+@Component({
+  selector: 'ng-model-nested',
+  template: `
+    <dialog open>
+      <form #form method="dialog">
+        <button>Submit</button>
+      </form>
+    </dialog>
+  `
+})
+class NativeDialogForm {
+  @ViewChild('form') form!: ElementRef<HTMLFormElement>;
+}
diff --git a/packages/platform-browser/testing/src/browser_util.ts b/packages/platform-browser/testing/src/browser_util.ts
@@ -97,10 +97,11 @@ export class BrowserDetection {
 
 export const browserDetection: BrowserDetection = BrowserDetection.setup();
 
-export function dispatchEvent(element: any, eventType: any): void {
+export function dispatchEvent(element: any, eventType: any): Event {
   const evt: Event = getDOM().getDefaultDocument().createEvent('Event');
   evt.initEvent(eventType, true, true);
   getDOM().dispatchEvent(element, evt);
+  return evt;
 }
 
 export function createMouseEvent(eventType: string): MouseEvent {
