From 5bc21c09ff90509081a094566fae33b1ed33620c Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Sat, 30 Mar 2024 20:59:54 -0400
Subject: [PATCH] Ignore links and forms that target `"_blank"`

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]: https://github.com/hotwired/turbo/issues/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
---
 src/observers/form_submit_observer.js    | 14 ++++----------
 src/observers/link_click_observer.js     |  2 +-
 src/tests/fixtures/navigation.html       | 14 ++++++++++----
 src/tests/functional/navigation_tests.js | 14 +++++++++++++-
 src/util.js                              | 14 +++++++++-----
 5 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/src/observers/form_submit_observer.js b/src/observers/form_submit_observer.js
index 1158d59b9..204e4665a 100644
--- a/src/observers/form_submit_observer.js
+++ b/src/observers/form_submit_observer.js
@@ -1,3 +1,5 @@
+import { doesNotTargetIFrame } from "../util"
+
 export class FormSubmitObserver {
   started = false
 
@@ -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)
 }
diff --git a/src/observers/link_click_observer.js b/src/observers/link_click_observer.js
index 24c6aa235..c1e6abfb7 100644
--- a/src/observers/link_click_observer.js
+++ b/src/observers/link_click_observer.js
@@ -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()
diff --git a/src/tests/fixtures/navigation.html b/src/tests/fixtures/navigation.html
index 7b698a613..cd43d2b06 100644
--- a/src/tests/fixtures/navigation.html
+++ b/src/tests/fixtures/navigation.html
@@ -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>
diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js
index e1aa7e757..eebef546c 100644
--- a/src/tests/functional/navigation_tests.js
+++ b/src/tests/functional/navigation_tests.js
@@ -1,4 +1,4 @@
-import { test } from "@playwright/test"
+import { expect, test } from "@playwright/test"
 import { assert } from "chai"
 import {
   clickWithoutScrolling,
@@ -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()
@@ -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
 }) => {
diff --git a/src/util.js b/src/util.js
index dcb15c31b..57c48ea6a 100644
--- a/src/util.js
+++ b/src/util.js
@@ -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) {