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.

Sign up for GitHub

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

Jump to bottom

Head merge issue with turbo frame navigation promoted to a page visit #1237

Open
Cervenka opened this issue Apr 3, 2024 · 4 comments
Open

Head merge issue with turbo frame navigation promoted to a page visit #1237

Cervenka opened this issue Apr 3, 2024 · 4 comments

Comments

@Cervenka
Copy link

Cervenka commented Apr 3, 2024 •
edited

When using the data-turbo-action="advance" attribute the document's head is merged. (Handbook)
During that process meta-tags are removed from the head if the response's head doesn't include them. That includes meta-tags that may be required such as csrf-token or vapid-public-key.

I ran into this issue when using the custom layout provided by turbo-rails.

Thank you

Gems:
turbo-rails (2.0.5)

Packges:
@hotwired/turbo-rails@^8.0.4
@hotwired/turbo@^8.0.4
@Cervenka Cervenka changed the title Header merge issue with turbo frame navigation promoted to a page visit Head merge issue with turbo frame navigation promoted to a page visit Apr 3, 2024
@adampope
Copy link

adampope commented Apr 4, 2024

I have to just run into the exact same issue. I noticed because when navigating within the frame the browser tab title was being changed to the URL and I realised it was because the <title> tag and all <meta> tags were being removed from the <head>.

Initially I thought it was because we were using render partial: 'my_frame' to return just the frame, but swapping to rendering the whole view and using the turbo_rails/frame layout didn't improve the situation. Moving to render :show, layout: 'application' does work, as the <head> is present with all the tags to be replaced, but that's really inefficient to update the contents of one frame.

@adampope
Copy link

adampope commented Apr 9, 2024

I have found the problem in my application. I'm working on an old codebase that had been upgraded to use Turbo. In debugging this error I created a minimal reproduction using rails new and found that the contents of the <head> tag was preserved as you would expect when navigating. So I then started deleting code from the production app to get as close to the repro as possible. After deleting everything I could it still didn't work, so I started diffing the new app and the old app to see if there was anything I was missing. The magic change was

- stylesheet_link_tag "application"
+ stylesheet_link_tag "application",  "data-turbo-track": "reload"

In reverse, if I remove data-turbo-track from the minimal reproduction it exhibits the same behaviour of emptying the <head> of the title and meta tags when clicking a link with the data-turbo-action=advance option in a frame.

From reading the reference on attributes I'm not really sure why this should be the case, so I think this is probably a bug.

@adampope
Copy link

adampope commented Apr 9, 2024

I've now discovered that it isn't specific to the stylesheet, this behaviour happens with you have 0 occurrences of data-turbo-track. In my attempts to simplify my reproduction I had swapped

- <%= javascript_importmap_tags %>
+ <%= javascript_include_tag "turbo", type: "module" %>

The importmap tag appears to include data-turbo-track by default. If I go back to the importmap setup, then I can remove the data-turbo-track attribute from the stylesheet and everything continues to work. So the key seems to be to have at least one script or stylesheet tracked.

@Cervenka
Copy link
Author

Adding the data-turbo-track="reload" to an asset does fix the head merge issue. However it seems to break the back-forward-cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@adampope @Cervenka