Skip to content

Commit

Permalink
Respect turbo-visit-control for frame requests (#867)
Browse files Browse the repository at this point in the history
Turbo normally performs a fill page reload whenever a response contains
the appropriate `turbo-visit-control` meta tag:

    <meta name="turbo-visit-control" content="reload">

Such responses are considered "not visitable".

For frame requests, we have previously been ignoring any
`turbo-visit-control` set in the response, and instead treating all
valid frame responses as "visitable".

This commit changes this behaviour so that `turbo-visit-control` will be
treated consistently for both frame and non-frame requests.

As well as being more consistent, this provides a useful escape hatch
for situations where a frame request redirects to something that should
be a full page reload, but which would be prevented due to that content
missing the expected frame. The class example of this is when an expired
session causes a frame request to be redirected to a login page. By
including `turbo-visit-control` on that login page, we can ensure that
it is always rendered as a full page, and never hidden by a failed frame
request.
  • Loading branch information
kevinmcconnell authored Feb 7, 2023
1 parent 91ee8f6 commit 1e78f3b
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 27 deletions.
59 changes: 35 additions & 24 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,29 +160,13 @@ export class FrameController
try {
const html = await fetchResponse.responseHTML
if (html) {
const { body } = parseHTMLDocument(html)
const newFrameElement = await this.extractForeignFrameElement(body)

if (newFrameElement) {
const snapshot = new Snapshot(newFrameElement)
const renderer = new FrameRenderer(
this,
this.view.snapshot,
snapshot,
FrameRenderer.renderElement,
false,
false
)
if (this.view.renderPromise) await this.view.renderPromise
this.changeHistory()

await this.view.render(renderer)
this.complete = true
session.frameRendered(fetchResponse, this.element)
session.frameLoaded(this.element)
this.fetchResponseLoaded(fetchResponse)
} else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
this.handleFrameMissingFromResponse(fetchResponse)
const document = parseHTMLDocument(html)
const pageSnapshot = PageSnapshot.fromDocument(document)

if (pageSnapshot.isVisitable) {
await this.loadFrameResponse(fetchResponse, document)
} else {
await this.handleUnvisitableFrameResponse(fetchResponse)
}
}
} finally {
Expand Down Expand Up @@ -343,6 +327,25 @@ export class FrameController

// Private

private async loadFrameResponse(fetchResponse: FetchResponse, document: Document) {
const newFrameElement = await this.extractForeignFrameElement(document.body)

if (newFrameElement) {
const snapshot = new Snapshot(newFrameElement)
const renderer = new FrameRenderer(this, this.view.snapshot, snapshot, FrameRenderer.renderElement, false, false)
if (this.view.renderPromise) await this.view.renderPromise
this.changeHistory()

await this.view.render(renderer)
this.complete = true
session.frameRendered(fetchResponse, this.element)
session.frameLoaded(this.element)
this.fetchResponseLoaded(fetchResponse)
} else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
this.handleFrameMissingFromResponse(fetchResponse)
}
}

private async visit(url: URL) {
const request = new FetchRequest(this, FetchMethod.get, url, new URLSearchParams(), this.element)

Expand Down Expand Up @@ -405,6 +408,14 @@ export class FrameController
}
}

private async handleUnvisitableFrameResponse(fetchResponse: FetchResponse) {
console.warn(
`The response (${fetchResponse.statusCode}) from <turbo-frame id="${this.element.id}"> is performing a full page visit due to turbo-visit-control.`
)

await this.visitResponse(fetchResponse.response)
}

private willHandleFrameMissingFromResponse(fetchResponse: FetchResponse): boolean {
this.element.setAttribute("complete", "")

Expand Down Expand Up @@ -432,7 +443,7 @@ export class FrameController
}

private throwFrameMissingError(fetchResponse: FetchResponse) {
const message = `The response (${fetchResponse.statusCode}) did not contain the expected <turbo-frame id="${this.element.id}">`
const message = `The response (${fetchResponse.statusCode}) did not contain the expected <turbo-frame id="${this.element.id}"> and will be ignored. To perform a full page visit instead, set turbo-visit-control to reload.`
throw new TurboFrameMissingError(message)
}

Expand Down
3 changes: 2 additions & 1 deletion src/tests/fixtures/frames.html
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ <h2>Frames: #nested-child</h2>

<turbo-frame id="missing">
<a id="missing-frame-link" href="/src/tests/fixtures/frames/frame.html">Missing frame</a>
<a id="missing-page-link" href="/missing.html">404</a>
<a id="missing-page-link" href="/missing.html">Missing page</a>
<a id="unvisitable-page-link" href="/src/tests/fixtures/frames/unvisitable.html">Unvisitable page</a>
</turbo-frame>

<turbo-frame id="body-script" target="body-script">
Expand Down
16 changes: 16 additions & 0 deletions src/tests/fixtures/frames/unvisitable.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
<title>Frame</title>
<script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
<meta name="turbo-visit-control" content="reload" />
</head>
<body>
<h1>Unvisitable page loaded</h1>

<turbo-frame id="missing">
<h1>Frame content</h1>
</turbo-frame>
</body>
</html>
13 changes: 11 additions & 2 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,16 @@ test("successfully following a link to a page without a matching frame shows an
assert.match(await page.innerText("#missing"), /Content missing/)

assert.exists(error)
assert.equal(error!.message, `The response (200) did not contain the expected <turbo-frame id="missing">`)
assert.include(error!.message, `The response (200) did not contain the expected <turbo-frame id="missing">`)
})

test("successfully following a link to a page with `turbo-visit-control` `reload` performs a full page reload", async ({
page,
}) => {
await page.click("#unvisitable-page-link")
await page.getByText("Unvisitable page loaded").waitFor()

assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/unvisitable.html")
})

test("failing to follow a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
Expand All @@ -184,7 +193,7 @@ test("failing to follow a link to a page without a matching frame shows an error
assert.match(await page.innerText("#missing"), /Content missing/)

assert.exists(error)
assert.equal(error!.message, `The response (404) did not contain the expected <turbo-frame id="missing">`)
assert.include(error!.message, `The response (404) did not contain the expected <turbo-frame id="missing">`)
})

test("test the turbo:frame-missing event following a link to a page without a matching frame can be handled", async ({
Expand Down

0 comments on commit 1e78f3b

Please sign in to comment.