diff --git a/src/core/drive/navigator.ts b/src/core/drive/navigator.ts index e9d3c6c97..a2997ab00 100644 --- a/src/core/drive/navigator.ts +++ b/src/core/drive/navigator.ts @@ -133,8 +133,7 @@ export class Navigator { this.delegate.visitCompleted(visit) } - visitCachedSnapshot(visit: Visit) { - this.delegate.visitCachedSnapshot(visit) + visitCachedSnapshot() { } locationWithActionIsSamePage(location: URL, action?: Action): boolean { diff --git a/src/core/drive/page_renderer.ts b/src/core/drive/page_renderer.ts index 6fdb58768..a5318673b 100644 --- a/src/core/drive/page_renderer.ts +++ b/src/core/drive/page_renderer.ts @@ -11,7 +11,9 @@ export class PageRenderer extends Renderer { } async render() { - this.replaceBody() + if (this.willRender) { + this.replaceBody() + } } finishRendering() { diff --git a/src/core/drive/page_view.ts b/src/core/drive/page_view.ts index f168ac03a..62cd02ddd 100644 --- a/src/core/drive/page_view.ts +++ b/src/core/drive/page_view.ts @@ -15,8 +15,8 @@ export class PageView extends View readonly identifier = uuid() readonly restorationIdentifier: string readonly action: Action readonly referrer?: URL readonly timingMetrics: TimingMetrics = {} - readonly optionalDelegate: Partial + readonly willRender: boolean followedRedirect = false frame?: number @@ -91,14 +94,16 @@ export class Visit implements FetchRequestDelegate { this.location = location this.restorationIdentifier = restorationIdentifier || uuid() - const { action, historyChanged, referrer, snapshotHTML, response, delegate: optionalDelegate } = { ...defaultOptions, ...options } + const { action, historyChanged, referrer, snapshotHTML, response, delegate: delegateOverride, willRender } = { ...defaultOptions, ...options } this.action = action this.historyChanged = historyChanged this.referrer = referrer this.snapshotHTML = snapshotHTML this.response = response this.isSamePage = this.delegate.locationWithActionIsSamePage(this.location, this.action) - this.optionalDelegate = optionalDelegate + this.delegateOverride = delegateOverride + this.willRender = willRender + this.scrolled = !willRender } get adapter() { @@ -127,7 +132,6 @@ export class Visit implements FetchRequestDelegate { this.state = VisitState.started this.adapter.visitStarted(this) this.delegate.visitStarted(this) - if (this.optionalDelegate.visitStarted) this.optionalDelegate.visitStarted(this) } } @@ -213,7 +217,7 @@ export class Visit implements FetchRequestDelegate { this.cacheSnapshot() if (this.view.renderPromise) await this.view.renderPromise if (isSuccessful(statusCode) && responseHTML != null) { - await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML)) + await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML), false, this.willRender) this.adapter.visitRendered(this) this.complete() } else { @@ -255,7 +259,7 @@ export class Visit implements FetchRequestDelegate { this.adapter.visitRendered(this) } else { if (this.view.renderPromise) await this.view.renderPromise - await this.view.renderPage(snapshot, isPreview) + await this.view.renderPage(snapshot, isPreview, this.willRender) this.adapter.visitRendered(this) if (!isPreview) { this.complete() @@ -386,15 +390,14 @@ export class Visit implements FetchRequestDelegate { } else if (this.action == "restore") { return !this.hasCachedSnapshot() } else { - return true + return this.willRender } } cacheSnapshot() { if (!this.snapshotCached) { - this.view.cacheSnapshot() + this.view.cacheSnapshot().then(snapshot => snapshot && this.visitCachedSnapshot(snapshot)) this.snapshotCached = true - if (this.optionalDelegate.visitCachedSnapshot) this.optionalDelegate.visitCachedSnapshot(this) } } @@ -414,6 +417,14 @@ export class Visit implements FetchRequestDelegate { delete this.frame } } + + private visitCachedSnapshot(snapshot: PageSnapshot) { + if (this.delegateOverride.visitCachedSnapshot) { + this.delegateOverride.visitCachedSnapshot(snapshot) + } else { + this.delegate.visitCachedSnapshot(snapshot) + } + } } function isSuccessful(statusCode: number) { diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 53a585331..d1e2e07d8 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -4,7 +4,8 @@ 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, VisitDelegate } from "../drive/visit" +import { VisitDelegate } from "../drive/visit" +import { PageSnapshot } from "../drive/page_snapshot" import { Snapshot } from "../snapshot" import { ViewDelegate } from "../view" import { getAction, expandURL, urlsAreEqual, locationIsVisitable, Locatable } from "../url" @@ -23,6 +24,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest readonly formInterceptor: FormInterceptor currentURL?: string | null formSubmission?: FormSubmission + fetchResponseLoaded = (fetchResponse: FetchResponse) => {} private currentFetchRequest: FetchRequest | null = null private resolveVisitPromise = () => {} private connected = false @@ -108,15 +110,18 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest if (html) { const { body } = parseHTMLDocument(html) const snapshot = new Snapshot(await this.extractForeignFrameElement(body)) - const renderer = new FrameRenderer(this.view.snapshot, snapshot, false) + const renderer = new FrameRenderer(this.view.snapshot, snapshot, false, false) if (this.view.renderPromise) await this.view.renderPromise await this.view.render(renderer) session.frameRendered(fetchResponse, this.element) session.frameLoaded(this.element) + this.fetchResponseLoaded(fetchResponse) } } catch (error) { console.error(error) this.view.invalidate() + } finally { + this.fetchResponseLoaded = () => {} } } @@ -261,19 +266,16 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest const action = getAttribute("data-turbo-action", submitter, element, frame) if (isAction(action)) { - const delegate = new SnapshotSubstitution(frame) - const proposeVisit = (event: Event) => { - const { target, detail: { fetchResponse } } = event as CustomEvent - if (target instanceof FrameElement && target.src) { + const delegate = new SnapshotSubstitution(this.view.snapshot) + frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => { + if (frame.src) { const { statusCode, redirected } = fetchResponse - const responseHTML = target.ownerDocument.documentElement.outerHTML + const responseHTML = frame.ownerDocument.documentElement.outerHTML const response = { statusCode, redirected, responseHTML } - session.visit(target.src, { action, response, delegate }) + session.visit(frame.src, { action, response, delegate, willRender: false }) } } - - frame.addEventListener("turbo:frame-render", proposeVisit , { once: true }) } } @@ -396,24 +398,19 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest } class SnapshotSubstitution implements Partial { - private readonly clone: Node + private readonly substitution: Node private readonly id: string - private snapshot?: Snapshot - constructor(element: FrameElement) { - this.clone = element.cloneNode(true) + constructor({ element }: Snapshot) { + this.substitution = element.cloneNode(true) this.id = element.id } - visitStarted(visit: Visit) { - this.snapshot = visit.view.snapshot - } - - visitCachedSnapshot() { - const { snapshot, id, clone } = this + visitCachedSnapshot(snapshot: PageSnapshot) { + const newFrame = snapshot.element.querySelector("#" + this.id) - if (snapshot) { - snapshot.element.querySelector("#" + id)?.replaceWith(clone) + if (newFrame) { + newFrame.replaceWith(this.substitution) } } } diff --git a/src/core/renderer.ts b/src/core/renderer.ts index 981bd7676..7b5def56a 100644 --- a/src/core/renderer.ts +++ b/src/core/renderer.ts @@ -10,13 +10,15 @@ export abstract class Renderer = Snapsh readonly currentSnapshot: S readonly newSnapshot: S readonly isPreview: boolean + readonly willRender: boolean readonly promise: Promise private resolvingFunctions?: ResolvingFunctions - constructor(currentSnapshot: S, newSnapshot: S, isPreview: boolean) { + constructor(currentSnapshot: S, newSnapshot: S, isPreview: boolean, willRender = true) { this.currentSnapshot = currentSnapshot this.newSnapshot = newSnapshot this.isPreview = isPreview + this.willRender = willRender this.promise = new Promise((resolve, reject) => this.resolvingFunctions = { resolve, reject }) } diff --git a/src/core/session.ts b/src/core/session.ts index 70bd16133..6f5d46fd0 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -189,7 +189,7 @@ export class Session implements FormSubmitObserverDelegate, HistoryDelegate, Lin this.notifyApplicationAfterPageLoad(visit.getTimingMetrics()) } - visitCachedSnapshot(visit: Visit) { + visitCachedSnapshot() { } locationWithActionIsSamePage(location: URL, action?: Action): boolean { diff --git a/src/elements/frame_element.ts b/src/elements/frame_element.ts index bfde5d66c..94f4ac60b 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -11,6 +11,7 @@ export interface FrameElementDelegate { formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement): void linkClickIntercepted(element: Element, url: string): void loadResponse(response: FetchResponse): void + fetchResponseLoaded: (fetchResponse: FetchResponse) => void isLoading: boolean } diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html index acfc17257..e36604a0f 100644 --- a/src/tests/fixtures/frames.html +++ b/src/tests/fixtures/frames.html @@ -1,5 +1,5 @@ - + Frame @@ -9,9 +9,14 @@ addEventListener("click", ({ target }) => { if (target.id == "add-turbo-action-to-frame") { target.closest("turbo-frame")?.setAttribute("data-turbo-action", "advance") + } else if (target.id == "remove-target-from-hello") { + document.getElementById("hello").removeAttribute("target") } }) +

Frames

@@ -45,8 +50,12 @@

Frames: #frame

Frames: #hello

Load #frame + + + advance #hello +

Frames: #nested-root

Inner/Outer frame link @@ -104,5 +113,9 @@

Frames: #nested-child

+ +
+ + Navigate #frame diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js index 0cc3ea174..66a6871e4 100644 --- a/src/tests/fixtures/test.js +++ b/src/tests/fixtures/test.js @@ -7,7 +7,9 @@ } function eventListener(event) { - eventLogs.push([event.type, event.detail, event.target.id]) + const skipped = document.documentElement.getAttribute("data-skip-event-details") || "" + + eventLogs.push([event.type, skipped.includes(event.type) ? {} : event.detail, event.target.id]) } window.mutationLogs = [] diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index d853021da..7786744e3 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -288,13 +288,13 @@ export class FrameTests extends TurboDriveTestCase { this.assert.equal(await this.nextAttributeMutationNamed("frame", "aria-busy"), "true", "sets aria-busy on the ") 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 ") this.assert.equal(await this.nextAttributeMutationNamed("html", "aria-busy"), "true", "sets aria-busy on the ") + await this.nextEventOnTarget("html", "turbo:before-visit") + await this.nextEventOnTarget("html", "turbo:visit") await this.nextEventOnTarget("html", "turbo:before-cache") await this.nextEventOnTarget("html", "turbo:before-render") await this.nextEventOnTarget("html", "turbo:render") @@ -305,7 +305,34 @@ export class FrameTests extends TurboDriveTestCase { 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") - await this.nextBeat + await this.nextEventNamed("turbo:load") + + const title = await this.querySelector("h1") + const frameTitle = await this.querySelector("#frame h2") + + this.assert.equal(await title.getVisibleText(), "Frames") + this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded") + this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") + } + + async "test navigating turbo-frame[data-turbo-action=advance] to the same URL clears the [aria-busy] and [data-turbo-preview] state"() { + await this.clickSelector("#link-outside-frame-action-advance") + await this.nextEventNamed("turbo:load") + await this.clickSelector("#link-outside-frame-action-advance") + await this.nextEventNamed("turbo:load") + await this.clickSelector("#link-outside-frame-action-advance") + await this.nextEventNamed("turbo:load") + + this.assert.equal(await this.attributeForSelector("#frame", "aria-busy"), null, "clears turbo-frame[aria-busy]") + this.assert.equal(await this.attributeForSelector("#html", "aria-busy"), null, "clears html[aria-busy]") + this.assert.equal(await this.attributeForSelector("#html", "data-turbo-preview"), null, "clears html[aria-busy]") + } + + async "test navigating a turbo-frame with an a[data-turbo-action=advance] preserves page state"() { + await this.scrollToSelector("#below-the-fold-input") + await this.fillInSelector("#below-the-fold-input", "a value") + await this.clickSelector("#below-the-fold-link-frame-action") + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -313,11 +340,33 @@ export class FrameTests extends TurboDriveTestCase { this.assert.equal(await title.getVisibleText(), "Frames") this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded") this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") + this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html") + this.assert.equal(await this.propertyForSelector("#below-the-fold-input", "value"), "a value", "preserves page state") + + const { y } = await this.scrollPosition + this.assert.notEqual(y, 0, "preserves Y scroll position") + } + + async "test a turbo-frame that has been driven by a[data-turbo-action] can be navigated normally"() { + await this.clickSelector("#remove-target-from-hello") + await this.clickSelector("#link-hello-advance") + await this.nextEventNamed("turbo:load") + + this.assert.equal(await (await this.querySelector("h1")).getVisibleText(), "Frames") + this.assert.equal(await (await this.querySelector("#hello h2")).getVisibleText(), "Hello from a frame") + this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/hello.html") + + await this.clickSelector("#hello a") + await this.nextEventOnTarget("hello", "turbo:frame-load") + await this.noNextEventNamed("turbo:load") + + this.assert.equal(await (await this.querySelector("#hello h2")).getVisibleText(), "Frames: #hello") + this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/hello.html") } async "test navigating turbo-frame from within with a[data-turbo-action=advance] pushes URL state"() { await this.clickSelector("#link-nested-frame-action-advance") - await this.nextBeat + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -329,7 +378,7 @@ export class FrameTests extends TurboDriveTestCase { async "test navigating frame with a[data-turbo-action=advance] pushes URL state"() { await this.clickSelector("#link-outside-frame-action-advance") - await this.nextBeat + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -341,7 +390,7 @@ export class FrameTests extends TurboDriveTestCase { async "test navigating frame with form[method=get][data-turbo-action=advance] pushes URL state"() { await this.clickSelector("#form-get-frame-action-advance button") - await this.nextBeat + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -351,9 +400,22 @@ export class FrameTests extends TurboDriveTestCase { this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") } + async "test navigating frame with form[method=get][data-turbo-action=advance] to the same URL clears the [aria-busy] and [data-turbo-preview] state"() { + await this.clickSelector("#form-get-frame-action-advance button") + await this.nextEventNamed("turbo:load") + await this.clickSelector("#form-get-frame-action-advance button") + await this.nextEventNamed("turbo:load") + await this.clickSelector("#form-get-frame-action-advance button") + await this.nextEventNamed("turbo:load") + + this.assert.equal(await this.attributeForSelector("#frame", "aria-busy"), null, "clears turbo-frame[aria-busy]") + this.assert.equal(await this.attributeForSelector("#html", "aria-busy"), null, "clears html[aria-busy]") + this.assert.equal(await this.attributeForSelector("#html", "data-turbo-preview"), null, "clears html[aria-busy]") + } + async "test navigating frame with form[method=post][data-turbo-action=advance] pushes URL state"() { await this.clickSelector("#form-post-frame-action-advance button") - await this.nextBeat + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -363,9 +425,22 @@ export class FrameTests extends TurboDriveTestCase { this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") } + async "test navigating frame with form[method=post][data-turbo-action=advance] to the same URL clears the [aria-busy] and [data-turbo-preview] state"() { + await this.clickSelector("#form-post-frame-action-advance button") + await this.nextEventNamed("turbo:load") + await this.clickSelector("#form-post-frame-action-advance button") + await this.nextEventNamed("turbo:load") + await this.clickSelector("#form-post-frame-action-advance button") + await this.nextEventNamed("turbo:load") + + this.assert.equal(await this.attributeForSelector("#frame", "aria-busy"), null, "clears turbo-frame[aria-busy]") + this.assert.equal(await this.attributeForSelector("#html", "aria-busy"), null, "clears html[aria-busy]") + this.assert.equal(await this.attributeForSelector("#html", "data-turbo-preview"), null, "clears html[aria-busy]") + } + async "test navigating frame with button[data-turbo-action=advance] pushes URL state"() { await this.clickSelector("#button-frame-action-advance") - await this.nextBeat + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -380,7 +455,7 @@ export class FrameTests extends TurboDriveTestCase { await this.clickSelector("#link-frame") await this.nextEventNamed("turbo:load") await this.goBack() - await this.nextBody + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -395,9 +470,9 @@ export class FrameTests extends TurboDriveTestCase { await this.clickSelector("#link-frame") await this.nextEventNamed("turbo:load") await this.goBack() - await this.nextBody + await this.nextEventNamed("turbo:load") await this.goForward() - await this.nextBody + await this.nextEventNamed("turbo:load") const title = await this.querySelector("h1") const frameTitle = await this.querySelector("#frame h2") @@ -417,6 +492,20 @@ export class FrameTests extends TurboDriveTestCase { this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-response")) } + async getFrameSrc(id: string): Promise { + const src = await this.propertyForSelector("#" + id, "src") + + return new URL(src).pathname + } + + async fillInSelector(selector: string, value: string) { + const element = await this.querySelector(selector) + + await element.click() + + return element.type(value) + } + get frameScriptEvaluationCount(): Promise { return this.evaluate(() => window.frameScriptEvaluationCount) }