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

Fix relative resource URLs for boosted links #1960

Merged
merged 1 commit into from Nov 19, 2023

Conversation

danpalmer
Copy link
Contributor

Relative resource URLs on boosted pages don't work. The order of operations is:

  1. XHR request is made for new content
  2. Content is swapped
  3. Browser evaluates new content, loading resources
  4. URL is updated

This means that relative URLs are evaluated in the context of the previous page. The correct order of operations should be:

  1. XHR request is made for new content
  2. URL is updated
  3. Content is swapped
  4. Browser evaluates new content, loading resources from URLs relative to requested URL.

This PR aims to resolve this issue. As history is disabled in the unit tests I think it's impossible to write an automated test for this change, so I've written a manual test. I've confirmed that the manual test fails without the change and succeeds after it.


I have not done a full manual test run myself as I'm not sure of the exact setup for doing so and don't have a range of browsers available. Also there are missing manual tests in the index and it's unclear if this intentional or if they are a little unmaintained? Any advice here would be appreciated.

@danpalmer
Copy link
Contributor Author

@1cg please have a look and let me know any feedback you'd like incorporated, or any risks of this. I'm new to HTMX and hit this roadblock early on so I may have missed ways in which this breaks other functionality. Hopefully we can get a fix for this issue merged soon though!

@alexpetros alexpetros added the bug Something isn't working label Nov 5, 2023
@alexpetros alexpetros changed the title Fix relative resource URLs Fix relative resource URLs for boosted links Nov 5, 2023
@danpalmer
Copy link
Contributor Author

Hey, friendly ping on this 😄 . Let me know if there's anything I can do to help get a fix for this issue shipped.

@1cg
Copy link
Contributor

1cg commented Nov 17, 2023

Hi Dan, sorry for the delay on this. History is one of the touchiest parts of htmx, so I'm going to need to spend some time w/ the PR to get comfortable w/ it.

Thank you!

@alexpetros alexpetros added the under discussion Issues that are being considered but require further alignment label Nov 17, 2023
@danpalmer
Copy link
Contributor Author

@1cg thanks, I understand. While I don't want to hurry things, this does feel like a major bug/regression, and while I can imagine this fix being incomplete in some way that I haven't realised, I hope the bug is clear and obvious – that history must be changed after the page content swap?

@1cg
Copy link
Contributor

1cg commented Nov 18, 2023

Hey Dan, do you mean to say that the page URL must be updated before the content swap?

@1cg 1cg merged commit 0b37d05 into bigskysoftware:dev Nov 19, 2023
1 check passed
@1cg
Copy link
Contributor

1cg commented Nov 19, 2023

OK, spent some time looking at the code and this is less dangerous than it looked at first and i agree it is the correct behavior

@danpalmer
Copy link
Contributor Author

Sorry didn't see this when it happened, but thanks for the review and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working under discussion Issues that are being considered but require further alignment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants