Submitter detaching issue

[muan]

Hi @javan 👋🏻,

I found this while working with activestorage/ujs.js. I thought it's worth opening an issue here because the code works when there's native support for form.requestSubmit .

Specifically, this is the ordering of events for the problematic flow:

1. polyfill: Submission triggered by a requestSubmit call
2. activestorage/ujs.js: didClick stores the submitter created by the polyfill in the WeakMap
3. polyfill: calls removeChild, the submitter is detached
4. activestorage/ujs.js: handles async upload
5. activestorage/ujs.js: gets the submitter from WeakMap and click(), which does not do anything since submitter.form is now null.

And here's a test page for a simplified test case. Form submits in Chrome, but not in Safari.

A potential fix I can think of would be firing a submit event instead of creating/appending/removing a submitter. What do you think?

[javan]

Hi! :)

Would https://github.com/javan/form-request-submit-polyfill/pull/4 (or a similar implementation) solve this issue by preventing item 2 in your list from happening?

[javan]

This could potentially be fixed in Rails too by changing its assumption that the clicked submit button is still connected:

--- a/activestorage/app/javascript/activestorage/ujs.js
+++ b/activestorage/app/javascript/activestorage/ujs.js
@@ -58,7 +58,11 @@ function handleFormSubmissionEvent(event) {
 }
 
 function submitForm(form) {
-  let button = submitButtonsByForm.get(form) || findElement(form, "input[type=submit], button[type=submit]")
+  let button = submitButtonsByForm.get(form)
+
+  if (!button || !button.isConnected) {
+    button = findElement(form, "input[type=submit], button[type=submit]")
+  }

[muan]

Would #4 (or a similar implementation) solve this issue by preventing item 2 in your list from happening?

I don't believe so, as Rails is capturing the click event on document.

This could potentially be fixed in Rails too by changing its assumption that the clicked submit button is still connected:

Yes. But since this deviates from then native requestSubmit behavior, it seems to me it should be ideally fixed on the polyfill side.

[javan]

But since this deviates from then native requestSubmit behavior, it seems to me it should be ideally fixed on the polyfill side.

The only functional difference is that the polyfilled version results in a submit button click(), which is the source of this issue. If you know of a viable alternative, I’m all ears.

I don't believe so, as Rails is capturing the click event on document.

I’m leaning towards this being an issue/bug with Rails and not this polyfill, and here’s why: the same problem could occur without requestSubmit() (polyfilled or not) if an application removed its submit button on click (to prevent double form submissions, for example).

Here’s a minimal requestSubmit()-free test case:

<meta charset="utf-8">

<script type="module">
  // A simulated approximation of Active Storage's direct upload implementation
  // https://github.com/rails/rails/blob/d94b65aad605fc53b196cfe06329b60c6468b9a7/activestorage/app/javascript/activestorage/ujs.js

  document.addEventListener("click", didClick, true)
  document.addEventListener("submit", didSubmitForm, true)

  const submitButtonsByForm = new WeakMap

  function didClick(event) {
    const { target } = event
    if ((target.tagName == "INPUT" || target.tagName == "BUTTON") && target.type == "submit" && target.form) {
      submitButtonsByForm.set(target.form, target)
    }
  }

  function didSubmitForm(event) {
    const form = event.target

    if (form.hasAttribute("data-processed")) return

    event.preventDefault()

    setTimeout(() => {
      form.toggleAttribute("data-processed")
      const button = submitButtonsByForm.get(form)
      if (button) {
        console.log("Submitting form by clicking stashed button", button)
        button.click()
      }
    }, 1000)
  }
</script>

<form>
  <input type="text" name="name" autofocus>
  <input type="submit" onclick="setTimeout(() => this.replaceWith(`Submitting…`))">
</form>

[muan]

same problem could occur without requestSubmit() (polyfilled or not) if an application removed its submit button on click (to prevent double form submissions, for example).

Fair. It's true that no other solutions would really be bulletproof for a polyfill. Feel free to close this.

Thanks!