Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that the turbo-frame header is not sent when the turbo-frame has a target of _top #1138

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

afcapel
Copy link
Collaborator

@afcapel afcapel commented Jan 23, 2024

With Instaclick enabled and given an HTML structure like this:

<turbo-frame id="frame" target="_top">
  <a href="/src/tests/fixtures/prefetched.html">Hover to prefetch me</a>
</turbo-frame>

When the user hovers over the link, the turbo-frame header should not be sent in the prefetch request because the wrapping turbo-frame has a target of _top.

If we don't respect this turbo-rails sees the turbo-frame header and renders a turbo-frame response with a minimal layout that doesn't include any assets in the head. When turbo processes the response it may find a missmatch in tracked assets and cause a reload.

//cc @davidalejandroaguilar

…as a target of _top

Given an HTML structure like this:

```html
<turbo-frame id="frame" target="_top">
  <a href="/src/tests/fixtures/prefetched.html">Hover to prefetch me</a>
</turbo-frame>
```

When the user hovers over the link, the turbo-frame header should not be sent
in a prefetch request because the wrapping turbo-frame has a target of `_top`.

If we don't respect this turbo-rails sees the turbo-frame header and renders
a turbo-frame response with a minimal layout that doesn't include any assets
in the head. When turbo processes the response it may find a missmatch in
tracked assets and cause a reload.
Copy link
Collaborator

@kevinmcconnell kevinmcconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Nice job chasing this one down!

@afcapel afcapel merged commit 5902f3b into main Jan 24, 2024
2 checks passed
@afcapel afcapel deleted the prefetch-headers branch January 24, 2024 15:02
@davidalejandroaguilar
Copy link
Contributor

Sorry I couldn't chime in on time, but thanks for this @afcapel!

afcapel added a commit to pfeiffer/turbo that referenced this pull request Jan 29, 2024
* origin/main:
  Introduce `turbo:{before-,}morph-{element,attribute}` events
  Turbo 8.0.0-beta.4
  Introduce data-turbo-track="dynamic" (hotwired#1140)
  Ensure that the turbo-frame header is not sent when the turbo-frame has a target of _top (hotwired#1138)
  Turbo 8.0.0-beta.3
  Fix attribute name (hotwired#1134)
  Add InstantClick behavior (hotwired#1101)
  Revert hotwired#926. (hotwired#1126)
  Keep Trix dynamic styles in the head (hotwired#1133)
  Remove unused stylesheets when navigating (hotwired#1128)
  Upgrade idiomorph to 0.3.0 (hotwired#1122)
  Debounce page refreshes triggered via Turbo streams
  Update copyright year to 2024 (hotwired#1118)
  Turbo 8.0.0-beta.2
  Set aria-busy on the form element during a form submission (hotwired#1110)
  Dispatch `turbo:before-fetch-{request,response}` during preloading (hotwired#1034)
afcapel added a commit that referenced this pull request Jan 29, 2024
This commit changes the behavior of frame navigations promoted with the
`[data-turbo-action]` attribute so that they don't include the
`Turbo-Frame` header in their requests. This mimics the existing
behavior of frame navigations promoted with `[data-turbo-target=_top]`.

If we don't do this turbo-rails will see the `Turbo-Frame` header and
render the response as a frame update, with a minimal layout that doesn't
include any assets. When Turbo receives the response it believes it's a
full page response in which the assets have gone stale and it issues a full
page reload with `tracked_element_mismatch` as the reason.

Fixes #1047

This is similar to #1138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants