Back/forward scroll restore: drop the pre-swap popstate listener that corrupted saved offsets#212
Merged
Merged
Conversation
…ved offsets shell.js registered restoreScroll on popstate as well as htmx:historyRestore. Because shell.js loads before htmx.min.js (both defer, template order), its popstate listener fires before htmx's — while the DEPARTING page is still mounted in #main-content but location already names the destination. That scrolled the departing page to the destination's saved offset (clamped to the departing page's max), which htmx:beforeHistorySave then persisted under the departing page's key: every back/forward traversal corrupted both entries, and a back → forward → back round trip lost the reading position entirely. htmx 2.0.4 fires htmx:historyRestore after the swap on BOTH history-cache paths — hit and server-refetch miss — so the popstate listener covered nothing. Drop it; keep the single htmx:historyRestore binding. Verified in headless Chrome (CDP, fixture archive): scroll /c/2 to 400, boosted nav to /media, back/forward/back — /c/2 restores 400 both times, /media restores 0, keys stay clean; a forced htmx history-cache miss (cacheMiss:true) also restores 400. TestShellScrollRestoreBindings pins the event wiring statically. Part of #190. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #190.
Fixes the back/forward reading-position corruption introduced with the #197 scroll-restore mechanism (shell.js).
Root cause
restoreScrollwas registered on popstate in addition tohtmx:historyRestore. shell.js loads before htmx.min.js (bothdefer, template order inpartials.html), so its popstate listener fires before htmx's own — while the departing page is still mounted in#main-contentbutlocationhas already flipped to the destination. It therefore scrolled the departing page to the destination's saved offset (clamped to the departing page's max). htmx's popstate handler then firedhtmx:beforeHistorySavefor the departing page, andsaveScrollpersisted that corrupted value under the departing page's key.Net effect, deterministically: scroll
/c/Nto 400, boosted-nav to/media, then Back →/media's key corrupts 0→24; Forward →/medialands at 24 and/c/N's key clobbers 400→24; Back again → reading position gone.Fix
Drop the popstate binding;
htmx:historyRestoreis the only restore trigger. The popstate listener covered nothing: the vendored htmx 2.0.4 fireshtmx:historyRestoreafter the swap on both history-cache paths — cache hit and server-refetch miss ({cacheMiss: true}) — and on popstates htmx ignores (non-htmx state entries) no swap happens, so touching the still-mounted page's scroller is exactly the corruption above. The block comment now documents why popstate must never restore.TestShellScrollRestoreBindingspins the wiring statically: restore bound tohtmx:historyRestoreonly, exactly one popstate listener (the idempotent tab-statesync), andsaveScrollkeyed bydetail.path.Verification (headless Chrome CDP, fixture archive, before → after)
5-step repro — scroll
/c/2to 400, boosted nav to/media, then traverse history:/c/2/mediakey 0→24 ❌/mediakey stays 0 ✅/media/c/2key 400→24 ❌/c/2key stays 400 ✅/c/2Cache-miss coverage: with
htmx-history-cacheforcibly cleared on/media, Back refetched/c/2from the server andhtmx:historyRestorefired withcacheMiss: true— scroller restored to 400, so the dropped listener loses no coverage on the no-snapshot path.Hygiene:
rm -rf .tools && make cssleaves the committed app.css byte-identical (drift guard green); fullgo test ./...passes.🤖 Generated with Claude Code
🤖 Posted on behalf of
@joestumpby Claude.