Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Render 4xx responses within frame #210

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Render 4xx responses within frame #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/core/frames/frame_controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { isAction, Action } from "../types"
import { VisitOptions } from "../drive/visit"
import { TurboBeforeFrameRenderEvent, TurboFetchRequestErrorEvent } from "../session"
import { StreamMessage } from "../streams/stream_message"
import { navigator } from "../../core"

export type TurboFrameMissingEvent = CustomEvent<{ fetchResponse: FetchResponse }>

Expand Down Expand Up @@ -273,11 +274,17 @@ export class FrameController
}

formSubmissionSucceededWithResponse(formSubmission: FormSubmission, response: FetchResponse) {
const frame = this.findFrameElement(formSubmission.formElement, formSubmission.submitter)
const { formElement, submitter } = formSubmission
const frame = this.findFrameElement(formElement, submitter)
const target = getAttribute("data-turbo-frame", submitter, formElement) || frame.getAttribute("target")

this.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)

frame.delegate.loadResponse(response)
if (target == "_top") {
navigator.formSubmission = formSubmission
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sstephenson this felt like the shortest distance to achieve the outcome. The awkwardness signals to me that there is a missing concept or delegate here.

A means of delegating or directly accessing the Navigator would also enable the transformation of a frame navigation to a fully formed Visit to drive the page.

navigator.formSubmissionSucceededWithResponse(formSubmission, response)
} else {
this.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)
frame.delegate.loadResponse(response)
}
}

formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {
Expand Down Expand Up @@ -447,11 +454,15 @@ export class FrameController
private shouldInterceptNavigation(element: Element, submitter?: HTMLElement) {
const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target")

if (element instanceof HTMLFormElement && !this.formActionIsVisitable(element, submitter)) {
if (!this.enabled) {
return false
}

if (!this.enabled || id == "_top") {
if (element instanceof HTMLFormElement) {
if (!this.formActionIsVisitable(element, submitter)) {
return false
}
} else if (id == "_top") {
return false
}

Expand All @@ -470,7 +481,7 @@ export class FrameController
return false
}

return true
return this.element == element.closest("turbo-frame:not([disabled])")
}

// Computed properties
Expand Down
30 changes: 22 additions & 8 deletions src/core/frames/frame_redirector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { FrameElement } from "../../elements/frame_element"
import { expandURL, getAction, locationIsVisitable } from "../url"
import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer"
import { Session } from "../session"
import { getAttribute } from "../../util"

export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObserverDelegate {
readonly session: Session
Expand Down Expand Up @@ -69,19 +70,32 @@ export class FrameRedirector implements LinkClickObserverDelegate, FormSubmitObs

if (isNavigatable) {
const frame = this.findFrameElement(element, submitter)
return frame ? frame != element.closest("turbo-frame") : false
} else {
return false

if (frame) {
const id = getAttribute("data-turbo-frame", submitter, element) || frame.getAttribute("target")

if (frame == findClosestFrameElement(element)) {
return id == "_top"
} else {
return true
}
} else {
return false
}
}
}

private findFrameElement(element: Element, submitter?: HTMLElement) {
const id = submitter?.getAttribute("data-turbo-frame") || element.getAttribute("data-turbo-frame")
const id = getAttribute("data-turbo-frame", submitter, element)

if (id && id != "_top") {
const frame = this.element.querySelector(`#${id}:not([disabled])`)
if (frame instanceof FrameElement) {
return frame
}
return this.element.querySelector<FrameElement>(`turbo-frame#${id}:not([disabled])`)
} else {
return findClosestFrameElement(element)
}
}
}

function findClosestFrameElement(element: Element) {
return element.closest<FrameElement>("turbo-frame:not([disabled])")
}
26 changes: 26 additions & 0 deletions src/tests/fixtures/form.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
position: static;
}
</style>
<script>
addEventListener("click", ({ target }) => {
if (target.id == "frame-target-top") {
const frame = document.getElementById("frame")
frame?.setAttribute("target", "_top")
}
})
</script>
</head>
<body>
<h1>Form</h1>
Expand Down Expand Up @@ -293,6 +301,24 @@ <h2>Frame: Form</h2>
</form>
<div id="messages">
</div>
<h>
<form action="/__turbo/redirect" method="post" data-turbo-frame="_top">
<input type="hidden" name="path" value="/src/tests/fixtures/one.html">
<button id="frame-form-targets-top-303">303 data-turbo-frame="_top"</button>
</form>
<form action="/__turbo/reject" method="post" data-turbo-frame="_top">
<input type="hidden" name="status" value="422">
<button id="frame-form-targets-top-422">422 form[data-turbo-frame="_top"]</button>
</form>
<form action="/__turbo/reject" method="post">
<input type="hidden" name="status" value="422">
<button id="frame-button-targets-top-422" data-turbo-frame="_top">422 button[data-turbo-frame="_top"]</button>
</form>
<form action="/__turbo/reject" method="post">
<input type="hidden" name="status" value="422">
<button id="frame-frame-targets-top-422">422 turbo-frame[target="_top"]</button>
</form>
<button id="set-frame-target-top" type="button">Set #frame[target="_top"]</button>
</turbo-frame>
<a href="/src/tests/fixtures/frames/hello.html" data-turbo-method="get" id="link-method-outside-frame">Method link outside frame</a><br />
<a href="/__turbo/messages?content=Link!&type=stream" data-turbo-method="post" id="stream-link-method-outside-frame">Stream link outside frame</a>
Expand Down
46 changes: 46 additions & 0 deletions src/tests/functional/form_submission_tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,52 @@ test("test invalid frame form submission with internal server errror status", as
assert.equal(await page.textContent("#frame h2"), "Frame: Internal Server Error")
})

test("test frame form submission with form[data-turbo-frame=_top]", async ({ page }) => {
await page.click("#frame-form-targets-top-303")
await nextEventNamed(page, "turbo:load")

assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
assert.equal(await page.textContent("h1"), "One", "follows form redirect")
assert.notOk(await hasSelector(page, "#frame"), "redirects entire page")
})

test("test invalid frame form submission submitted by form[data-turbo-frame=_top]", async ({ page }) => {
await page.click("#frame-form-targets-top-422")
await nextBeat()

assert.equal(await page.textContent("h1"), "Form", "does not replace entire page")
assert.equal(
await page.textContent("#frame h2"),
"Frame: Unprocessable Entity",
"renders the response HTML inside the frame"
)
})

test("test invalid frame form submission submitted by button[data-turbo-frame=_top]", async ({ page }) => {
await page.click("#frame-button-targets-top-422")
await nextBeat()

assert.equal(await page.textContent("h1"), "Form", "does not replace entire page")
assert.equal(
await page.textContent("#frame h2"),
"Frame: Unprocessable Entity",
"renders the response HTML inside the frame"
)
})

test("test unprocessable entity frame form submission submitted in turbo-frame[target=_top]", async ({ page }) => {
await page.click("#set-frame-target-top")
await page.click("#frame-form-targets-top-422")
await nextBeat()

assert.equal(await page.textContent("h1"), "Form", "does not replace entire page")
assert.equal(
await page.textContent("#frame h2"),
"Frame: Unprocessable Entity",
"renders the response HTML inside the frame"
)
})

test("test frame form submission with stream response", async ({ page }) => {
const button = await page.locator("#frame form.stream[method=post] input[type=submit]")
await button.click()
Expand Down