Turbo doesn't send Turbo-Frame header when frame's src is updated

[steerio]

Turbo doesn't send Turbo-Frame header when frame's src is updated

[seanpdoyle]

@steerio I've opened #1198 to try and resolve this issue. I've added tests that aim to reproduce the issue as you've described it.

Unfortunately, they pass without any implementation changes. Could you provide some feedback on that PR to make those tests fail consistently?

[seanpdoyle]

Thank you for opening this issue. I've tried to reproduce it on main with the following change:

index f192590..c31bfc0 100644
--- a/src/tests/functional/frame_tests.js
+++ b/src/tests/functional/frame_tests.js
@@ -2,6 +2,7 @@ import { test, expect } from "@playwright/test"
 import { assert, Assertion } from "chai"
 import {
   attributeForSelector,
+  getSearchParam,
   hasSelector,
   listenForEventOnTarget,
   nextAttributeMutationNamed,
@@ -82,6 +83,21 @@ test("following a link sets the frame element's [src]", async ({ page }) => {
   assert.equal(src.searchParams.get("key"), "value", "[src] attribute encodes query parameters")
 })
 
+test("setting the element's [src] navigates the frame", async ({ page }) => {
+  const frame = page.locator("#frame")
+
+  await expect(frame.locator("h2")).toHaveText("Frames: #frame")
+  await frame.evaluate((frame) => frame.src = "/src/tests/fixtures/frames/frame.html?key=value")
+
+  const { url, fetchOptions: { headers } } = await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
+
+  expect(headers["Turbo-Frame"]).toEqual("frame")
+  expect(pathname(url)).toEqual("/src/tests/fixtures/frames/frame.html")
+  expect(getSearchParam(url, "key"), "fetch request encodes query parameters").toEqual("value")
+
+  await expect(frame.locator("h2")).toHaveText("Frame: Loaded")
+})
+
 test("following a link doesn't set the frame element's [src] if the link has [data-turbo-stream]", async ({ page }) => {
   await page.goto("/src/tests/fixtures/form.html")

Unfortunately, I was not able to fail the test. Does it reflect the circumstances in which you've encountered this unexpected behavior?

In case you want to try it locally, here's the block:

test("setting the element's [src] navigates the frame", async ({ page }) => {
  const frame = page.locator("#frame")

  await expect(frame.locator("h2")).toHaveText("Frames: #frame")
  await frame.evaluate((frame) => frame.src = "/src/tests/fixtures/frames/frame.html?key=value")

  const { url, fetchOptions: { headers } } = await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")

  expect(headers["Turbo-Frame"]).toEqual("frame")
  expect(pathname(url)).toEqual("/src/tests/fixtures/frames/frame.html")
  expect(getSearchParam(url, "key"), "fetch request encodes query parameters").toEqual("value")

  await expect(frame.locator("h2")).toHaveText("Frame: Loaded")
})

[steerio]

Thanks, Sean, I'll check that.

In the meantime I created this minimal Rails app to showcase the issue.

[seanpdoyle]

@steerio thank you for creating that repository!

As a piece of advice, git diffs can be really useful in reproduction repositories. In its current format, that repository has a single commit that includes both rails new-generated content alongside the specific changes you've made to demonstrate the issue. It's almost impossible to separate the two types of code at a glance without reading every Ruby, JavaScript, or HTML file and mentally comparing them to what I expect a new Rails application to have.

Could you revise the repository so that the "Intial commit" only contains the rails new code, then a second commit only contains the changes necessary to reproduce this behavior?

[steerio]

All right, I've done this and made a force push to the same repo, and here is the commit with the changes.

To see it in action you only need to bundle install and then rails s. Everything is on the front page.

It's worthy to note that if you downgrade turbo-rails with this change in the Gemfile, it all starts working.

-gem "turbo-rails"
+gem "turbo-rails", "<2"

[steerio]

I noticed something interesting with the demo code:

1. The page is already loaded on mouseover. Looks like this is InstaClick, which indeed has been introduced by v8.
2. The first click makes no request, hinting that the preload is being reused.
3. If you make two clicks (while managing to stay over the link), the second one will work properly.

So the problem is that the InstaClick preload is used even when the final request is not exactly the same as the preload one. In our case, when the request is preloaded, Turbo expects a visit to happen (and can't have any idea about our onclick intentions), so that request is made without the header we're expecting to be present.

The second click (provided that you didn't leave the link with the mouse before it) will make a proper request.

I think the situation can be rectified by adding the headers to the cache key. For the time being, I'll switch it off with a meta tag - even with that fix, we'd have two requests done instead of one.

[steerio]

To add a bit of an explanation: our use case is that some pages can be loaded in a modal and standalone as well. To tell these situations apart (and to give an ID to the Turbo Frame holding the content) we're using the Turbo-Frame header.

We also try to cut down on payload sizes for larger, Turbo-heavy pages, so in case of Turbo Frame requests, we try to only render what was requested.

We defined some handy helpers to help with this kind of thing:

• requested_frame: string, the value of the header basically.
• frame_request?: boolean, whether the header was present.
• frame_wanted?(id): boolean, true if the header equals to the argument or if the header was missing

It all worked well until turbo-rails 2 bumped the version of Turbo to v8.

[seanpdoyle]

I think you're correct that pre-fetching is at the root of this behavior.

In the meantime, are you able to render <meta name="turbo-prefetch" content="false"> into your <head> element?

[steerio]

Yes, that's what I ended up doing, and it works.

[seanpdoyle]

Looping in @afcapel @davidalejandroaguilar to help troubleshoot this behavior.

[steerio]

@seanpdoyle I don't readily have a system that would be able to run these tests (Arch has some issues with libpcre.so.3 and they didn't work on a Mac for some reason either). I'll set up a Debian in Docker, have them all pass, and see if I can write one that fails on this issue.

Just by glancing over it I'd say that it probably works with frame requests that it triggers itself, i.e. even at mouseover time it knows that when clicked, it would load in a given frame. In my case there was a Stimulus controller (which could be any event listener) on the click event with custom code that would do that.

This custom code simply updates the src attribute on a Turbo frame, and this should not blindly reuse the mouseover cache. Also, there could be a way to disable the prefetch feature on a per-link basis, because if this bug is fixed, there's still a useless extra request creating unnecessary load.

[seanpdoyle]

there could be a way to disable the prefetch feature on a per-link basis

Have you tried adding the [data-turbo-prefetch="false"] attribute to the link?

[steerio]

I haven't - I disabled the whole thing (which is fairly new to me, having only recently updated) via the meta tag. This looks like a nice alternative, thanks for pointing it out.