Skip to content

Commit

Permalink
Ignore links and forms that target "_blank"
Browse files Browse the repository at this point in the history
Closes [#1171][]

Forms that target [_blank][form-target-blank] along with anchors that
target [_blank][] navigate new tabs, and are therefore incompatible with
Turbo's long-lived session.

This commit ensures that both `<a>` and `<form>` submissions that target
`_blank` are ignored.

[#1171]: #1171
[form-target-blank]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form#target
[anchor-target-blank]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#target
  • Loading branch information
seanpdoyle committed Mar 31, 2024
1 parent 9fb05e3 commit 5bc21c0
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 21 deletions.
14 changes: 4 additions & 10 deletions src/observers/form_submit_observer.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { doesNotTargetIFrame } from "../util"

export class FormSubmitObserver {
started = false

Expand Down Expand Up @@ -51,15 +53,7 @@ function submissionDoesNotDismissDialog(form, submitter) {
}

function submissionDoesNotTargetIFrame(form, submitter) {
if (submitter?.hasAttribute("formtarget") || form.hasAttribute("target")) {
const target = submitter?.getAttribute("formtarget") || form.target

for (const element of document.getElementsByName(target)) {
if (element instanceof HTMLIFrameElement) return false
}
const target = submitter?.getAttribute("formtarget") || form.getAttribute("target")

return true
} else {
return true
}
return doesNotTargetIFrame(target)
}
2 changes: 1 addition & 1 deletion src/observers/link_click_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class LinkClickObserver {
if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) {
const target = (event.composedPath && event.composedPath()[0]) || event.target
const link = findLinkFromClickTarget(target)
if (link && doesNotTargetIFrame(link)) {
if (link && doesNotTargetIFrame(link.target)) {
const location = getLocationForLink(link)
if (this.delegate.willFollowLinkToLocation(link, location, event)) {
event.preventDefault()
Expand Down
14 changes: 10 additions & 4 deletions src/tests/fixtures/navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -94,24 +94,30 @@ <h1>Navigation</h1>
<p><a id="delayed-failure-link" href="/__turbo/delayed_response?status=500">Delayed failure link</a></p>
<p><a id="link-target-iframe" href="/src/tests/fixtures/one.html" target="iframe">Targets iframe[name="iframe"]</a></p>
<p><a id="link-target-empty-name-iframe" href="/src/tests/fixtures/one.html" target="">Targets iframe[name=""]</a></p>
<p>
<form id="form-target-blank" action="/src/tests/fixtures/one.html" target="_blank">
<button>form[target="_blank"]</button>
</form>
</p>
<p>
<form id="form-target-iframe" action="/src/tests/fixtures/one.html" target="iframe">
<button>Targets iframe[name="iframe"]</button>
<button>form[target="iframe"]</button>
</form>
</p>
<p>
<form id="form-target-empty-name-iframe" action="/src/tests/fixtures/one.html" target="">
<button>Targets iframe[name=""]</button>
<button>form[target=""]</button>
</form>
</p>
<p>
<form action="/src/tests/fixtures/one.html">
<button id="button-formtarget-iframe" formtarget="iframe">Targets iframe[name="iframe"]</button>
<button id="button-formtarget-blank" formtarget="_blank">button[formtarget="_blank"]</button>
<button id="button-formtarget-iframe" formtarget="iframe">button[formtarget="iframe"]</button>
</form>
</p>
<p>
<form action="/src/tests/fixtures/one.html">
<button id="button-formtarget-empty-name-iframe" formtarget="">Targets iframe[name=""]</button>
<button id="button-formtarget-empty-name-iframe" formtarget="">button[formtarget=""]</button>
</form>
</p>
<p><a id="redirect-to-cache-observer" href="/__turbo/redirect?path=/src/tests/fixtures/cache_observer.html">Redirect to cache_observer.html</a></p>
Expand Down
14 changes: 13 additions & 1 deletion src/tests/functional/navigation_tests.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test } from "@playwright/test"
import { expect, test } from "@playwright/test"
import { assert } from "chai"
import {
clickWithoutScrolling,
Expand Down Expand Up @@ -473,6 +473,12 @@ test("ignores links with a [target] attribute that targets an iframe with [name=
assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
})

test("ignores forms with a [target=_blank] attribute", async ({ page }) => {
const [popup] = await Promise.all([page.waitForEvent("popup"), page.click("#form-target-blank button")])

expect(pathname(popup.url())).toContain("/src/tests/fixtures/one.html")
})

test("ignores forms with a [target] attribute that targets an iframe with a matching [name]", async ({ page }) => {
await page.click("#form-target-iframe button")
await nextBeat()
Expand All @@ -482,6 +488,12 @@ test("ignores forms with a [target] attribute that targets an iframe with a matc
assert.equal(await pathnameForIFrame(page, "iframe"), "/src/tests/fixtures/one.html")
})

test("ignores forms with a button[formtarget=_blank] attribute", async ({ page }) => {
const [popup] = await Promise.all([page.waitForEvent("popup"), page.click("#button-formtarget-blank")])

expect(pathname(popup.url())).toContain("/src/tests/fixtures/one.html")
})

test("ignores forms with a button[formtarget] attribute that targets an iframe with [name='']", async ({
page
}) => {
Expand Down
14 changes: 9 additions & 5 deletions src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,18 @@ export async function around(callback, reader) {
return [before, after]
}

export function doesNotTargetIFrame(anchor) {
if (anchor.hasAttribute("target")) {
for (const element of document.getElementsByName(anchor.target)) {
export function doesNotTargetIFrame(name) {
if (name === "_blank") {
return false
} else if (name) {
for (const element of document.getElementsByName(name)) {
if (element instanceof HTMLIFrameElement) return false
}
}

return true
return true
} else {
return true
}
}

export function findLinkFromClickTarget(target) {
Expand Down

0 comments on commit 5bc21c0

Please sign in to comment.