From ac4b0f9b5e49761e9fdd009964a1fd3afcfafa05 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 14 Jan 2021 17:03:34 -0500 Subject: [PATCH] GET form submissions to the current path Closes https://github.com/hotwired/turbo/issues/107 The [HTML specification][] outlines the algorithm for handling form submissions, and describes the steps involved in handling a `method=get` request with `application/x-www-form-urlencoded` encoding. However, it doesn't explicitly mention how to handle a `
` submission when the `action` attribute is omitted and the current URL includes a query parameter that would be included within the `` element fields' values. Determining the test cases to reproduce browser default behavior involved was some trial and error troubleshooting in real browsers with default (JavaScript-less) `` submission handling. The algorithm appears to be as follows: * If there are no query parameters, append all field values as query parameters * If there are existing query parameters whose keys exist as `` fields, override them based on the form field's value * If there are existing query parameters whose keys exist as _multiple_ `` fields, override the first occurrence, and then append subsequent values [HTML specification]: https://html.spec.whatwg.org/#form-submission-algorithm --- src/http/fetch_request.ts | 21 +++++--- src/tests/fixtures/form.html | 17 ++++++ src/tests/functional/form_submission_tests.ts | 54 +++++++++++++++++-- src/tests/helpers/functional_test_case.ts | 4 ++ 4 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/http/fetch_request.ts b/src/http/fetch_request.ts index 0dcc206f2..404429408 100644 --- a/src/http/fetch_request.ts +++ b/src/http/fetch_request.ts @@ -55,13 +55,22 @@ export class FetchRequest { } get url() { - const url = this.location.absoluteURL - const query = this.params.toString() - if (this.isIdempotent && query.length) { - return [url, query].join(url.includes("?") ? "&" : "?") - } else { - return url + const url = new URL(this.location.absoluteURL) + + if (this.isIdempotent) { + const currentUrl = new URL(window.location.href) + + for (const [ key, value ] of this.params) { + if (currentUrl.searchParams.has(key)) { + currentUrl.searchParams.delete(key) + url.searchParams.set(key, value) + } else { + url.searchParams.append(key, value) + } + } } + + return url.href } get params() { diff --git a/src/tests/fixtures/form.html b/src/tests/fixtures/form.html index c4c566f43..3b36fae7b 100644 --- a/src/tests/fixtures/form.html +++ b/src/tests/fixtures/form.html @@ -26,7 +26,24 @@
+
+ + +
+
+
+
+ + +
+
+ + + +
+
+
diff --git a/src/tests/functional/form_submission_tests.ts b/src/tests/functional/form_submission_tests.ts index a83b291a1..349b7964b 100644 --- a/src/tests/functional/form_submission_tests.ts +++ b/src/tests/functional/form_submission_tests.ts @@ -5,7 +5,7 @@ export class FormSubmissionTests extends TurboDriveTestCase { await this.goToLocation("/src/tests/fixtures/form.html") } - async "test standard form submission with redirect response"() { + async "test standard POST form submission with redirect response"() { this.listenForFormSubmissions() const button = await this.querySelector("#standard form.redirect input[type=submit]") await button.click() @@ -16,6 +16,17 @@ export class FormSubmissionTests extends TurboDriveTestCase { this.assert.equal(await this.visitAction, "advance") } + async "test standard GET form submission"() { + this.listenForFormSubmissions() + await this.clickSelector("#standard form[method=get] input[type=submit]") + await this.nextBody + + this.assert.ok(this.turboFormSubmitted) + this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html") + this.assert.equal(await this.search, "?query=true") + this.assert.equal(await this.visitAction, "advance") + } + async "test standard form submission with empty created response"() { const htmlBefore = await this.outerHTMLForSelector("body") const button = await this.querySelector("#standard form.created input[type=submit]") @@ -36,6 +47,42 @@ export class FormSubmissionTests extends TurboDriveTestCase { this.assert.equal(htmlAfter, htmlBefore) } + async "test no-action form submission with single parameter"() { + await this.clickSelector("#no-action form.single input[type=submit]") + await this.nextBody + + this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html") + this.assert.equal(await this.search, "?query=1") + + await this.clickSelector("#no-action form.single input[type=submit]") + await this.nextBody + + this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html") + this.assert.equal(await this.search, "?query=1") + + await this.goToLocation("/src/tests/fixtures/form.html?query=2") + await this.clickSelector("#no-action form.single input[type=submit]") + await this.nextBody + + this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html") + this.assert.equal(await this.search, "?query=1") + } + + async "test no-action form submission with multiple parameters"() { + await this.goToLocation("/src/tests/fixtures/form.html?query=2") + await this.clickSelector("#no-action form.multiple input[type=submit]") + await this.nextBody + + this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html") + this.assert.equal(await this.search, "?query=1&query=2") + + await this.clickSelector("#no-action form.multiple input[type=submit]") + await this.nextBody + + this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html") + this.assert.equal(await this.search, "?query=1&query=2") + } + async "test invalid form submission with unprocessable entity status"() { await this.clickSelector("#reject form.unprocessable_entity input[type=submit]") await this.nextBody @@ -168,10 +215,9 @@ export class FormSubmissionTests extends TurboDriveTestCase { } listenForFormSubmissions() { - this.remote.execute(() => addEventListener("turbo:submit-start", function eventListener(event) { - removeEventListener("turbo:submit-start", eventListener, false) + this.remote.execute(() => addEventListener("turbo:submit-start", (event) => { document.head.insertAdjacentHTML("beforeend", ``) - }, false)) + }, { once: true })) } get turboFormSubmitted(): Promise { diff --git a/src/tests/helpers/functional_test_case.ts b/src/tests/helpers/functional_test_case.ts index 9d1a6edef..3ed8ab7c1 100644 --- a/src/tests/helpers/functional_test_case.ts +++ b/src/tests/helpers/functional_test_case.ts @@ -90,6 +90,10 @@ export class FunctionalTestCase extends InternTestCase { return this.evaluate(() => location.pathname) } + get search(): Promise { + return this.evaluate(() => location.search) + } + get hash(): Promise { return this.evaluate(() => location.hash) }