Update amp-form docs to match validators and runtime updates. (amppro…

…ject#5671)

extensions/amp-form/0.1/amp-form.js

@@ -211,9 +211,12 @@ export class AmpForm {
         this.renderTemplate_(error.responseJson || {});
         rethrowAsync('Form submission failed:', error);
       });
-    } else if (this.target_ == '_top' && this.method_ == 'POST') {
-      this.cleanupRenderedTemplate_();
-      this.setState_(FormState_.SUBMITTING);
+    } else if (this.method_ == 'POST') {
+      e.preventDefault();
+      user().assert(false,
+          'Only XHR based (via action-xhr attribute) submissions are support ' +
+          'for POST requests. %s',
+          this.form_);
     }
   }
 

extensions/amp-form/0.1/test/test-amp-form.js

@@ -149,6 +149,22 @@ describe('amp-form', () => {
     expect(ampForm.xhr_.fetchJson.called).to.be.false;
   });
 
+  it('should throw error if POST non-xhr', () => {
+    const form = getForm();
+    form.removeAttribute('action-xhr');
+    const ampForm = new AmpForm(form);
+    const event = {
+      stopImmediatePropagation: sandbox.spy(),
+      target: form,
+      preventDefault: sandbox.spy(),
+    };
+    sandbox.spy(ampForm.xhr_, 'fetchJson');
+    sandbox.spy(form, 'checkValidity');
+    expect(() => ampForm.handleSubmit_(event)).to.throw(
+        /Only XHR based \(via action-xhr attribute\) submissions are support/);
+    expect(event.preventDefault).to.be.called;
+  });
+
   it('should respect novalidate on a form', () => {
     setReportValiditySupported(true);
     const form = getForm();
@@ -176,14 +192,13 @@ describe('amp-form', () => {
     };
     sandbox.spy(form, 'checkValidity');
     sandbox.spy(emailInput, 'reportValidity');
-    ampForm.xhrAction_ = null;
+
     ampForm.handleSubmit_(event);
     // Check validity should always be called regardless of novalidate.
     expect(form.checkValidity.called).to.be.true;
 
     // However reporting validity shouldn't happen when novalidate.
     expect(emailInput.reportValidity.called).to.be.false;
-    expect(event.preventDefault.called).to.be.false;
     expect(form.hasAttribute('amp-novalidate')).to.be.true;
   });
 

extensions/amp-form/amp-form.md

@@ -71,18 +71,22 @@ __required__
 Target attribute of the `<form>` must be either `_blank` or `_top`.
 
 **action**
-__required__
+__invalid__ when `method=POST`
+__required__ when `method=GET`
 
 Action must be provided, `https` and is non-cdn link (does **NOT** link to https://cdn.ampproject.org).
 
 __Note__: `target` and `action` will only be used for non-xhr GET requests. AMP runtime will use `action-xhr` to make the request and will ignore `action` and `target`. When `action-xhr` is not provided AMP would make a GET request to `action` endpoint and use `target` to open a new window (if `_blank`). AMP runtime might also fallback to using action and target in cases where `amp-form` extension fails to load.
 
 **action-xhr**
-__(optional)__ for `GET` __required__ for `POST` requests 
+__required__ when `method=POST`
+__optional__ when `method=GET`
+
 You can also provide an action-xhr attribute, if provided, the form will be submitted in an XHR fashion.
 
-This attribute can be the same or a different endpoint than `action` and has the same action requirements above.
+An XHR request is where the browser would make the request without a full load of the page or opening a new page also sometimes called Ajax request. Browsers will send the request in the background using [Fetch API](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API) when available and fallback to [XMLHttpRequest API](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest) for older browsers.
 
+This attribute can be the same or a different endpoint than `action` and has the same action requirements above.
 
 **Important**: See [Security Considerations](#security-considerations) for notes on how to secure your forms endpoints.
 

src/document-submit.js

@@ -60,29 +60,38 @@ export function onDocumentFormSubmit_(e) {
   const actionXhr = form.getAttribute('action-xhr');
   const method = (form.getAttribute('method') || 'GET').toUpperCase();
   if (method == 'GET') {
+    // TODO(#5670): Make action optional for method=GET when action-xhr is provided.
     user().assert(action,
         'form action attribute is required for method=GET: %s', form);
     assertHttpsUrl(action, dev().assertElement(form), 'action');
     user().assert(!startsWith(action, urls.cdn),
         'form action should not be on AMP CDN: %s', form);
     checkCorsUrl(action);
-  } else if (action) {
-    e.preventDefault();
-    user().assert(false,
-        'form action attribute is invalid for method=POST: %s', form);
-  } else if (!actionXhr) {
-    e.preventDefault();
-    user().assert(false,
-        'Only XHR based (via action-xhr attribute) submissions are support ' +
-        'for POST requests. %s',
-        form);
+  } else if (method == 'POST') {
+    if (action) {
+      e.preventDefault();
+      user().assert(false,
+          'form action attribute is invalid for method=POST: %s', form);
+    }
+
+    if (!actionXhr) {
+      e.preventDefault();
+      user().assert(false,
+          'Only XHR based (via action-xhr attribute) submissions are support ' +
+          'for POST requests. %s',
+          form);
+    }
   }
-  checkCorsUrl(actionXhr);
 
+  // TODO(#5607): Only require this with method=GET.
   const target = form.getAttribute('target');
-  user().assert(target, 'form target attribute is required: %s', form);
+  user().assert(target,
+      'form target attribute is required: %s', form);
   user().assert(target == '_blank' || target == '_top',
       'form target=%s is invalid can only be _blank or _top: %s', target, form);
+  if (actionXhr) {
+    checkCorsUrl(actionXhr);
+  }
 
   // amp-form extension will add novalidate to all forms to manually trigger
   // validation. In that case `novalidate` doesn't have the same meaning.
@@ -97,9 +106,6 @@ export function onDocumentFormSubmit_(e) {
   // Safari does not trigger validation check on submission, hence we
   // trigger it manually. In other browsers this would never execute since
   // the submit event wouldn't be fired if the form is invalid.
-  // TODO: This doesn't display the validation error messages. Safari makes them
-  // available per input.validity object. We need to figure out a way of
-  // displaying these.
   if (shouldValidate && form.checkValidity && !form.checkValidity()) {
     e.preventDefault();
     return;