Skip to content

Move currentPathForHistory to session storage - #3330

Merged
1cg merged 4 commits into
bigskysoftware:devfrom
MichaelWest22:historypath
Jun 16, 2025
Merged

Move currentPathForHistory to session storage#3330
1cg merged 4 commits into
bigskysoftware:devfrom
MichaelWest22:historypath

Conversation

@MichaelWest22

@MichaelWest22 MichaelWest22 commented Jun 3, 2025

Copy link
Copy Markdown
Collaborator

Description

Moves the currentPathForHistory state variable to session storage so it is never lost. This variable is used to save the current URL you are on when a back/forward action popstates and changes the url so the current page can be captured against this URL for restoration later. This variable needs to stay intact during history restoration process and it is currently just a local to htmx let variable. If htmx script is re-executed during history navigation when the script is placed in the body this variable state is lost and htmx can backup history with the wrong URL causing weird history navigations. By moving it to sessionStorage it will be retained just for the current users session which is what we want.

This was originally in PR #3305 but was asked to split it out on its own to simplify review

Corresponding issue:
#2865

Testing

Ran test suite and also had to test history back forward actions in a test application to to validate it works as expected.

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

@croxton

croxton commented Jun 7, 2025

Copy link
Copy Markdown
Contributor

You're checking for canAccessLocalStorage() but writing to sessionStorage. So you probably need a corresponding function canAccessSessionStorage(). Specifically checking support is important because one or the other may be disabled in the browser's privacy settings.

I'd also be curious to test if things work as expected when the same site/app is open in multiple tabs - particularly when switching between tabs - given that localStorage is available to all tabs but sessionStorage is limited to the selected tab only (I think it should, but worth checking to be sure).

@MichaelWest22

MichaelWest22 commented Jun 7, 2025

Copy link
Copy Markdown
Collaborator Author

You're checking for canAccessLocalStorage() but writing to sessionStorage. So you probably need a corresponding function canAccessSessionStorage(). Specifically checking support is important because one or the other may be disabled in the browser's privacy settings.

Yeah this should be checking session storage. this change was taken out of another PR to split it #3305 and is designed to merged along with this other change but Carson wants them split up to make them easier to review. Once that one merges then this one will be checking session storage. Note we may have to leave the function name as canAccesslocalStorage because for some reason this function was exported to the extension internalApi so renaming it would break things. Also note that the local storage api's sessionStorage and localStorage are going to be either both disabled or both enabled in current browsers and so checking for one will also check for the other.

I'd also be curious to test if things work as expected when the same site/app is open in multiple tabs - particularly when switching between tabs - given that localStorage is available to all tabs but sessionStorage is limited to the selected tab only (I think it should, but worth checking to be sure).

Yes this change has to use session storage as if we were to use localStorage it would cause strange behaviour if you had two tabs open at the same time as the last page url you accessed would be used in tab A would be used as the url to save history for in a tab B navigation. We also need that other PR #3305 change merged to resolve this properly though as right now an htmx app that is opened in two tabs shares the same history cache storage so client state can move between tabs during back navigation..

@1cg
1cg merged commit c091b95 into bigskysoftware:dev Jun 16, 2025
1 check passed
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.

3 participants