Skip to content

Commit

Permalink
Bugfix: Form Mode opt-in handle outside submitters
Browse files Browse the repository at this point in the history
Prior to this commit, the `Session.formElementIsNavigatable` method
checked for the closest `<form>` element ancestor to its `element`
argument. Unfortunately, checking for an ancestor form is an incomplete
solution, since form controls can exist outside of the `<form>`
elsewhere in the document and reference the form through their
[`[form]`][form-attr] attributes.

This commit incorporates a check for that into the
`Session.formElementIsNavigatable` method. Since that method is only
ever called on a submission's `submitter` argument after it's been
verified to be present, this commit changes the signature to expect an
`element` (by omitting the `?` modifier) and renames the method to
`submitterIsNavigatable`.

[form-attr]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Input#form
  • Loading branch information
seanpdoyle committed Jul 29, 2022
1 parent c4d9b00 commit 3ae1d46
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 6 deletions.
13 changes: 8 additions & 5 deletions src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export class Session

return (
this.elementIsNavigatable(form) &&
(!submitter || this.formElementIsNavigatable(submitter)) &&
(!submitter || this.submitterIsNavigatable(submitter)) &&
locationIsVisitable(expandURL(action), this.snapshot.rootLocation)
)
}
Expand Down Expand Up @@ -382,13 +382,12 @@ export class Session

// Helpers

formElementIsNavigatable(element?: Element) {
submitterIsNavigatable(element: Element) {
if (this.formMode == "off") {
return false
}
if (this.formMode == "optin") {
const form = element?.closest("form[data-turbo]")
return form?.getAttribute("data-turbo") == "true"
if (this.formMode == "optin" && hasForm(element) && element.form) {
return element.form.closest('[data-turbo="true"]') != null
}
return this.elementIsNavigatable(element)
}
Expand Down Expand Up @@ -449,3 +448,7 @@ const deprecatedLocationPropertyDescriptors = {
},
},
}

function hasForm(element: Element): element is Element & { form: HTMLFormElement | null } {
return "form" in element
}
4 changes: 3 additions & 1 deletion src/tests/fixtures/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,12 @@ <h1>Form</h1>
<input type="submit">
<button type="submit" name="greeting" id="secondary_submitter" data-turbo-confirm="Are you really sure?" formaction="/__turbo/redirect?path=/src/tests/fixtures/one.html" formmethod="post" value="secondary_submitter">Secondary action</button>
</form>
<form action="/__turbo/submit" method="post" data-turbo="true" class="turbo-enabled">
<form id="turbo-enabled-form" action="/__turbo/submit" method="post" data-turbo="true" class="turbo-enabled">
<input type="hidden" name="query" value="2">
<input type="submit">
</form>

<button form="turbo-enabled-form">Submit #turbo-enabled-form</button>
</div>
<hr>
<div id="no-action">
Expand Down
7 changes: 7 additions & 0 deletions src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,13 @@ test("test form submission with form mode optin and form enabled", async ({ page
assert.ok(await formSubmitStarted(page))
})

test("test form submission with form mode optin and form enabled from submitter outside form", async ({ page }) => {
await page.evaluate(() => window.Turbo.setFormMode("optin"))
await page.click("#standard button[form=turbo-enabled-form]")

assert.ok(await formSubmitStarted(page))
})

test("test turbo:before-fetch-request fires on the form element", async ({ page }) => {
await page.click('#targets-frame form.one [type="submit"]')
assert.ok(await nextEventOnTarget(page, "form_one", "turbo:before-fetch-request"))
Expand Down

0 comments on commit 3ae1d46

Please sign in to comment.