From f52a47f49567bd1b2aca637d41d8f7eba5850c88 Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Sat, 16 Jul 2022 12:37:34 -0400
Subject: [PATCH 1/2] Introduce `FormLinkInterceptor`

In an effort to match the patterns established by `LinkInterceptor`
and `FormInterceptor`, this commit introduces a
`FormLinkInterceptor` and `FormLinkInterceptorDelegate`.

Behind the scenes, the `FormLinkInterceptor` relies on an instance of
the `LinkInterceptor` to intervene in `<a>` element clicks when
`[data-turbo-method]` or `[data-turbo-stream]` are present. When those
clicks are detected, it creates a `<form hidden>` element, attaches it
to the document, delegates to a `FormLinkInterceptorDelegate` to map
the `<a>` element's attributes to the `<form>` element, submits the form
through the polyfilled [HTMLFormElement.requestSubmit][] method, then
removes the `<form>` from the document.

The `Session` serves as a `FormLinkInterceptorDelegate`, making sure
to start and stop the observer _before_ its `LinkInterceptor`
instance, so that clicks that are intercepted by the
`FormLinkInterceptor` are not also intercepted by the
`LinkInterceptor`.

[HTMLFormElement.requestSubmit]: https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/requestSubmit
---
 src/core/frames/frame_controller.ts           | 25 ++++++--
 src/core/session.ts                           | 63 +++++--------------
 src/observers/form_link_interceptor.ts        | 52 +++++++++++++++
 src/tests/fixtures/form.html                  |  1 +
 src/tests/functional/form_submission_tests.ts | 10 +++
 5 files changed, 97 insertions(+), 54 deletions(-)
 create mode 100644 src/observers/form_link_interceptor.ts

diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts
index b90808058..d246f74c8 100644
--- a/src/core/frames/frame_controller.ts
+++ b/src/core/frames/frame_controller.ts
@@ -7,7 +7,7 @@ import {
 import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
 import { FetchResponse } from "../../http/fetch_response"
 import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
-import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy, attributeTrue } from "../../util"
+import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy } from "../../util"
 import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
@@ -15,6 +15,7 @@ import { getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
 import { FormInterceptor, FormInterceptorDelegate } from "./form_interceptor"
 import { FrameView } from "./frame_view"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
+import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../../observers/form_link_interceptor"
 import { FrameRenderer } from "./frame_renderer"
 import { session } from "../index"
 import { isAction } from "../types"
@@ -26,12 +27,14 @@ export class FrameController
     FormInterceptorDelegate,
     FormSubmissionDelegate,
     FrameElementDelegate,
+    FormLinkInterceptorDelegate,
     LinkInterceptorDelegate,
     ViewDelegate<Snapshot<FrameElement>>
 {
   readonly element: FrameElement
   readonly view: FrameView
   readonly appearanceObserver: AppearanceObserver
+  readonly formLinkInterceptor: FormLinkInterceptor
   readonly linkInterceptor: LinkInterceptor
   readonly formInterceptor: FormInterceptor
   formSubmission?: FormSubmission
@@ -46,6 +49,7 @@ export class FrameController
     this.element = element
     this.view = new FrameView(this, this.element)
     this.appearanceObserver = new AppearanceObserver(this, this.element)
+    this.formLinkInterceptor = new FormLinkInterceptor(this, this.element)
     this.linkInterceptor = new LinkInterceptor(this, this.element)
     this.formInterceptor = new FormInterceptor(this, this.element)
   }
@@ -58,6 +62,7 @@ export class FrameController
       } else {
         this.loadSourceURL()
       }
+      this.formLinkInterceptor.start()
       this.linkInterceptor.start()
       this.formInterceptor.start()
     }
@@ -67,6 +72,7 @@ export class FrameController
     if (this.connected) {
       this.connected = false
       this.appearanceObserver.stop()
+      this.formLinkInterceptor.stop()
       this.linkInterceptor.stop()
       this.formInterceptor.stop()
     }
@@ -146,14 +152,21 @@ export class FrameController
     this.loadSourceURL()
   }
 
+  // Form link interceptor delegate
+
+  shouldInterceptFormLinkClick(link: Element): boolean {
+    return this.shouldInterceptNavigation(link)
+  }
+
+  formLinkClickIntercepted(link: Element, form: HTMLFormElement): void {
+    const frame = this.findFrameElement(link)
+    if (frame) form.setAttribute("data-turbo-frame", frame.id)
+  }
+
   // Link interceptor delegate
 
   shouldInterceptLinkClick(element: Element, _url: string) {
-    if (element.hasAttribute("data-turbo-method") || attributeTrue(element, "data-turbo-stream")) {
-      return false
-    } else {
-      return this.shouldInterceptNavigation(element)
-    }
+    return this.shouldInterceptNavigation(element)
   }
 
   linkClickIntercepted(element: Element, url: string) {
diff --git a/src/core/session.ts b/src/core/session.ts
index 4b723577f..8817c50ed 100644
--- a/src/core/session.ts
+++ b/src/core/session.ts
@@ -5,6 +5,7 @@ import { FormSubmitObserver, FormSubmitObserverDelegate } from "../observers/for
 import { FrameRedirector } from "./frames/frame_redirector"
 import { History, HistoryDelegate } from "./drive/history"
 import { LinkClickObserver, LinkClickObserverDelegate } from "../observers/link_click_observer"
+import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../observers/form_link_interceptor"
 import { getAction, expandURL, locationIsVisitable, Locatable } from "./url"
 import { Navigator, NavigatorDelegate } from "./drive/navigator"
 import { PageObserver, PageObserverDelegate } from "../observers/page_observer"
@@ -12,7 +13,7 @@ import { ScrollObserver } from "../observers/scroll_observer"
 import { StreamMessage } from "./streams/stream_message"
 import { StreamObserver } from "../observers/stream_observer"
 import { Action, Position, StreamSource, isAction } from "./types"
-import { attributeTrue, clearBusyState, dispatch, markAsBusy } from "../util"
+import { clearBusyState, dispatch, markAsBusy } from "../util"
 import { PageView, PageViewDelegate } from "./drive/page_view"
 import { Visit, VisitOptions } from "./drive/visit"
 import { PageSnapshot } from "./drive/page_snapshot"
@@ -35,6 +36,7 @@ export class Session
   implements
     FormSubmitObserverDelegate,
     HistoryDelegate,
+    FormLinkInterceptorDelegate,
     LinkClickObserverDelegate,
     NavigatorDelegate,
     PageObserverDelegate,
@@ -53,7 +55,7 @@ export class Session
   readonly formSubmitObserver = new FormSubmitObserver(this)
   readonly scrollObserver = new ScrollObserver(this)
   readonly streamObserver = new StreamObserver(this)
-
+  readonly formLinkInterceptor = new FormLinkInterceptor(this, document.documentElement)
   readonly frameRedirector = new FrameRedirector(document.documentElement)
 
   drive = true
@@ -66,6 +68,7 @@ export class Session
     if (!this.started) {
       this.pageObserver.start()
       this.cacheObserver.start()
+      this.formLinkInterceptor.start()
       this.linkClickObserver.start()
       this.formSubmitObserver.start()
       this.scrollObserver.start()
@@ -86,6 +89,7 @@ export class Session
     if (this.started) {
       this.pageObserver.stop()
       this.cacheObserver.stop()
+      this.formLinkInterceptor.stop()
       this.linkClickObserver.stop()
       this.formSubmitObserver.stop()
       this.scrollObserver.stop()
@@ -157,6 +161,14 @@ export class Session
     this.history.updateRestorationData({ scrollPosition: position })
   }
 
+  // Form link interceptor delegate
+
+  shouldInterceptFormLinkClick(_link: Element): boolean {
+    return true
+  }
+
+  formLinkClickIntercepted(_link: Element, _form: HTMLFormElement) {}
+
   // Link click observer delegate
 
   willFollowLinkToLocation(link: Element, location: URL, event: MouseEvent) {
@@ -169,39 +181,7 @@ export class Session
 
   followedLinkToLocation(link: Element, location: URL) {
     const action = this.getActionForLink(link)
-    this.convertLinkWithMethodClickToFormSubmission(link) || this.visit(location.href, { action })
-  }
-
-  convertLinkWithMethodClickToFormSubmission(link: Element) {
-    const linkMethod = link.getAttribute("data-turbo-method")
-    const useTurboStream = attributeTrue(link, "data-turbo-stream")
-
-    if (linkMethod || useTurboStream) {
-      const form = document.createElement("form")
-      form.setAttribute("method", linkMethod || "get")
-      form.action = link.getAttribute("href") || "undefined"
-      form.hidden = true
-
-      const attributes = ["data-turbo-confirm", "data-turbo-stream"]
-      attributes.forEach((attribute) => {
-        if (link.hasAttribute(attribute)) {
-          form.setAttribute(attribute, link.getAttribute(attribute)!)
-        }
-      })
-
-      const frame = this.getTargetFrameForLink(link)
-      if (frame) {
-        form.setAttribute("data-turbo-frame", frame)
-        form.addEventListener("turbo:submit-start", () => form.remove())
-      } else {
-        form.addEventListener("submit", () => form.remove())
-      }
-
-      document.body.appendChild(form)
-      return dispatch("submit", { cancelable: true, target: form })
-    } else {
-      return false
-    }
+    this.visit(location.href, { action })
   }
 
   // Navigator delegate
@@ -423,19 +403,6 @@ export class Session
     return isAction(action) ? action : "advance"
   }
 
-  getTargetFrameForLink(link: Element) {
-    const frame = link.getAttribute("data-turbo-frame")
-
-    if (frame) {
-      return frame
-    } else {
-      const container = link.closest("turbo-frame")
-      if (container) {
-        return container.id
-      }
-    }
-  }
-
   get snapshot() {
     return this.view.snapshot
   }
diff --git a/src/observers/form_link_interceptor.ts b/src/observers/form_link_interceptor.ts
new file mode 100644
index 000000000..8bc7b720d
--- /dev/null
+++ b/src/observers/form_link_interceptor.ts
@@ -0,0 +1,52 @@
+import { LinkInterceptor, LinkInterceptorDelegate } from "../core/frames/link_interceptor"
+
+export type FormLinkInterceptorDelegate = {
+  shouldInterceptFormLinkClick(link: Element): boolean
+  formLinkClickIntercepted(link: Element, form: HTMLFormElement): void
+}
+
+export class FormLinkInterceptor implements LinkInterceptorDelegate {
+  readonly linkInterceptor: LinkInterceptor
+  readonly delegate: FormLinkInterceptorDelegate
+
+  constructor(delegate: FormLinkInterceptorDelegate, element: HTMLElement) {
+    this.delegate = delegate
+    this.linkInterceptor = new LinkInterceptor(this, element)
+  }
+
+  start() {
+    this.linkInterceptor.start()
+  }
+
+  stop() {
+    this.linkInterceptor.stop()
+  }
+
+  shouldInterceptLinkClick(link: Element): boolean {
+    return (
+      this.delegate.shouldInterceptFormLinkClick(link) &&
+      (link.hasAttribute("data-turbo-method") || link.hasAttribute("data-turbo-stream"))
+    )
+  }
+
+  linkClickIntercepted(link: Element, action: string): void {
+    const form = document.createElement("form")
+    form.setAttribute("action", action)
+    form.hidden = true
+
+    const method = link.getAttribute("data-turbo-method")
+    if (method) form.setAttribute("method", method)
+
+    const turboConfirm = link.getAttribute("data-turbo-confirm")
+    if (turboConfirm) form.setAttribute("data-turbo-confirm", turboConfirm)
+
+    const turboStream = link.getAttribute("data-turbo-stream")
+    if (turboStream) form.setAttribute("data-turbo-stream", turboStream)
+
+    this.delegate.formLinkClickIntercepted(link, form)
+
+    document.body.appendChild(form)
+    form.requestSubmit()
+    form.remove()
+  }
+}
diff --git a/src/tests/fixtures/form.html b/src/tests/fixtures/form.html
index cbdbb0bc9..cbf10aa34 100644
--- a/src/tests/fixtures/form.html
+++ b/src/tests/fixtures/form.html
@@ -299,6 +299,7 @@ <h2>Frame: Form</h2>
     <form method="post" action="https://httpbin.org/post">
       <button id="submit-external">POST to https://httpbin.org/post</button>
     </form>
+     <a href="/__turbo/redirect?path=/src/tests/fixtures/frames/hello.html" data-turbo-method="post" data-turbo-frame="hello" id="turbo-method-post-to-targeted-frame">Turbo method post to targeted frame</a>
     <turbo-frame id="hello"></turbo-frame>
     <hr>
 
diff --git a/src/tests/functional/form_submission_tests.ts b/src/tests/functional/form_submission_tests.ts
index cc713df85..fcbd8eaea 100644
--- a/src/tests/functional/form_submission_tests.ts
+++ b/src/tests/functional/form_submission_tests.ts
@@ -976,6 +976,16 @@ test("test POST to external action targetting frame ignored", async ({ page }) =
   assert.equal(page.url(), "https://httpbin.org/post")
 })
 
+test("test following a link with data-turbo-method set and a target set navigates the target frame", async ({
+  page,
+}) => {
+  await page.click("#turbo-method-post-to-targeted-frame")
+  await nextBeat()
+
+  const frameText = await page.locator("#hello h2")
+  assert.equal(await frameText.textContent(), "Hello from a frame")
+})
+
 function formSubmitStarted(page: Page) {
   return getFromLocalStorage(page, "formSubmitStarted")
 }

From 2777608f12b41869596b02237c51adf211b003f0 Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Sat, 16 Jul 2022 17:25:56 -0400
Subject: [PATCH 2/2] Account for `[data-turbo="false"]` and
 `Turbo.session.drive = false`

Closes https://github.com/hotwired/turbo/pull/500
---
 src/observers/form_link_interceptor.ts        |  3 +-
 src/tests/functional/form_submission_tests.ts | 65 +++++++++++++++----
 2 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/src/observers/form_link_interceptor.ts b/src/observers/form_link_interceptor.ts
index 8bc7b720d..c7623fe44 100644
--- a/src/observers/form_link_interceptor.ts
+++ b/src/observers/form_link_interceptor.ts
@@ -31,8 +31,9 @@ export class FormLinkInterceptor implements LinkInterceptorDelegate {
 
   linkClickIntercepted(link: Element, action: string): void {
     const form = document.createElement("form")
+    form.setAttribute("data-turbo", "true")
     form.setAttribute("action", action)
-    form.hidden = true
+    form.setAttribute("hidden", "")
 
     const method = link.getAttribute("data-turbo-method")
     if (method) form.setAttribute("method", method)
diff --git a/src/tests/functional/form_submission_tests.ts b/src/tests/functional/form_submission_tests.ts
index fcbd8eaea..eabaa2ccd 100644
--- a/src/tests/functional/form_submission_tests.ts
+++ b/src/tests/functional/form_submission_tests.ts
@@ -803,7 +803,7 @@ test("test link method form submission inside frame", async ({ page }) => {
   await page.click("#link-method-inside-frame")
   await nextBeat()
 
-  assert.equal(await await page.textContent("#frame h2"), "Frame: Loaded")
+  assert.equal(await page.textContent("#frame h2"), "Frame: Loaded")
   assert.notOk(await hasSelector(page, "#nested-child"))
 })
 
@@ -898,6 +898,59 @@ test("test link method form submission outside frame", async ({ page }) => {
   assert.equal(await title.textContent(), "Hello")
 })
 
+test("test following a link with [data-turbo-method] set and a target set navigates the target frame", async ({
+  page,
+}) => {
+  await page.click("#turbo-method-post-to-targeted-frame")
+
+  assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")
+})
+
+test("test following a link with [data-turbo-method] and [data-turbo=true] set when html[data-turbo=false]", async ({
+  page,
+}) => {
+  const html = await page.locator("html")
+  await html.evaluate((html) => html.setAttribute("data-turbo", "false"))
+
+  const link = await page.locator("#turbo-method-post-to-targeted-frame")
+  await link.evaluate((link) => link.setAttribute("data-turbo", "true"))
+
+  await link.click()
+
+  assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page")
+  assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")
+})
+
+test("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))
+
+  const link = await page.locator("#turbo-method-post-to-targeted-frame")
+  await link.evaluate((link) => link.setAttribute("data-turbo", "true"))
+
+  await link.click()
+
+  assert.equal(await page.textContent("h1"), "Form", "does not navigate the full page")
+  assert.equal(await page.textContent("#hello h2"), "Hello from a frame", "drives the turbo-frame")
+})
+
+test("test following a link with [data-turbo-method] set when html[data-turbo=false]", async ({ page }) => {
+  const html = await page.locator("html")
+  await html.evaluate((html) => html.setAttribute("data-turbo", "false"))
+
+  await page.click("#turbo-method-post-to-targeted-frame")
+
+  assert.equal(await page.textContent("h1"), "Hello", "treats link as a full-page navigation")
+})
+
+test("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.click("#turbo-method-post-to-targeted-frame")
+
+  assert.equal(await page.textContent("h1"), "Hello", "treats link as a full-page navigation")
+})
+
 test("test stream link method form submission outside frame", async ({ page }) => {
   await page.click("#stream-link-method-outside-frame")
   await nextBeat()
@@ -976,16 +1029,6 @@ test("test POST to external action targetting frame ignored", async ({ page }) =
   assert.equal(page.url(), "https://httpbin.org/post")
 })
 
-test("test following a link with data-turbo-method set and a target set navigates the target frame", async ({
-  page,
-}) => {
-  await page.click("#turbo-method-post-to-targeted-frame")
-  await nextBeat()
-
-  const frameText = await page.locator("#hello h2")
-  assert.equal(await frameText.textContent(), "Hello from a frame")
-})
-
 function formSubmitStarted(page: Page) {
   return getFromLocalStorage(page, "formSubmitStarted")
 }