Resolve Frame-to-Page Visit event ordering

src/core/drive/visit.ts

@@ -41,7 +41,6 @@ export type VisitOptions = {
   action: Action,
   delegate: Partial<VisitDelegate>
   historyChanged: boolean,
-  willRender: boolean
   referrer?: URL,
   snapshotHTML?: string,
   response?: VisitResponse
@@ -51,7 +50,6 @@ const defaultOptions: VisitOptions = {
   action: "advance",
   delegate: {},
   historyChanged: false,
-  willRender: true
 }
 
 export type VisitResponse = {
@@ -75,7 +73,6 @@ export class Visit implements FetchRequestDelegate {
   readonly timingMetrics: TimingMetrics = {}
   readonly optionalDelegate: Partial<VisitDelegate>
 
-  willRender: boolean
   followedRedirect = false
   frame?: number
   historyChanged = false
@@ -94,8 +91,7 @@ export class Visit implements FetchRequestDelegate {
     this.location = location
     this.restorationIdentifier = restorationIdentifier || uuid()
 
-    const { action, historyChanged, referrer, snapshotHTML, response, willRender, delegate: optionalDelegate } = { ...defaultOptions, ...options }
-    this.willRender = willRender
+    const { action, historyChanged, referrer, snapshotHTML, response, delegate: optionalDelegate } = { ...defaultOptions, ...options }
     this.action = action
     this.historyChanged = historyChanged
     this.referrer = referrer
@@ -211,7 +207,7 @@ export class Visit implements FetchRequestDelegate {
   }
 
   loadResponse() {
-    if (this.response && this.willRender) {
+    if (this.response) {
       const { statusCode, responseHTML } = this.response
       this.render(async () => {
         this.cacheSnapshot()

src/core/frames/frame_controller.ts

@@ -4,7 +4,7 @@ import { FetchResponse } from "../../http/fetch_response"
 import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
 import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy } from "../../util"
 import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
-import { Visit } from "../drive/visit"
+import { Visit, VisitDelegate } from "../drive/visit"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
 import { getAction, expandURL, urlsAreEqual, locationIsVisitable, Locatable } from "../url"
@@ -261,23 +261,15 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
     const action = getAttribute("data-turbo-action", submitter, element, frame)
 
     if (isAction(action)) {
-      const clone = frame.cloneNode(true)
-      const proposeVisit = () => {
-        const { ownerDocument, id, src } = frame
-        if (src) {
-          const snapshotHTML = ownerDocument.documentElement.outerHTML
-          let snapshot: Snapshot
-
-          const delegate = {
-            visitStarted(visit: Visit) {
-              snapshot = visit.view.snapshot
-            },
-            visitCachedSnapshot() {
-              snapshot.element.querySelector("#" + id)?.replaceWith(clone)
-            }
-          }
-
-          session.visit(src, { willRender: false, action, snapshotHTML, delegate })
+      const delegate = new SnapshotSubstitution(frame)
+      const proposeVisit = (event: Event) => {
+        const { target, detail: { fetchResponse } } = event as CustomEvent
+        if (target instanceof FrameElement && target.src) {
+          const { statusCode, redirected } = fetchResponse
+          const responseHTML = target.ownerDocument.documentElement.outerHTML
+          const response = { statusCode, redirected, responseHTML }
+
+          session.visit(target.src, { action, response, delegate })
         }
       }
 
@@ -403,6 +395,29 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   }
 }
 
+class SnapshotSubstitution implements Partial<VisitDelegate> {
+  private readonly clone: Node
+  private readonly id: string
+  private snapshot?: Snapshot
+
+  constructor(element: FrameElement) {
+    this.clone = element.cloneNode(true)
+    this.id = element.id
+  }
+
+  visitStarted(visit: Visit) {
+    this.snapshot = visit.view.snapshot
+  }
+
+  visitCachedSnapshot() {
+    const { snapshot, id, clone } = this
+
+    if (snapshot) {
+      snapshot.element.querySelector("#" + id)?.replaceWith(clone)
+    }
+  }
+}
+
 function getFrameElementById(id: string | null) {
   if (id != null) {
     const element = document.getElementById(id)

src/tests/functional/form_submission_tests.ts

@@ -584,14 +584,14 @@ export class FormSubmissionTests extends TurboDriveTestCase {
     await this.clickSelector('#dialog-formmethod-turbo-frame [formmethod="dialog"]')
     await this.nextBeat
 
-    this.assert.notOk(await this.formSubmitted)
+    this.assert.notOk(await this.formSubmitEnded)
   }
 
   async "test form submission targetting frame skipped within method=dialog"() {
     await this.clickSelector('#dialog-method-turbo-frame button')
     await this.nextBeat
 
-    this.assert.notOk(await this.formSubmitted)
+    this.assert.notOk(await this.formSubmitEnded)
   }
 
   async "test form submission targetting frame skipped with submitter formmethod=dialog"() {

src/tests/functional/frame_tests.ts

@@ -282,6 +282,26 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.equal(requestLogs.length, 0)
   }
 
+  async "test navigating pushing URL state from a frame navigation fires events"() {
+    await this.clickSelector("#link-outside-frame-action-advance")
+
+    this.assert.equal(await this.nextAttributeMutationNamed("frame", "aria-busy"), "true", "sets aria-busy on the <turbo-frame>")
+    await this.nextEventOnTarget("frame", "turbo:before-fetch-request")
+    await this.nextEventOnTarget("frame", "turbo:before-fetch-response")
+    await this.nextEventOnTarget("html", "turbo:before-visit")
+    await this.nextEventOnTarget("html", "turbo:visit")
+    await this.nextEventOnTarget("frame", "turbo:frame-render")
+    await this.nextEventOnTarget("frame", "turbo:frame-load")
+    this.assert.notOk(await this.nextAttributeMutationNamed("frame", "aria-busy"), "removes aria-busy from the <turbo-frame>")
+
+    this.assert.equal(await this.nextAttributeMutationNamed("html", "aria-busy"), "true", "sets aria-busy on the <html>")
+    await this.nextEventOnTarget("html", "turbo:before-cache")
+    await this.nextEventOnTarget("html", "turbo:before-render")
+    await this.nextEventOnTarget("html", "turbo:render")
+    await this.nextEventOnTarget("html", "turbo:load")
+    this.assert.notOk(await this.nextAttributeMutationNamed("html", "aria-busy"), "removes aria-busy from the <html>")
+  }
+
   async "test navigating turbo-frame[data-turbo-action=advance] from within pushes URL state"() {
     await this.clickSelector("#add-turbo-action-to-frame")
     await this.clickSelector("#link-frame")
@@ -358,7 +378,7 @@ export class FrameTests extends TurboDriveTestCase {
   async "test navigating back after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames previous contents"() {
     await this.clickSelector("#add-turbo-action-to-frame")
     await this.clickSelector("#link-frame")
-    await this.nextBody
+    await this.nextEventNamed("turbo:load")
     await this.goBack()
     await this.nextBody
 
@@ -373,7 +393,7 @@ export class FrameTests extends TurboDriveTestCase {
   async "test navigating back then forward after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames next contents"() {
     await this.clickSelector("#add-turbo-action-to-frame")
     await this.clickSelector("#link-frame")
-    await this.nextBody
+    await this.nextEventNamed("turbo:load")
     await this.goBack()
     await this.nextBody
     await this.goForward()