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

Refactor history state management to use history.pushState() within 500ms of last user interaction. #1087

Closed
wants to merge 10 commits into from

Conversation

croxton
Copy link
Contributor

@croxton croxton commented Oct 17, 2022

Fix for #1076.

Safari on iOS skips history state entries when using navigating backwards/forwards IF they were created using history.pushState() more than approx 500ms after the last user interaction (link click, select menu change etc). This makes back/forwards buttons behave unpredictably with htmx applications on all current iPhones and iPads, given that network conditions and application response times may vary.

Currently htmx executes a history.pushState() once a response has been received. This PR moves the pushState to before the response has been received, and then updates the final intended URL with a history.replaceState() (since replaceState doesn't create a history entry, this isn't subject to the user interaction limit). This change has a knock-on effect on cache management, which is also addressed. There may be some other implications around error handling and redirects initiated by the response that will require review (since we can no longer assume that pushState is possible after waiting for a response to be received).

Some automated tests relating to hx-push-url are failing, and any help with that would be much appreciated. Manual tests seem to work as expected.

@1cg
Copy link
Contributor

1cg commented Oct 19, 2022

@croxton do you have a reference for this behavior for safari?

@croxton
Copy link
Contributor Author

croxton commented Oct 19, 2022

I couldn't track down any specific details of Safari's current history API implementation, but I did find this thread where various interventions to prevent pushState() abuse were discussed, and the figure of 500ms was first mentioned:

WICG/interventions#21

My testing strongly suggests Safari has indeed implemented the 500ms intervention, or something that approximates it, in the latest major version of iOS (I did not test older versions). Specifically, a call to pushState() after 500ms since user interaction creates a history entry (history.length is incremented) but that is somehow flagged to be skipped on browser back button press. A call to history.back() triggered by a click DOES work, confirming the entry exists. Clicking the forward button also skips the history state entry.

@croxton
Copy link
Contributor Author

croxton commented Oct 19, 2022

"Per WICG/interventions#21, some browsers have implemented a heuristic where pressing the back button skips certain entries in the joint session history. Generally, these are entries where there is no user interaction. The intent is to avoid "back trapping", e.g. if you arrive on a malicious site which does history.pushState() 10 times, this makes it hard to escape the site by pressing the back button.":
whatwg/html#7832

@1cg
Copy link
Contributor

1cg commented Oct 19, 2022

@croxton reading through the docs here and there, would the following logic make sense:

On every htmx request, regardless if it pushes state or not, issue an htmx.replaceState() that looks at the current history.state, doc title and path? Is my understand that, once we have this entry in place, we can mutate later, even after 500ms? And if we don't then nothing has really changed? Does that sound right to you?

@croxton
Copy link
Contributor Author

croxton commented Oct 19, 2022

replaceState() won't create an entry, it just modifies the entry at the top of the stack (the current page). I think of it like a git commit --amend but for the URL bar.

You need to do a pushState() within 500ms of the last user interaction to get a new entry, that the user can then navigate to with the back button. Without it the back button reverses out of the htmx entirely, to the last full page load in the browser tab.

So, for every htmx request where we know the URL will change I think we need to issue a pushState() at the time the request is fired, and then once the request is received (potentially more than 500ms later) change the final URL (if required) using replaceState(). A lot of the time (particularly with boosted links) the pushState url and the requestState url will be the same, but that's not always the case.

@1cg
Copy link
Contributor

1cg commented Oct 19, 2022

DAMMIT. OK, lemme think some more. (Sorry, grug brained)

…everted to original htmx behaviour). Only change URL on pushState if we know for sure what it will be (hx-push-url or hx-boost).
…ecify that it should not be cached in localStorage (useful for sensitive or session-specific data).
@croxton
Copy link
Contributor Author

croxton commented Oct 20, 2022

Couple of updates, which came from trying out this change on two more projects.

First, the point at which the current page is cached I changed back to on response received (i.e. reverted to how it always has worked in htmx). I was able to get that working without the currentPage url being mangled by the earlier pushState. That means except for pushState and the replaceState call points, things work much as before.

I also realised that data could enter localStorage that you may not want in there, when the current page is saved to cache. I added an option to use hx-history="false" anywhere in the html response to allow any individual response to not allow itself to be cached. A new history entry will still be created and the user can therefore navigate back/forward to the page with sensitive data on it, but htmx will just do a request to the live server instead of looking in the cache. I'm not sure if that's the best "htmx" approach so I'm totally happy to revert that if you have a better way to do it!

@croxton
Copy link
Contributor Author

croxton commented Oct 21, 2022

The main reason this change wasn't passing the hx-push-url automated tests was that history is set to disabled in the meta tag of the test index page:

<meta name="htmx-config" content='{"historyEnabled":false,"defaultSettleDelay":0}'>

I'm not sure why/if it worked before but for stuff to be written to the history cache that value would need to be true (the default), unless I'm misunderstanding what that config option is meant to do?

Anyway, the relevant automated tests now all pass if history is enabled.

@croxton
Copy link
Contributor Author

croxton commented Oct 25, 2022

I fixed HX-Location to work with the revised history state management.

@1cg
Copy link
Contributor

1cg commented Oct 30, 2022

Hi @croxton, given our conversation I'm still interested in the ignore history and normalize path parts of this change, and I'd like to keep your work around in a branch in case this behavior rears its ugly head in the future.

I'm planning on cutting the 1.8.3 release at the end of the week. No worries if your stuff doesn't make it in, but wanted to let you know in case you want it in the relase.

Thanks again for all your amazing work on this issue, sorry about the gray hairs!

@croxton
Copy link
Contributor Author

croxton commented Nov 4, 2022

Hey no problem at all. Those separate PRs are ready for review: #1112 and #1113

@1cg
Copy link
Contributor

1cg commented Dec 6, 2022

@croxton are you ok w/ me closing this for now? looking forward to what you do w/ htmx 2.0 history!

@croxton
Copy link
Contributor Author

croxton commented Dec 6, 2022

@1cg Sure please go ahead. I have some ideas which I need to ruminate on a little, and may be a little out of scope (not sure yet) but we'll see. Excited to see where this is going!

@1cg
Copy link
Contributor

1cg commented Dec 6, 2022

image

@1cg 1cg closed this Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants