Fix: Dispatch turbo:click when driving a Frame

Closes hotwired#726

Prior to this commit, clicking on `<a>` elements nested within
`<turbo-frame>` elements, or `<a>` elements that drive `<turbo-frame>`
elements did not dispatch `turbo:click` events in the same way that they
did before [hotwired#412][].

This commit re-instates those events as part of the `FrameController`
and `FrameRedirector` implementations for the `willFollowLinkToLocation`
methods they define as part of the `LinkClickObserverDelegate`
interface.

To be consistent with the existing `turbo:click` dispatch behavior, and
to guard against introducing similar regressions in the future, this
commit also adds test coverage for falling back to page-wide navigations
when `turbo:click` events are canceled.

In support of those changes, first, change the `cancelNextVisit`
signature to accept the name of a Turbo event that is cancellable (in
this case, `turbo:click` and `turbo:before-visit`). Next, change all the
call sites. Finally, extract it to the shared `helpers/page` utility
module for re-use elsewhere.

Next, use the `cancelNextVisit` helper in the Frame test coverage to
ensure that canceling a `turbo:click` prevents navigating the Frame and
falls back to built-in browser behavior.

[hotwired#412]: hotwired#412

src/core/frames/frame_controller.ts

@@ -26,7 +26,7 @@ import { FrameView } from "./frame_view"
 import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer"
 import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../observers/form_link_click_observer"
 import { FrameRenderer } from "./frame_renderer"
-import { session } from "../index"
+import { TurboClickEvent, session } from "../index"
 import { isAction, Action } from "../types"
 import { VisitOptions } from "../drive/visit"
 import { TurboBeforeFrameRenderEvent } from "../session"
@@ -204,8 +204,8 @@ export class FrameController
 
   // Link click observer delegate
 
-  willFollowLinkToLocation(element: Element) {
-    return this.shouldInterceptNavigation(element)
+  willFollowLinkToLocation(element: Element, location: URL, event: MouseEvent) {
+    return this.shouldInterceptNavigation(element) && this.frameAllowsVisitingLocation(element, location, event)
   }
 
   followedLinkToLocation(element: Element, location: URL) {
@@ -545,6 +545,16 @@ export class FrameController
     return expandURL(root)
   }
 
+  private frameAllowsVisitingLocation(target: Element, { href: url }: URL, originalEvent: MouseEvent): boolean {
+    const event = dispatch<TurboClickEvent>("turbo:click", {
+      target,
+      detail: { url, originalEvent },
+      cancelable: true,
+    })
+
+    return !event.defaultPrevented
+  }
+
   private isIgnoringChangesTo(attributeName: FrameElementObservedAttribute): boolean {
     return this.ignoredAttributes.has(attributeName)
   }

src/core/frames/frame_redirector.ts

@@ -2,7 +2,8 @@ import { FormSubmitObserver, FormSubmitObserverDelegate } from "../../observers/
 import { FrameElement } from "../../elements/frame_element"
 import { expandURL, getAction, locationIsVisitable } from "../url"
 import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer"
-import { Session } from "../session"
+import { Session, TurboClickEvent } from "../session"
+import { dispatch } from "../../util"
 
 export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObserverDelegate {
   readonly session: Session
@@ -27,8 +28,8 @@ export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObs
     this.formSubmitObserver.stop()
   }
 
-  willFollowLinkToLocation(element: Element) {
-    return this.shouldRedirect(element)
+  willFollowLinkToLocation(element: Element, location: URL, event: MouseEvent) {
+    return this.shouldRedirect(element) && this.frameAllowsVisitingLocation(element, location, event)
   }
 
   followedLinkToLocation(element: Element, url: URL) {
@@ -53,6 +54,16 @@ export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObs
     }
   }
 
+  private frameAllowsVisitingLocation(target: Element, { href: url }: URL, originalEvent: MouseEvent): boolean {
+    const event = dispatch<TurboClickEvent>("turbo:click", {
+      target,
+      detail: { url, originalEvent },
+      cancelable: true,
+    })
+
+    return !event.defaultPrevented
+  }
+
   private shouldSubmit(form: HTMLFormElement, submitter?: HTMLElement) {
     const action = getAction(form, submitter)
     const meta = this.element.ownerDocument.querySelector<HTMLMetaElement>(`meta[name="turbo-root"]`)

src/tests/fixtures/frames.html

@@ -88,6 +88,7 @@ <h2>Frames: #nested-child</h2>
       <a href="/src/tests/fixtures/one.html?key=value">Visit one.html?key=value</a>
       <a href="/src/tests/fixtures/one.html" data-turbo-frame="_self">Visit self</a>
     </turbo-frame>
+    <a id="outside-navigate-top-link" href="/src/tests/fixtures/one.html">Visit one.html from outside #navigate-top</a>
 
     <turbo-frame id="missing">
       <a id="missing-frame-link" href="/src/tests/fixtures/frames/frame.html">Missing frame</a>

src/tests/fixtures/test.js

@@ -47,6 +47,7 @@
      }
    }).observe(document, { subtree: true, childList: true, attributes: true })
 })([
+  "turbo:click",
   "turbo:before-stream-render",
   "turbo:before-cache",
   "turbo:before-render",

src/tests/functional/frame_tests.ts

@@ -2,6 +2,7 @@ import { Page, test } from "@playwright/test"
 import { assert, Assertion } from "chai"
 import {
   attributeForSelector,
+  cancelNextVisit,
   hasSelector,
   innerHTMLForSelector,
   nextAttributeMutationNamed,
@@ -401,9 +402,26 @@ test("test 'turbo:frame-render' is triggered after frame has finished rendering"
   assert.include(fetchResponse.response.url, "/src/tests/fixtures/frames/part.html")
 })
 
-test("test navigating a frame fires events", async ({ page }) => {
+test("test navigating a frame from an outer form fires events", async ({ page }) => {
   await page.click("#outside-frame-form")
 
+  await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
+  await nextEventOnTarget(page, "frame", "turbo:before-fetch-response")
+  const { fetchResponse } = await nextEventOnTarget(page, "frame", "turbo:frame-render")
+  assert.include(fetchResponse.response.url, "/src/tests/fixtures/frames/form.html")
+
+  await nextEventOnTarget(page, "frame", "turbo:frame-load")
+
+  const otherEvents = await readEventLogs(page)
+  assert.equal(otherEvents.length, 0, "no more events")
+})
+
+test("test navigating a frame from an outer link fires events", async ({ page }) => {
+  await page.click("#outside-frame-form")
+
+  await nextEventOnTarget(page, "outside-frame-form", "turbo:click")
+  await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
+  await nextEventOnTarget(page, "frame", "turbo:before-fetch-response")
   const { fetchResponse } = await nextEventOnTarget(page, "frame", "turbo:frame-render")
   assert.include(fetchResponse.response.url, "/src/tests/fixtures/frames/form.html")
 
@@ -413,6 +431,42 @@ test("test navigating a frame fires events", async ({ page }) => {
   assert.equal(otherEvents.length, 0, "no more events")
 })
 
+test("test canceling a turbo:cilck event falls back to built-in browser navigation", async ({ page }) => {
+  await cancelNextVisit(page, "turbo:click")
+  await Promise.all([page.waitForNavigation(), page.click("#link-frame")])
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.html")
+})
+
+test("test navigating a frame from an inner link fires events", async ({ page }) => {
+  await page.click("#link-frame")
+
+  await nextEventOnTarget(page, "link-frame", "turbo:click")
+  await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
+  await nextEventOnTarget(page, "frame", "turbo:before-fetch-response")
+  const { fetchResponse } = await nextEventOnTarget(page, "frame", "turbo:frame-render")
+  assert.include(fetchResponse.response.url, "/src/tests/fixtures/frames/frame.html")
+
+  await nextEventOnTarget(page, "frame", "turbo:frame-load")
+
+  const otherEvents = await readEventLogs(page)
+  assert.equal(otherEvents.length, 0, "no more events")
+})
+
+test("test navigating a frame targeting _top from an outer link fires events", async ({ page }) => {
+  await page.click("#outside-navigate-top-link")
+
+  await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:click")
+  await nextEventOnTarget(page, "html", "turbo:before-fetch-request")
+  await nextEventOnTarget(page, "html", "turbo:before-fetch-response")
+  await nextEventOnTarget(page, "html", "turbo:before-render")
+  await nextEventOnTarget(page, "html", "turbo:render")
+  await nextEventOnTarget(page, "html", "turbo:load")
+
+  const otherEvents = await readEventLogs(page)
+  assert.equal(otherEvents.length, 0, "no more events")
+})
+
 test("test following inner link reloads frame on every click", async ({ page }) => {
   await page.click("#hello a")
   await nextEventNamed(page, "turbo:before-fetch-request")

src/tests/functional/visit_tests.ts

@@ -2,12 +2,14 @@ import { Page, test } from "@playwright/test"
 import { assert } from "chai"
 import { get } from "http"
 import {
+  cancelNextVisit,
   getSearchParam,
   isScrolledToSelector,
   isScrolledToTop,
   nextBeat,
   nextEventNamed,
   noNextAttributeMutationNamed,
+  pathname,
   readEventLogs,
   scrollToSelector,
   visitAction,
@@ -62,8 +64,15 @@ test("test visiting a location served with a non-HTML content type", async ({ pa
   assert.equal(await visitAction(page), "load")
 })
 
+test("test canceling a turbo:click event falls back to built-in browser navigation", async ({ page }) => {
+  await cancelNextVisit(page, "turbo:click")
+  await Promise.all([page.waitForNavigation(), page.click("#same-origin-link")])
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
+})
+
 test("test canceling a before-visit event prevents navigation", async ({ page }) => {
-  await cancelNextVisit(page)
+  await cancelNextVisit(page, "turbo:before-visit")
   const urlBeforeVisit = page.url()
 
   assert.notOk<boolean>(
@@ -83,7 +92,7 @@ test("test navigation by history is not cancelable", async ({ page }) => {
 
   assert.equal(await page.textContent("h1"), "One")
 
-  await cancelNextVisit(page)
+  await cancelNextVisit(page, "turbo:before-visit")
   await page.goBack()
   await nextEventNamed(page, "turbo:load")
 
@@ -163,10 +172,6 @@ test("test cache does not override response after redirect", async ({ page }) =>
   assert.equal(await page.locator("some-cached-element").count(), 0)
 })
 
-function cancelNextVisit(page: Page): Promise<void> {
-  return page.evaluate(() => addEventListener("turbo:before-visit", (event) => event.preventDefault(), { once: true }))
-}
-
 function contentTypeOfURL(url: string): Promise<string | undefined> {
   return new Promise((resolve) => {
     get(url, ({ headers }) => resolve(headers["content-type"]))

src/tests/helpers/page.ts

@@ -14,6 +14,15 @@ export function attributeForSelector(page: Page, selector: string, attributeName
   return page.locator(selector).getAttribute(attributeName)
 }
 
+type CancellableEvent = "turbo:click" | "turbo:before-visit"
+
+export function cancelNextVisit(page: Page, eventName: CancellableEvent): Promise<void> {
+  return page.evaluate(
+    (eventName) => addEventListener(eventName, (event) => event.preventDefault(), { once: true }),
+    eventName
+  )
+}
+
 export function clickWithoutScrolling(page: Page, selector: string, options = {}) {
   const element = page.locator(selector, options)