Close Issue #834

[seanpdoyle]

Closes hotwired/turbo#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 "":

<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):

<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.

[rbgrouleff]

Sorry for bothering you. But is there an ETA on when this fix is released? Or is there some way I can start using it without a new release?

[adrienpoly]

you have the dev builds available here https://github.com/hotwired/dev-builds/tags

[rbgrouleff]

you have the dev builds available here https://github.com/hotwired/dev-builds/tags

Thanks, but there doesn't seem to be a build that contains this PR.

[marcoroth]

@adrienpoly sadly the dev-builds were turned of a few weeks ago in 65be4bd

[dhh]

They were busted. If someone wants to put in the effort to get them going again, please do!

[marcoroth]

@rbgrouleff this should work now:

yarn add @hotwired/turbo@https://github.com/hotwired/dev-builds/archive/@hotwired/turbo/ce327c2.tar.gz

[rbgrouleff]

Thank you, @marcoroth! I'll give it a try 🙏

[rbgrouleff]

Just a followup question: If I'm using turbo through the turbo-rails gem, is it enough to just update the turbo yarn package?

[marcoroth]

It depends. If you are using esbuild/webpack and if you are directly importing @hotwired/turbo in your JavaScript files you can just add it to your package.json:

// package.json
{
  "dependencies": { 
    /* ... */,
   "@hotwired/turbo": "https://github.com/hotwired/dev-builds/archive/@hotwired/turbo/ce327c2.tar.gz"
  }
}

However, if you are importing from @hotwired/turbo-rails you would need to add a yarn resolution:

// package.json
{
  "dependencies": { /* ... */ },
  "resolutions": {
    "@hotwired/turbo": "https://github.com/hotwired/dev-builds/archive/@hotwired/turbo/ce327c2.tar.gz"
  }
}

Sadly it's not quite as straight-forward with import maps due to the nature how they work and how the dev-builds are hosted.

[rbgrouleff]

I'm importing from @hotwired/turbo-rails. I've just pasted the resolutions part into packages.json and tested it out, and it is now working.

Thank you so much for your help, @marcoroth 🙏