From 238ec2688b2f4907e05ad1eaeb011e93bae75995 Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Sat, 2 Mar 2024 15:41:37 -0500
Subject: [PATCH 1/2] Introduce `Turbo.config` object

The initial implementation aims to replace the ad hoc configurations
spread across `window.Turbo` and `Turbo.session`. Those objects
shouldn't be considered part of the public interface, especially for
extension.

This commit exports a single object from `src/core/config` to serve as a
centralized, user-facing repository for configuration values.

The main `src/core/config` object is composed of two constituent parts:

* `src/core/config/drive`
* `src/core/config/forms`

In the future, the directory structure can be expanded upon to suit the
needs of the project.

Any user-facing configuration outside of `Turbo.config` is deprecated.
---
 src/core/config/drive.js                      |  4 ++++
 src/core/config/forms.js                      |  3 +++
 src/core/config/index.js                      |  7 ++++++
 src/core/drive/form_submission.js             |  9 ++++++--
 src/core/index.js                             | 19 +++++++++++-----
 src/core/session.js                           | 22 ++++++++++++-------
 src/tests/fixtures/drive_disabled.html        |  2 +-
 src/tests/functional/form_submission_tests.js |  4 ++--
 src/tests/unit/export_tests.js                |  1 +
 9 files changed, 53 insertions(+), 18 deletions(-)
 create mode 100644 src/core/config/drive.js
 create mode 100644 src/core/config/forms.js
 create mode 100644 src/core/config/index.js

diff --git a/src/core/config/drive.js b/src/core/config/drive.js
new file mode 100644
index 000000000..80f3c3c69
--- /dev/null
+++ b/src/core/config/drive.js
@@ -0,0 +1,4 @@
+export const drive = {
+  enabled: true,
+  progressBarDelay: 500
+}
diff --git a/src/core/config/forms.js b/src/core/config/forms.js
new file mode 100644
index 000000000..a4061cb7d
--- /dev/null
+++ b/src/core/config/forms.js
@@ -0,0 +1,3 @@
+export const forms = {
+  mode: "on"
+}
diff --git a/src/core/config/index.js b/src/core/config/index.js
new file mode 100644
index 000000000..e8be3598e
--- /dev/null
+++ b/src/core/config/index.js
@@ -0,0 +1,7 @@
+import { drive } from "./drive"
+import { forms } from "./forms"
+
+export const config = {
+  drive,
+  forms
+}
diff --git a/src/core/drive/form_submission.js b/src/core/drive/form_submission.js
index c2ac1a0db..ca04914a6 100644
--- a/src/core/drive/form_submission.js
+++ b/src/core/drive/form_submission.js
@@ -3,6 +3,7 @@ import { expandURL } from "../url"
 import { clearBusyState, dispatch, getAttribute, getMetaContent, hasAttribute, markAsBusy } from "../../util"
 import { StreamMessage } from "../streams/stream_message"
 import { prefetchCache } from "./prefetch_cache"
+import { config } from "../config"
 
 export const FormSubmissionState = {
   initialized: "initialized",
@@ -22,7 +23,7 @@ export const FormEnctype = {
 export class FormSubmission {
   state = FormSubmissionState.initialized
 
-  static confirmMethod(message, _element, _submitter) {
+  static confirmMethod(message) {
     return Promise.resolve(confirm(message))
   }
 
@@ -78,7 +79,11 @@ export class FormSubmission {
     const confirmationMessage = getAttribute("data-turbo-confirm", this.submitter, this.formElement)
 
     if (typeof confirmationMessage === "string") {
-      const answer = await FormSubmission.confirmMethod(confirmationMessage, this.formElement, this.submitter)
+      const confirmMethod = typeof config.forms.confirm === "function" ?
+        config.forms.confirm :
+        FormSubmission.confirmMethod
+
+      const answer = await confirmMethod(confirmationMessage, this.formElement, this.submitter)
       if (!answer) {
         return
       }
diff --git a/src/core/index.js b/src/core/index.js
index a4a4f2d23..17f804bcf 100644
--- a/src/core/index.js
+++ b/src/core/index.js
@@ -2,12 +2,12 @@ import { Session } from "./session"
 import { PageRenderer } from "./drive/page_renderer"
 import { PageSnapshot } from "./drive/page_snapshot"
 import { FrameRenderer } from "./frames/frame_renderer"
-import { FormSubmission } from "./drive/form_submission"
 import { fetch, recentRequests } from "../http/fetch"
+import { config } from "./config"
 
 const session = new Session(recentRequests)
 const { cache, navigator } = session
-export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch }
+export { navigator, session, cache, PageRenderer, PageSnapshot, FrameRenderer, fetch, config }
 
 /**
  * Starts the main session.
@@ -97,13 +97,22 @@ export function clearCache() {
  * @param delay Time to delay in milliseconds
  */
 export function setProgressBarDelay(delay) {
-  session.setProgressBarDelay(delay)
+  console.warn(
+    "Please replace `Turbo.setProgressBarDelay(delay)` with `Turbo.config.drive.progressBarDelay = delay`. The top-level function is deprecated and will be removed in a future version of Turbo.`"
+  )
+  config.drive.progressBarDelay = delay
 }
 
 export function setConfirmMethod(confirmMethod) {
-  FormSubmission.confirmMethod = confirmMethod
+  console.warn(
+    "Please replace `Turbo.setConfirmMethod(confirmMethod)` with `Turbo.config.forms.confirm = confirmMethod`. The top-level function is deprecated and will be removed in a future version of Turbo.`"
+  )
+  config.forms.confirm = confirmMethod
 }
 
 export function setFormMode(mode) {
-  session.setFormMode(mode)
+  console.warn(
+    "Please replace `Turbo.setFormMode(mode)` with `Turbo.config.forms.mode = mode`. The top-level function is deprecated and will be removed in a future version of Turbo.`"
+  )
+  config.forms.mode = mode
 }
diff --git a/src/core/session.js b/src/core/session.js
index cdb978348..c66fc8e54 100644
--- a/src/core/session.js
+++ b/src/core/session.js
@@ -18,6 +18,7 @@ import { PageView } from "./drive/page_view"
 import { FrameElement } from "../elements/frame_element"
 import { Preloader } from "./drive/preloader"
 import { Cache } from "./cache"
+import { config } from "./config"
 
 export class Session {
   navigator = new Navigator(this)
@@ -37,11 +38,8 @@ export class Session {
   streamMessageRenderer = new StreamMessageRenderer()
   cache = new Cache(this)
 
-  drive = true
   enabled = true
-  progressBarDelay = 500
   started = false
-  formMode = "on"
   #pageRefreshDebouncePeriod = 150
 
   constructor(recentRequests) {
@@ -131,11 +129,19 @@ export class Session {
   }
 
   setProgressBarDelay(delay) {
+    console.warn(
+      "Please replace `session.setProgressBarDelay(delay)` with `session.progressBarDelay = delay`. The function is deprecated and will be removed in a future version of Turbo.`"
+    )
+
     this.progressBarDelay = delay
   }
 
-  setFormMode(mode) {
-    this.formMode = mode
+  set progressBarDelay(delay) {
+    config.drive.progressBarDelay = delay
+  }
+
+  get progressBarDelay() {
+    return config.drive.progressBarDelay
   }
 
   get location() {
@@ -425,12 +431,12 @@ export class Session {
   // Helpers
 
   submissionIsNavigatable(form, submitter) {
-    if (this.formMode == "off") {
+    if (config.forms.mode == "off") {
       return false
     } else {
       const submitterIsNavigatable = submitter ? this.elementIsNavigatable(submitter) : true
 
-      if (this.formMode == "optin") {
+      if (config.forms.mode == "optin") {
         return submitterIsNavigatable && form.closest('[data-turbo="true"]') != null
       } else {
         return submitterIsNavigatable && this.elementIsNavigatable(form)
@@ -443,7 +449,7 @@ export class Session {
     const withinFrame = findClosestRecursively(element, "turbo-frame")
 
     // Check if Drive is enabled on the session or we're within a Frame.
-    if (this.drive || withinFrame) {
+    if (config.drive.enabled || withinFrame) {
       // Element is navigatable by default, unless `data-turbo="false"`.
       if (container) {
         return container.getAttribute("data-turbo") != "false"
diff --git a/src/tests/fixtures/drive_disabled.html b/src/tests/fixtures/drive_disabled.html
index 92585467c..da787f0bb 100644
--- a/src/tests/fixtures/drive_disabled.html
+++ b/src/tests/fixtures/drive_disabled.html
@@ -15,7 +15,7 @@
       })
     </script>
     <script>
-      Turbo.session.drive = false
+      Turbo.config.drive = false
     </script>
   </head>
   <body>
diff --git a/src/tests/functional/form_submission_tests.js b/src/tests/functional/form_submission_tests.js
index e7447944f..99be694e7 100644
--- a/src/tests/functional/form_submission_tests.js
+++ b/src/tests/functional/form_submission_tests.js
@@ -1142,7 +1142,7 @@ test("following a link with [data-turbo-method] and [data-turbo=true] set when h
 test("following a link with [data-turbo-method] and [data-turbo=true] set when Turbo.session.drive = false", async ({
   page
 }) => {
-  await page.evaluate(() => (window.Turbo.session.drive = false))
+  await page.evaluate(() => (window.Turbo.config.drive = false))
 
   const link = await page.locator("#turbo-method-post-to-targeted-frame")
   await link.evaluate((link) => link.setAttribute("data-turbo", "true"))
@@ -1163,7 +1163,7 @@ test("following a link with [data-turbo-method] set when html[data-turbo=false]"
 })
 
 test("following a link with [data-turbo-method] set when Turbo.session.drive = false", async ({ page }) => {
-  await page.evaluate(() => (window.Turbo.session.drive = false))
+  await page.evaluate(() => (window.Turbo.config.drive = false))
   await page.click("#turbo-method-post-to-targeted-frame")
 
   assert.equal(await page.textContent("h1"), "Hello", "treats link full-page navigation")
diff --git a/src/tests/unit/export_tests.js b/src/tests/unit/export_tests.js
index 24ae7d655..d255a3fc0 100644
--- a/src/tests/unit/export_tests.js
+++ b/src/tests/unit/export_tests.js
@@ -15,6 +15,7 @@ test("Turbo interface", () => {
   assert.equal(typeof Turbo.setConfirmMethod, "function")
   assert.equal(typeof Turbo.setFormMode, "function")
   assert.equal(typeof Turbo.cache, "object")
+  assert.equal(typeof Turbo.config, "object")
   assert.equal(typeof Turbo.cache.clear, "function")
   assert.equal(typeof Turbo.navigator, "object")
   assert.equal(typeof Turbo.session, "object")

From 145faeeea512157a5b20dd82eb5f4aa0a4fe31a9 Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Sat, 2 Mar 2024 13:22:43 -0500
Subject: [PATCH 2/2] Configure Submitter disabling

Follow-up to [#386][]

While Turbo's support for disabling a Form Submissions `<input
type="submit">` or `<button>` element is rooted in [Rails UJS][], it
degrades (and has always degraded) the accessibility of those
experiences.

To learn more about the risks involved, read the [Don't Disable
Submits][] section of Adrian Roselli's [Don't Disable Form Controls][]
along with the additional resources mentioned therein.

The risk of degrading accessibility is especially true for Morph-enabled
Form Submissions. If a form submission will trigger a morphing Page
Refresh with the submitter focused, it's likely that the focus *is
intended to* remain on the submitter.

With the current `[disabled]` behavior, that is not possible without a
bespoke event handler like:

```js
addEventListener("submit", ({ target, submitter }) => {
  if (submitter) {
    target.addEventListener("turbo:submit-start", () => {
      submitter.disabled = false
      submitter.focus()
    }, { once: true })
  }
})
```

This commit introduces a `Turbo.config.forms.submitter` object with two
pre-defined keys: `"disabled"` (the default until we can deprecate it),
and `"aria-disabled"`.

When applications specify either `Turbo.config.forms.submitter =
"disabled"` or `Turbo.config.forms.submitter = "aria-disabled"`, they
will be able to leverage those pre-packed hooks. Otherwise, they can
provide their own object with `beforeSubmit(submitter)` and
`afterSubmit(submitter)` functions.

[#386]: https://github.com/hotwired/turbo/pull/386
[Rails UJS]: https://guides.rubyonrails.org/v6.0/working_with_javascript_in_rails.html#automatic-disabling
[Don't Disable Form Controls]: https://adrianroselli.com/2024/02/dont-disable-form-controls.html
[Don't Disable Submits]: https://adrianroselli.com/2024/02/dont-disable-form-controls.html#Submit
---
 src/core/config/forms.js                      | 42 +++++++++++-
 src/core/drive/form_submission.js             |  4 +-
 src/tests/functional/form_submission_tests.js | 64 +++++++++++++++++++
 src/util.js                                   |  5 ++
 4 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/src/core/config/forms.js b/src/core/config/forms.js
index a4061cb7d..7116218e3 100644
--- a/src/core/config/forms.js
+++ b/src/core/config/forms.js
@@ -1,3 +1,41 @@
-export const forms = {
-  mode: "on"
+import { cancelEvent } from "../../util"
+
+const submitter = {
+  "aria-disabled": {
+    beforeSubmit: submitter => {
+      submitter.setAttribute("aria-disabled", "true")
+      submitter.addEventListener("click", cancelEvent)
+    },
+
+    afterSubmit: submitter => {
+      submitter.removeAttribute("aria-disabled")
+      submitter.removeEventListener("click", cancelEvent)
+    }
+  },
+
+  "disabled": {
+    beforeSubmit: submitter => submitter.disabled = true,
+    afterSubmit: submitter => submitter.disabled = false
+  }
 }
+
+class Config {
+  #submitter = null
+
+  constructor(config) {
+    Object.assign(this, config)
+  }
+
+  get submitter() {
+    return this.#submitter
+  }
+
+  set submitter(value) {
+    this.#submitter = submitter[value] || value
+  }
+}
+
+export const forms = new Config({
+  mode: "on",
+  submitter: "disabled"
+})
diff --git a/src/core/drive/form_submission.js b/src/core/drive/form_submission.js
index ca04914a6..c31d8ce60 100644
--- a/src/core/drive/form_submission.js
+++ b/src/core/drive/form_submission.js
@@ -121,7 +121,7 @@ export class FormSubmission {
 
   requestStarted(_request) {
     this.state = FormSubmissionState.waiting
-    this.submitter?.setAttribute("disabled", "")
+    if (this.submitter) config.forms.submitter.beforeSubmit(this.submitter)
     this.setSubmitsWith()
     markAsBusy(this.formElement)
     dispatch("turbo:submit-start", {
@@ -167,7 +167,7 @@ export class FormSubmission {
 
   requestFinished(_request) {
     this.state = FormSubmissionState.stopped
-    this.submitter?.removeAttribute("disabled")
+    if (this.submitter) config.forms.submitter.afterSubmit(this.submitter)
     this.resetSubmitterText()
     clearBusyState(this.formElement)
     dispatch("turbo:submit-end", {
diff --git a/src/tests/functional/form_submission_tests.js b/src/tests/functional/form_submission_tests.js
index 99be694e7..8e48789ad 100644
--- a/src/tests/functional/form_submission_tests.js
+++ b/src/tests/functional/form_submission_tests.js
@@ -240,6 +240,22 @@ test("standard POST form submission toggles submitter [disabled] attribute", asy
   )
 })
 
+test("standard POST form submission toggles submitter [aria-disabled=true] attribute", async ({ page }) => {
+  await page.evaluate(() => window.Turbo.config.forms.submitter = "aria-disabled")
+  await page.click("#standard-post-form-submit")
+
+  assert.equal(
+    await nextAttributeMutationNamed(page, "standard-post-form-submit", "aria-disabled"),
+    "true",
+    "sets [aria-disabled=true] on the submitter"
+  )
+  assert.equal(
+    await nextAttributeMutationNamed(page, "standard-post-form-submit", "aria-disabled"),
+    null,
+    "removes [aria-disabled] from the submitter"
+  )
+})
+
 test("replaces input value with data-turbo-submits-with on form submission", async ({ page }) => {
   page.click("#submits-with-form-input")
 
@@ -410,6 +426,22 @@ test("standard GET form submission toggles submitter [disabled] attribute", asyn
   )
 })
 
+test("standard GET form submission toggles submitter [aria-disabled] attribute", async ({ page }) => {
+  await page.evaluate(() => window.Turbo.config.forms.submitter = "aria-disabled")
+  await page.click("#standard-get-form-submit")
+
+  assert.equal(
+    await nextAttributeMutationNamed(page, "standard-get-form-submit", "aria-disabled"),
+    "true",
+    "sets [aria-disabled] on the submitter"
+  )
+  assert.equal(
+    await nextAttributeMutationNamed(page, "standard-get-form-submit", "aria-disabled"),
+    null,
+    "removes [aria-disabled] from the submitter"
+  )
+})
+
 test("standard GET form submission appending keys", async ({ page }) => {
   await page.goto("/src/tests/fixtures/form.html?query=1")
   await page.click("#standard form.conflicting-values input[type=submit]")
@@ -692,6 +724,22 @@ test("frame POST form targeting frame toggles submitter's [disabled] attribute",
   )
 })
 
+test("frame POST form targeting frame toggles submitter's [aria-disabled] attribute", async ({ page }) => {
+  await page.evaluate(() => window.Turbo.config.forms.submitter = "aria-disabled")
+  await page.click("#targets-frame-post-form-submit")
+
+  assert.equal(
+    await nextAttributeMutationNamed(page, "targets-frame-post-form-submit", "aria-disabled"),
+    "true",
+    "sets [aria-disabled] on the submitter"
+  )
+  assert.equal(
+    await nextAttributeMutationNamed(page, "targets-frame-post-form-submit", "aria-disabled"),
+    null,
+    "removes [aria-disabled] from the submitter"
+  )
+})
+
 test("frame GET form targeting frame submission", async ({ page }) => {
   await page.click("#targets-frame-get-form-submit")
 
@@ -731,6 +779,22 @@ test("frame GET form targeting frame toggles submitter's [disabled] attribute",
   )
 })
 
+test("frame GET form targeting frame toggles submitter's [aria-disabled] attribute", async ({ page }) => {
+  await page.evaluate(() => window.Turbo.config.forms.submitter = "aria-disabled")
+  await page.click("#targets-frame-get-form-submit")
+
+  assert.equal(
+    await nextAttributeMutationNamed(page, "targets-frame-get-form-submit", "aria-disabled"),
+    "true",
+    "sets [aria-disabled] on the submitter"
+  )
+  assert.equal(
+    await nextAttributeMutationNamed(page, "targets-frame-get-form-submit", "aria-disabled"),
+    null,
+    "removes [aria-disabled] from the submitter"
+  )
+})
+
 test("frame form GET submission from submitter referencing another frame", async ({ page }) => {
   await page.click("#frame form[method=get] [type=submit][data-turbo-frame=hello]")
   await nextBeat()
diff --git a/src/util.js b/src/util.js
index dcb15c31b..4960b53e9 100644
--- a/src/util.js
+++ b/src/util.js
@@ -45,6 +45,11 @@ export function dispatch(eventName, { target, cancelable, detail } = {}) {
   return event
 }
 
+export function cancelEvent(event) {
+  event.preventDefault()
+  event.stopImmediatePropagation()
+}
+
 export function nextRepaint() {
   if (document.visibilityState === "hidden") {
     return nextEventLoopTick()