From 728a56119efe98ec157ddb7b57524dd8d5d184c0 Mon Sep 17 00:00:00 2001 From: Kevin McConnell Date: Wed, 12 Oct 2022 08:25:42 +0100 Subject: [PATCH] Reload page if failed form changes tracked content (#759) When a form submission fails with a client error, the response from the form replaces the current page content. In Turbo 7.1.x, whenever this replacement would affect resources marked with `data-turbo-track="reload"`, a reload of the page is triggered instead. In 7.2.0, we do not perform this reload. Instead, the form's response is discarded. This difference can affect situations like expired sessions, where the previous behaviour of reloading the page would cause a new session to be established. If we instead allow the response to be discarded, actions can fail silently. This commit changes the behaviour to reload the current page when `reload` is called outside of a current visit, in order to match the behaviour of 7.1, and to not discard the response from the form submission. --- src/core/native/browser_adapter.ts | 4 +--- src/tests/fixtures/rendering.html | 1 + src/tests/functional/rendering_tests.ts | 31 +++++++++++++++++++++++++ src/tests/server.ts | 4 ++++ 4 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/core/native/browser_adapter.ts b/src/core/native/browser_adapter.ts index 9b5988e3c..ca268e341 100644 --- a/src/core/native/browser_adapter.ts +++ b/src/core/native/browser_adapter.ts @@ -123,9 +123,7 @@ export class BrowserAdapter implements Adapter { reload(reason: ReloadReason) { dispatch("turbo:reload", { detail: reason }) - if (!this.location) return - - window.location.href = this.location.toString() + window.location.href = this.location?.toString() || window.location.href } get navigator() { diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html index 44e2d09f6..67076e4ce 100644 --- a/src/tests/fixtures/rendering.html +++ b/src/tests/fixtures/rendering.html @@ -33,6 +33,7 @@

Rendering

Same-origin link

Tracked asset change

+

Tracked nonce tag

Additional assets

Head script

diff --git a/src/tests/functional/rendering_tests.ts b/src/tests/functional/rendering_tests.ts index 6ed9f8a87..e95d67e1a 100644 --- a/src/tests/functional/rendering_tests.ts +++ b/src/tests/functional/rendering_tests.ts @@ -65,6 +65,37 @@ test("test reloads when tracked elements change", async ({ page }) => { assert.equal(reason, "tracked_element_mismatch") }) +test("test reloads when tracked elements change due to failed form submission", async ({ page }) => { + await page.click("#tracked-asset-change-form button") + await page.evaluate(() => { + window.addEventListener( + "turbo:reload", + (e: any) => { + localStorage.setItem("reason", e.detail.reason) + }, + { once: true } + ) + + window.addEventListener( + "beforeunload", + () => { + localStorage.setItem("unloaded", "true") + }, + { once: true } + ) + }) + + await page.click("#tracked-asset-change-form button") + + const reason = await page.evaluate(() => localStorage.getItem("reason")) + const unloaded = await page.evaluate(() => localStorage.getItem("unloaded")) + + assert.equal(pathname(page.url()), "/src/tests/fixtures/rendering.html") + assert.equal(await visitAction(page), "load") + assert.equal(reason, "tracked_element_mismatch") + assert.equal(unloaded, "true") +}) + test("test before-render event supports custom render function", async ({ page }) => { await page.evaluate(() => addEventListener("turbo:before-render", (event) => { diff --git a/src/tests/server.ts b/src/tests/server.ts index 6f3bf2d6d..a0b59cd0f 100644 --- a/src/tests/server.ts +++ b/src/tests/server.ts @@ -86,6 +86,10 @@ router.post("/messages", (request, response) => { } }) +router.post("/notfound", (request, response) => { + response.type("html").status(404).send("

Not found

") +}) + router.get("/stream-response", (request, response) => { const params = { ...request.body, ...request.query } const { content, target, targets } = params