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

HX-Request and HX-Current-URL headers are missing in the request to restore from history #2013

Merged

Conversation

xhaggi
Copy link
Contributor

@xhaggi xhaggi commented Nov 20, 2023

Description

The request to restore a page from history is missing the HX-Request and HX-Current-URL request headers. The request headers documentation for HX-Request states that the HX-Request header is always present and has the value "true". Some third-party libraries such as htmx-spring-boot actually rely on this.

Testing

Added unit tests.

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@xhaggi xhaggi force-pushed the fix/hx-request-header-on-history-restore branch from e6c4240 to b76c981 Compare November 20, 2023 09:44
@alexpetros alexpetros added bug Something isn't working ready for review Issues that are ready to be considered for merging labels Nov 20, 2023
@alexpetros
Copy link
Collaborator

Looks great, thanks @xhaggi

@1cg
Copy link
Contributor

1cg commented Nov 20, 2023

@xhaggi what do you think the value of HX-Current-URL is in this case?

@xhaggi
Copy link
Contributor Author

xhaggi commented Nov 21, 2023

The same as for normal htmx requests getDocument().location.href.

@1cg 1cg merged commit b89e4f4 into bigskysoftware:dev Nov 30, 2023
1 check passed
@brablc
Copy link

brablc commented Jan 16, 2024

Adding HX-Request header to the history restore request made the header unusable for detection of partial/full page reload (the documentation does not say it is always present at this moment ;-) ). At the moment django-htmx is broken.

It is now necessary to check HX-History-Restore-Request additionally, and if it is present return full page (I smell some problems when using hx-history-elt).

The effect of this change is erratic behavior when local storage history is deleted or hx-history="false" is used - navigation back would bring partial page into the body and make the web application unusable.

@alexpetros
Copy link
Collaborator

the documentation does not say it is always present at this moment

The documentation also does not say that it will not be present for history restore requests, so that was always a questionable basis for building that behavior. We're happy to work with django-htmx to help them update the package though.

The effect of this change is erratic behavior when local storage history is deleted or hx-history="false" is used - navigation back would bring partial page into the body and make the web application unusable.

To be clear, is the effect of the broken django-htmx integration, or something about htmx itself?

@viktor2097
Copy link

viktor2097 commented Jan 16, 2024

the documentation does not say it is always present at this moment

The documentation also does not say that it will not be present for history restore requests, so that was always a questionable basis for building that behavior. We're happy to work with django-htmx to help them update the package though.

The effect of this change is erratic behavior when local storage history is deleted or hx-history="false" is used - navigation back would bring partial page into the body and make the web application unusable.

To be clear, is the effect of the broken django-htmx integration, or something about htmx itself?

Django-htmx is practically just a super thin middleware that makes accessing htmx related headers a bit easier.
At it's simplest, you do if request.htmx in your view which returns a boolean if the HX-Request header is present.

So the issue he explains is that it's not enough to check for HX-Request, you also need to check for HX-History-Restore-Request on every request.

I've personally tried to replicate the issue in multiple ways without any success. Feel free to skim through the issue, it's not really django-htmx specific.

@xhaggi
Copy link
Contributor Author

xhaggi commented Jan 16, 2024

Adding HX-Request header to the history restore request made the header unusable for detection of partial/full page reload

Why are you relying on this header to detect a partial or full page load? IMO the header indicates that it is just a request from htmx, nothing else.

@brablc
Copy link

brablc commented Jan 16, 2024

The documentation also does not say that it will not be present for history restore requests, so that was always a questionable basis for building that behavior.

Yes I agree, but there should be a canonical way how backend should distinguish full/partial page request on the same URL.

To be clear, is the effect of the broken django-htmx integration, or something about htmx itself?

I have only seen examples of backend integrations that use HX-Request alone, for instance angelofallars/htmx-go. So this commit probably introduced potential regression in many libraries.

@brablc
Copy link

brablc commented Jan 16, 2024

Why are you relying on this header to detect a partial or full page load? IMO the header indicates that it is just a request from htmx, nothing else.

I take advantage of pushing URLs to history (mainly hx-boost="true") - URLs can be bookmarked and shared and on non-htmx request full page needs to be loaded. This works fine - unless I use hx-history="false" to avoid caching - such page needs to be loaded fully when navigating back to it (as revealed on https://htmx.org/docs/#history ).

@viktor2097
Copy link

viktor2097 commented Jan 16, 2024

I've reviewed the sample project provided by @brablc

It's related to attributes that push history, like hx-boost, hx-push-url in combination with hx-history false

Take this simple code as an example:

  <div id="gl"></div>
  <button
  hx-get="{{ request.path }}" hx-target="#gl" hx-push-url="true" hx-history="false">test</button>

If you load your page and press this button and try to go back, it will issue a htmx request, so checking for hx-request header will simply not be enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants