Close Issue hotwired#834

Closes [hotwired#834][]

When applications embed [Google Adsense][]-powered `<iframe>` elements,
the snippets they provide render them **with** a `[name]` attribute
that's set to the empty string `""`:

```html
<iframe
  frameborder="0"
  src="REDACTED"
  id="google_ads_iframe_/REDACTED"
  title="3rd party ad content"
  name=""
  scrolling="no"
  marginwidth="0"
  marginheight="0"
  width="728" height="90"
  data-is-safeframe="true"
  sandbox="allow-forms allow-popups allow-popups-to-escape-sandbox allow-same-origin allow-scripts allow-top-navigation-by-user-activation"
  role="region"
  aria-label="Advertisement"
  tabindex="0"
  data-google-container-id="1"
  style="border: 0px; vertical-align: bottom;"
  data-load-complete="true"></iframe>
```

Note the `[name=""]` in the snippet.

The guard clauses in the `FormSubmitObserver` and `LinkClickObserver`
classes that prevent Turbo Drive from interfering with `<a>` clicks and
`<form>` submissions that target `<iframe>` elements do not account for
the presence of an `<iframe name="">`. This commit extends those guard
clauses to first check for the presence of `a[target]`, `form[target]`,
and `input[formtarget]` or `button[formtarget]` attributes before
searching the document for an `iframe[name]` that matches.

Additionally, it adds tests to cover a special-case scenario where there
**is** an `iframe[name=""]` **and** an element that targets it (for
example, `a[target=""]`).

For example, consider the following example (along with a Turbo-less
[JSFiddle][] that reproduces the behavior):

```html
<iframe name=""></iframe>

<a href="https://example.com" target="">Targets [name=""]</a>
```

When clicked, the `<a>` element drives the entire page. In our test
suite, there are test cases that cover this behavior, and ensure that
Turbo doesn't interfere in these scenarios.

[Google Adsense]: https://www.google.com/adsense/start/
[hotwired#834]: hotwired#834
[JSFiddle]: https://jsfiddle.net/hk6587oz/

src/observers/form_submit_observer.ts

@@ -58,11 +58,15 @@ function submissionDoesNotDismissDialog(form: HTMLFormElement, submitter?: HTMLE
 }
 
 function submissionDoesNotTargetIFrame(form: HTMLFormElement, submitter?: HTMLElement): boolean {
-  const target = submitter?.getAttribute("formtarget") || form.target
+  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
-  }
+    for (const element of document.getElementsByName(target)) {
+      if (element instanceof HTMLIFrameElement) return false
+    }
 
-  return true
+    return true
+  } else {
+    return true
+  }
 }

src/observers/link_click_observer.ts

@@ -71,9 +71,13 @@ export class LinkClickObserver {
 }
 
 function doesNotTargetIFrame(anchor: HTMLAnchorElement): boolean {
-  for (const element of document.getElementsByName(anchor.target)) {
-    if (element instanceof HTMLIFrameElement) return false
-  }
+  if (anchor.hasAttribute("target")) {
+    for (const element of document.getElementsByName(anchor.target)) {
+      if (element instanceof HTMLIFrameElement) return false
+    }
 
-  return true
+    return true
+  } else {
+    return true
+  }
 }

src/tests/fixtures/navigation.html

@@ -79,12 +79,34 @@ <h1>Navigation</h1>
         </turbo-toggle>
       </p>
       <p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
-      <p><a id="targets-iframe" href="/src/tests/fixtures/one.html" target="iframe">Targets iframe</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-iframe" action="/src/tests/fixtures/one.html" target="iframe">
+          <button>Targets iframe[name="iframe"]</button>
+        </form>
+      </p>
+      <p>
+        <form id="form-target-empty-name-iframe" action="/src/tests/fixtures/one.html" target="">
+          <button>Targets iframe[name=""]</button>
+        </form>
+      </p>
+      <p>
+        <form action="/src/tests/fixtures/one.html">
+          <button id="button-formtarget-iframe" formtarget="iframe">Targets iframe[name="iframe"]</button>
+        </form>
+      </p>
+      <p>
+        <form action="/src/tests/fixtures/one.html">
+          <button id="button-formtarget-empty-name-iframe" formtarget="">Targets iframe[name=""]</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>
     </section>
 
     <turbo-frame id="hello" disabled></turbo-frame>
 
     <iframe name="iframe"></iframe>
+    <iframe name=""></iframe>
   </body>
 </html>

src/tests/functional/navigation_tests.ts

@@ -11,6 +11,7 @@ import {
   nextEventNamed,
   noNextEventNamed,
   pathname,
+  pathnameForIFrame,
   readEventLogs,
   search,
   selectorHasFocus,
@@ -428,9 +429,57 @@ test("test navigating back whilst a visit is in-flight", async ({ page }) => {
   assert.equal(await visitAction(page), "restore")
 })
 
-test("test ignores links that target an iframe", async ({ page }) => {
-  await page.click("#targets-iframe")
+test("test ignores links with a [target] attribute that target an iframe with a matching [name]", async ({ page }) => {
+  await page.click("#link-target-iframe")
   await nextBeat()
+  await noNextEventNamed(page, "turbo:load")
 
   assert.equal(pathname(page.url()), "/src/tests/fixtures/navigation.html")
+  assert.equal(await pathnameForIFrame(page, "iframe"), "/src/tests/fixtures/one.html")
+})
+
+test("test ignores links with a [target] attribute that targets an iframe with [name='']", async ({ page }) => {
+  await page.click("#link-target-empty-name-iframe")
+  await nextBeat()
+  await noNextEventNamed(page, "turbo:load")
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
+})
+
+test("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()
+  await noNextEventNamed(page, "turbo:load")
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/navigation.html")
+  assert.equal(await pathnameForIFrame(page, "iframe"), "/src/tests/fixtures/one.html")
+})
+
+test("test ignores forms with a button[formtarget] attribute that targets an iframe with [name='']", async ({
+  page,
+}) => {
+  await page.click("#form-target-empty-name-iframe button")
+  await nextBeat()
+  await noNextEventNamed(page, "turbo:load")
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
+})
+
+test("test ignores forms with a button[formtarget] attribute that targets an iframe with a matching [name]", async ({
+  page,
+}) => {
+  await page.click("#button-formtarget-iframe")
+  await nextBeat()
+  await noNextEventNamed(page, "turbo:load")
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/navigation.html")
+  assert.equal(await pathnameForIFrame(page, "iframe"), "/src/tests/fixtures/one.html")
+})
+
+test("test ignores forms with a [target] attribute that target an iframe with [name='']", async ({ page }) => {
+  await page.click("#button-formtarget-empty-name-iframe")
+  await nextBeat()
+  await noNextEventNamed(page, "turbo:load")
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/one.html")
 })

src/tests/helpers/page.ts

@@ -156,6 +156,17 @@ export function pathname(url: string): string {
   return pathname
 }
 
+export async function pathnameForIFrame(page: Page, name: string) {
+  const locator = await page.locator(`[name="${name}"]`)
+  const location = await locator.evaluate((iframe: HTMLIFrameElement) => iframe.contentWindow?.location)
+
+  if (location) {
+    return pathname(location.href)
+  } else {
+    return ""
+  }
+}
+
 export function propertyForSelector(page: Page, selector: string, propertyName: string): Promise<any> {
   return page.locator(selector).evaluate((element, propertyName) => (element as any)[propertyName], propertyName)
 }