diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index ce98a5fae..5ce1aad57 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -1,4 +1,4 @@ -import { FrameElement, FrameElementDelegate, FrameLoadingStyle } from "../../elements/frame_element" +import { FrameElement, FrameElementDelegate, FrameLoadingStyle, FrameElementObservedAttribute } from "../../elements/frame_element" import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request" import { FetchResponse } from "../../http/fetch_response" import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer" @@ -14,20 +14,21 @@ import { FrameRenderer } from "./frame_renderer" import { session } from "../index" import { isAction } from "../types" +type ObservedAttribute = keyof FrameElement & FrameElementObservedAttribute + export class FrameController implements AppearanceObserverDelegate, FetchRequestDelegate, FormInterceptorDelegate, FormSubmissionDelegate, FrameElementDelegate, LinkInterceptorDelegate, ViewDelegate> { readonly element: FrameElement readonly view: FrameView readonly appearanceObserver: AppearanceObserver readonly linkInterceptor: LinkInterceptor readonly formInterceptor: FormInterceptor - currentURL?: string | null formSubmission?: FormSubmission fetchResponseLoaded = (fetchResponse: FetchResponse) => {} private currentFetchRequest: FetchRequest | null = null private resolveVisitPromise = () => {} private connected = false private hasBeenLoaded = false - private settingSourceURL = false + private ignoredAttributes: Set = new Set constructor(element: FrameElement) { this.element = element @@ -40,13 +41,13 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest connect() { if (!this.connected) { this.connected = true - this.reloadable = false + this.linkInterceptor.start() + this.formInterceptor.start() if (this.loadingStyle == FrameLoadingStyle.lazy) { this.appearanceObserver.start() + } else { + this.loadSourceURL() } - this.linkInterceptor.start() - this.formInterceptor.start() - this.sourceURLChanged() } } @@ -66,11 +67,23 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest } sourceURLChanged() { + if (this.isIgnoringChangesTo("src")) return + + if (this.element.isConnected) { + this.loaded = false + } + if (this.loadingStyle == FrameLoadingStyle.eager || this.hasBeenLoaded) { this.loadSourceURL() } } + loadedChanged() { + if (this.isIgnoringChangesTo("loaded")) return + + this.loadSourceURL() + } + loadingStyleChanged() { if (this.loadingStyle == FrameLoadingStyle.lazy) { this.appearanceObserver.start() @@ -80,21 +93,12 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest } } - async loadSourceURL() { - if (!this.settingSourceURL && this.enabled && this.isActive && (this.reloadable || this.sourceURL != this.currentURL)) { - const previousURL = this.currentURL - this.currentURL = this.sourceURL - if (this.sourceURL) { - try { - this.element.loaded = this.visit(expandURL(this.sourceURL)) - this.appearanceObserver.stop() - await this.element.loaded - this.hasBeenLoaded = true - } catch (error) { - this.currentURL = previousURL - throw error - } - } + private async loadSourceURL() { + if (this.enabled && this.isActive && !this.loaded && this.sourceURL) { + this.element.loaded = this.visit(expandURL(this.sourceURL)) + this.appearanceObserver.stop() + await this.element.loaded + this.hasBeenLoaded = true } } @@ -111,6 +115,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest const renderer = new FrameRenderer(this.view.snapshot, snapshot, false, false) if (this.view.renderPromise) await this.view.renderPromise await this.view.render(renderer) + this.loaded = true session.frameRendered(fetchResponse, this.element) session.frameLoaded(this.element) this.fetchResponseLoaded(fetchResponse) @@ -140,7 +145,6 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest } linkClickIntercepted(element: Element, url: string) { - this.reloadable = true this.navigateFrame(element, url) } @@ -155,7 +159,6 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest this.formSubmission.stop() } - this.reloadable = false this.formSubmission = new FormSubmission(this, element, submitter) const { fetchRequest } = this.formSubmission this.prepareHeadersForRequest(fetchRequest.headers, fetchRequest) @@ -256,7 +259,6 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest this.proposeVisitIfNavigatedWithAction(frame, element, submitter) - frame.setAttribute("reloadable", "") frame.src = url } @@ -287,11 +289,11 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest const id = CSS.escape(this.id) try { - if (element = activateElement(container.querySelector(`turbo-frame#${id}`), this.currentURL)) { + if (element = activateElement(container.querySelector(`turbo-frame#${id}`), this.sourceURL)) { return element } - if (element = activateElement(container.querySelector(`turbo-frame[src][recurse~=${id}]`), this.currentURL)) { + if (element = activateElement(container.querySelector(`turbo-frame[src][recurse~=${id}]`), this.sourceURL)) { await element.loaded return await this.extractForeignFrameElement(element) } @@ -355,25 +357,10 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest } } - get reloadable() { - const frame = this.findFrameElement(this.element) - return frame.hasAttribute("reloadable") - } - - set reloadable(value: boolean) { - const frame = this.findFrameElement(this.element) - if (value) { - frame.setAttribute("reloadable", "") - } else { - frame.removeAttribute("reloadable") - } - } - set sourceURL(sourceURL: string | undefined) { - this.settingSourceURL = true - this.element.src = sourceURL ?? null - this.currentURL = this.element.src - this.settingSourceURL = false + this.ignoringChangesToAttribute("src", () => { + this.element.src = sourceURL ?? null + }) } get loadingStyle() { @@ -384,6 +371,20 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest return this.formSubmission !== undefined || this.resolveVisitPromise() !== undefined } + get loaded() { + return this.element.hasAttribute("loaded") + } + + set loaded(value: boolean) { + this.ignoringChangesToAttribute("loaded", () => { + if (value) { + this.element.setAttribute("loaded", "") + } else { + this.element.removeAttribute("loaded") + } + }) + } + get isActive() { return this.element.isActive && this.connected } @@ -393,6 +394,16 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest const root = meta?.content ?? "/" return expandURL(root) } + + private isIgnoringChangesTo(attributeName: ObservedAttribute): boolean { + return this.ignoredAttributes.has(attributeName) + } + + private ignoringChangesToAttribute(attributeName: ObservedAttribute, callback: () => void) { + this.ignoredAttributes.add(attributeName) + callback() + this.ignoredAttributes.delete(attributeName) + } } class SnapshotSubstitution { diff --git a/src/core/frames/frame_redirector.ts b/src/core/frames/frame_redirector.ts index c300c43cb..0ab589341 100644 --- a/src/core/frames/frame_redirector.ts +++ b/src/core/frames/frame_redirector.ts @@ -42,7 +42,6 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptor formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement) { const frame = this.findFrameElement(element, submitter) if (frame) { - frame.removeAttribute("reloadable") frame.delegate.formSubmissionIntercepted(element, submitter) } } diff --git a/src/elements/frame_element.ts b/src/elements/frame_element.ts index 94f4ac60b..581d0b67d 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -2,9 +2,12 @@ import { FetchResponse } from "../http/fetch_response" export enum FrameLoadingStyle { eager = "eager", lazy = "lazy" } +export type FrameElementObservedAttribute = "disabled" | "loaded" | "loading" | "src" + export interface FrameElementDelegate { connect(): void disconnect(): void + loadedChanged(): void loadingStyleChanged(): void sourceURLChanged(): void disabledChanged(): void @@ -37,8 +40,8 @@ export class FrameElement extends HTMLElement { loaded: Promise = Promise.resolve() readonly delegate: FrameElementDelegate - static get observedAttributes() { - return ["disabled", "loading", "src"] + static get observedAttributes(): FrameElementObservedAttribute[] { + return ["disabled", "loaded", "loading", "src"] } constructor() { @@ -56,6 +59,7 @@ export class FrameElement extends HTMLElement { reload() { const { src } = this; + this.removeAttribute("loaded") this.src = null; this.src = src; } @@ -63,6 +67,8 @@ export class FrameElement extends HTMLElement { attributeChangedCallback(name: string) { if (name == "loading") { this.delegate.loadingStyleChanged() + } else if (name == "loaded") { + this.delegate.loadedChanged() } else if (name == "src") { this.delegate.sourceURLChanged() } else { diff --git a/src/tests/fixtures/loading.html b/src/tests/fixtures/loading.html index 9c0b9a673..8b925933d 100644 --- a/src/tests/fixtures/loading.html +++ b/src/tests/fixtures/loading.html @@ -1,5 +1,5 @@ - + Turbo @@ -13,6 +13,8 @@ + Navigate #loading-lazy turbo-frame +
Eager-loaded diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index b2550e832..e6df0aeb6 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -422,6 +422,7 @@ 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.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } async "test navigating frame with a[data-turbo-action=advance] pushes URL state"() { @@ -436,6 +437,7 @@ 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.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } async "test navigating frame with form[method=get][data-turbo-action=advance] pushes URL state"() { @@ -450,6 +452,7 @@ 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.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } 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"() { @@ -477,6 +480,7 @@ 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.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } 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"() { @@ -490,6 +494,7 @@ export class FrameTests extends TurboDriveTestCase { 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]") + this.assert.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } async "test navigating frame with button[data-turbo-action=advance] pushes URL state"() { @@ -504,6 +509,7 @@ 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.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } async "test navigating back after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames previous contents"() { @@ -539,6 +545,7 @@ 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.ok(await this.hasSelector("#frame[loaded]"), "marks the frame as [loaded]") } async "test turbo:before-fetch-request fires on the frame element"() { diff --git a/src/tests/functional/loading_tests.ts b/src/tests/functional/loading_tests.ts index 328b5acb1..98f9bfd1e 100644 --- a/src/tests/functional/loading_tests.ts +++ b/src/tests/functional/loading_tests.ts @@ -1,3 +1,4 @@ +import { FrameElement } from "../../elements/frame_element" import { TurboDriveTestCase } from "../helpers/turbo_drive_test_case" declare global { @@ -14,6 +15,7 @@ export class LoadingTests extends TurboDriveTestCase { async "test eager loading within a details element"() { await this.nextBeat this.assert.ok(await this.hasSelector("#loading-eager turbo-frame#frame h2")) + this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "has [loaded] attribute") } async "test lazy loading within a details element"() { @@ -21,12 +23,14 @@ export class LoadingTests extends TurboDriveTestCase { const frameContents = "#loading-lazy turbo-frame h2" this.assert.notOk(await this.hasSelector(frameContents)) + this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame:not([loaded])")) await this.clickSelector("#loading-lazy summary") await this.nextBeat const contents = await this.querySelector(frameContents) this.assert.equal(await contents.getVisibleText(), "Hello from a frame") + this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "has [loaded] attribute") } async "test changing loading attribute from lazy to eager loads frame"() { @@ -92,9 +96,12 @@ export class LoadingTests extends TurboDriveTestCase { const frameContent = "#loading-eager turbo-frame#frame h2" this.assert.ok(await this.hasSelector(frameContent)) - // @ts-ignore - await this.remote.execute(() => document.querySelector("#loading-eager turbo-frame")?.reload()) + this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "has [loaded] attribute") + + await this.remote.execute(() => document.querySelector("#loading-eager turbo-frame")?.reload()) + this.assert.ok(await this.hasSelector(frameContent)) + this.assert.ok(await this.hasSelector("#loading-eager turbo-frame:not([loaded])"), "clears [loaded] attribute") } async "test navigating away from a page does not reload its frames"() { @@ -106,6 +113,70 @@ export class LoadingTests extends TurboDriveTestCase { this.assert.equal(requestLogs.length, 1) } + async "test removing the [loaded] attribute of an eager frame reloads the content"() { + await this.nextEventOnTarget("frame", "turbo:frame-load") + await this.remote.execute(() => document.querySelector("#loading-eager turbo-frame")?.removeAttribute("loaded")) + await this.nextEventOnTarget("frame", "turbo:frame-load") + + this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "sets the [loaded] attribute after re-loading") + } + + async "test changing [src] attribute on a [loaded] frame with loading=lazy defers navigation"() { + await this.nextEventOnTarget("frame", "turbo:frame-load") + await this.clickSelector("#loading-lazy summary") + await this.nextEventOnTarget("hello", "turbo:frame-load") + + this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "lazy frame is loaded") + this.assert.equal(await (await this.querySelector("#hello h2")).getVisibleText(), "Hello from a frame") + + await this.clickSelector("#loading-lazy summary") + await this.clickSelector("#one") + await this.nextEventNamed("turbo:load") + await this.goBack() + await this.nextBody + await this.noNextEventNamed("turbo:frame-load") + + let src = new URL(await this.attributeForSelector("#hello", "src") || "") + + this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "lazy frame is loaded") + this.assert.equal(src.pathname, "/src/tests/fixtures/frames/hello.html", "lazy frame retains [src]") + + await this.clickSelector("#link-lazy-frame") + await this.noNextEventNamed("turbo:frame-load") + + this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame:not([loaded])"), "lazy frame is not loaded") + + await this.clickSelector("#loading-lazy summary") + await this.nextEventOnTarget("hello", "turbo:frame-load") + + src = new URL(await this.attributeForSelector("#hello", "src") || "") + + this.assert.equal(await (await this.querySelector("#loading-lazy turbo-frame h2")).getVisibleText(), "Frames: #hello") + this.assert.ok(await this.hasSelector("#loading-lazy turbo-frame[loaded]"), "lazy frame is loaded") + this.assert.equal(src.pathname, "/src/tests/fixtures/frames.html", "lazy frame navigates") + } + + async "test navigating away from a page and then back does not reload its frames"() { + await this.clickSelector("#one") + await this.nextBody + await this.eventLogChannel.read() + await this.goBack() + await this.nextBody + + const eventLogs = await this.eventLogChannel.read() + const requestLogs = eventLogs.filter(([ name ]) => name == "turbo:before-fetch-request") + const requestsOnEagerFrame = requestLogs.filter(record => record[2] == "frame") + const requestsOnLazyFrame = requestLogs.filter(record => record[2] == "hello") + + this.assert.equal(requestsOnEagerFrame.length, 0, "does not reload eager frame") + this.assert.equal(requestsOnLazyFrame.length, 0, "does not reload lazy frame") + + await this.clickSelector("#loading-lazy summary") + await this.nextEventOnTarget("hello", "turbo:before-fetch-request") + await this.nextEventOnTarget("hello", "turbo:frame-render") + await this.nextEventOnTarget("hello", "turbo:frame-load") + } + async "test disconnecting and reconnecting a frame does not reload the frame"() { await this.nextBeat