diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 85d514387..e7eec013d 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -1,4 +1,9 @@ -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" @@ -29,14 +34,13 @@ export class FrameController 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 @@ -49,13 +53,13 @@ export class FrameController connect() { if (!this.connected) { this.connected = true - this.reloadable = false if (this.loadingStyle == FrameLoadingStyle.lazy) { this.appearanceObserver.start() + } else { + this.loadSourceURL() } this.linkInterceptor.start() this.formInterceptor.start() - this.sourceURLChanged() } } @@ -75,11 +79,23 @@ export class FrameController } 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() @@ -89,26 +105,12 @@ export class FrameController } } - 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 } } @@ -125,6 +127,7 @@ export class FrameController 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) @@ -154,7 +157,6 @@ export class FrameController } linkClickIntercepted(element: Element, url: string) { - this.reloadable = true this.navigateFrame(element, url) } @@ -169,7 +171,6 @@ export class FrameController this.formSubmission.stop() } - this.reloadable = false this.formSubmission = new FormSubmission(this, element, submitter) const { fetchRequest } = this.formSubmission this.prepareHeadersForRequest(fetchRequest.headers, fetchRequest) @@ -272,7 +273,6 @@ export class FrameController this.proposeVisitIfNavigatedWithAction(frame, element, submitter) - frame.setAttribute("reloadable", "") frame.src = url } @@ -308,12 +308,12 @@ export class FrameController const id = CSS.escape(this.id) try { - element = activateElement(container.querySelector(`turbo-frame#${id}`), this.currentURL) + element = activateElement(container.querySelector(`turbo-frame#${id}`), this.sourceURL) if (element) { return element } - element = activateElement(container.querySelector(`turbo-frame[src][recurse~=${id}]`), this.currentURL) + element = activateElement(container.querySelector(`turbo-frame[src][recurse~=${id}]`), this.sourceURL) if (element) { await element.loaded return await this.extractForeignFrameElement(element) @@ -379,24 +379,9 @@ export class FrameController } set sourceURL(sourceURL: string | undefined) { - this.settingSourceURL = true - this.element.src = sourceURL ?? null - this.currentURL = this.element.src - this.settingSourceURL = false - } - - 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") - } + this.ignoringChangesToAttribute("src", () => { + this.element.src = sourceURL ?? null + }) } get loadingStyle() { @@ -407,6 +392,20 @@ export class FrameController 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 } @@ -416,6 +415,16 @@ export class FrameController const root = meta?.content ?? "/" return expandURL(root) } + + private isIgnoringChangesTo(attributeName: FrameElementObservedAttribute): boolean { + return this.ignoredAttributes.has(attributeName) + } + + private ignoringChangesToAttribute(attributeName: FrameElementObservedAttribute, 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 8e41aea8e..1ddb5a0d5 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 d64e552e0..83c9a8ce1 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -5,9 +5,12 @@ export enum FrameLoadingStyle { lazy = "lazy", } +export type FrameElementObservedAttribute = keyof FrameElement & ("disabled" | "loaded" | "loading" | "src") + export interface FrameElementDelegate { connect(): void disconnect(): void + loadedChanged(): void loadingStyleChanged(): void sourceURLChanged(): void disabledChanged(): void @@ -40,8 +43,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() { @@ -59,6 +62,7 @@ export class FrameElement extends HTMLElement { reload() { const { src } = this + this.removeAttribute("loaded") this.src = null this.src = src } @@ -66,6 +70,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 9ef2ff199..cf15f9508 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -450,6 +450,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"() { @@ -464,6 +465,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"() { @@ -478,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=get][data-turbo-action=advance] to the same URL clears the [aria-busy] and [data-turbo-preview] state"() { @@ -505,6 +508,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"() { @@ -518,6 +522,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"() { @@ -532,6 +537,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"() { @@ -567,6 +573,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 abd11af32..3a433c8df 100644 --- a/src/tests/functional/loading_tests.ts +++ b/src/tests/functional/loading_tests.ts @@ -14,6 +14,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 +22,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"() { @@ -98,8 +101,11 @@ export class LoadingTests extends TurboDriveTestCase { const frameContent = "#loading-eager turbo-frame#frame h2" this.assert.ok(await this.hasSelector(frameContent)) + this.assert.ok(await this.hasSelector("#loading-eager turbo-frame[loaded]"), "has [loaded] attribute") + await this.remote.execute(() => (document.querySelector("#loading-eager turbo-frame") as any)?.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"() { @@ -111,6 +117,76 @@ 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