explorer: fix URL state round-trip (closes #201 parts A+B)#203
Merged
Conversation
Bug A: camera-handler URL write was deferred behind the resolution-load and point-mode-transition awaits inside the debounced callback. When a user panned into a region that triggered cluster→res8 or cold-cache point-mode (60–90s on first load per isamplesorg#190), the URL didn't update until those awaits settled — and a URL copy in the meantime captured the PREVIOUS camera state. Reproduced empirically: iter 2 of the harness moved the camera but `location.href` stayed at the original deep-link. Fix: write the URL hash via `history.replaceState` at the START of the debounced callback, before any awaits. The camera position/heading is known synchronously. The existing trailing write is kept so any mode change produced by the awaits still gets captured. Bug B: deep-link `mode=point` was silently ignored when the URL omitted `heading` (i.e. heading at default 0). The boot path positions the camera via `camera.setView`, which in some cases does not raise `camera.changed`, so the camera-handler that drives the cluster→point transition never runs. Reproduced: iters 3 and 6 of the harness (URLs without `heading=`) failed to enter point mode in a fresh tab; iters 4 and 5 (with non-zero heading) succeeded. Fix: explicit `tryEnterPointModeIfNeeded()` call at the end of the deep-link boot path when `ih.mode === 'point'`. The helper short-circuits if alt >= ENTER_POINT_ALT or we're already in point mode, so this is a no-op for cluster deep-links. Verification: tests/playwright/url_roundtrip_investigation.js, run against localhost with these fixes — all 6 iterations now pass (cf. 4 failures against the live site pre-fix). Harness is shipped under tests/playwright/ as a regression artifact and starting point for a proper spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 11, 2026
rdhyee
added a commit
that referenced
this pull request
May 11, 2026
#203 fixed the await-blocking case for Bug A but a residual remained: small camera moves below Cesium's `percentageChanged` threshold (set to 0.1 at line 2043) don't fire `camera.changed`, so the debounced callback containing the URL write never runs. A user who pans a small distance and immediately copies the URL gets stale state. Empirically confirmed via `url_roundtrip_investigation.js` against the post-#203 deploy: iter 2 (Δlat ≈ 0.02° pan-only, no zoom) had Context A at the new camera position but `location.href` still at the original deep-link, so Context B opened the stale URL. Fix: add `viewer.camera.moveEnd` as a parallel URL-write trigger. `moveEnd` fires once per discrete settled camera move regardless of magnitude (mouseup on drag-pan, wheel-stop on zoom, flight-complete on flyTo), so every position the user can reach gets captured. Keeps `camera.changed` for its existing role (mode transitions, resolution loads); just adds a lightweight URL-only backstop. The `_suppressHashWrite` gate already handles the hashchange-flight path correctly, so no fight with back/forward navigation. Verification: harness now passes all 6 iterations against localhost with this fix (post-#203 it failed on iter 2). Co-authored-by: Claude Opus 4.7 (1M context) <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
1 participant
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.
Closes #201 (parts A + B). Part 1 — the misleading 'Samples in View' counter — is left for a separate PR.
What this changes
Two small, independent fixes in
explorer.qmd:Bug A — camera-handler URL write was deferred behind awaits
explorer.qmd:1965-2030— the camera-changed handler's debounced callback awaitsloadRes/tryEnterPointModeIfNeeded(cold-cache point-mode boot can be 60–90s per #190) before reaching the trailinghistory.replaceState. When a user pans into a region that triggers a resolution change or point-mode transition, the URL write is deferred until those awaits settle. A URL copy in the meantime captures the previous camera state.Fix: write the URL hash at the START of the debounced callback, before any awaits. The camera state is known synchronously. The trailing write is kept so any mode change produced by the awaits still gets captured.
Bug B — deep-link
mode=pointsilently ignored when heading=0explorer.qmdboot path positions the camera viacamera.setView, which in some cases (notably when the URL omitsheading=and it defaults to 0) does not raisecamera.changed. The camera-handler that drives the cluster→point transition therefore never runs, and the URL'smode=pointis ignored.Fix: explicit
tryEnterPointModeIfNeeded()call at the end of the deep-link boot path whenih.mode === 'point'. The helper short-circuits if alt >= ENTER_POINT_ALT or we're already in point mode, so this is a no-op for cluster deep-links.Verification
tests/playwright/url_roundtrip_investigation.jsis the harness that originally detected the bugs (see #201 comment with empirical evidence). It drives Context A through 6 deterministic pan/zoom steps from the Cyprus deep-link, then opens each resulting URL in a fresh Context B and probes at 5s/15s/30s.Harness logs land at
/tmp/url_roundtrip_log.json. Run withTEST_URL=http://localhost:5860/explorer.html node tests/playwright/url_roundtrip_investigation.js.Risks
replaceState(no history-entry growth). If the awaits don't change mode, the second write is identical to the first. Acceptable cost.tryEnterPointModeIfNeeded()being safe to call from boot. Reviewed: it short-circuits onmode === 'point'and on alt >= ENTER_POINT_ALT, and uses the sameloadRespath that the camera handler would. Functionally equivalent to "what would happen ifcamera.changedfired."setView-doesn't-fire-camera.changed is the underlying cause of Bug B's mechanism. Documented inline at the boot call site. This PR works around it; a longer-term fix would be to ensuresetViewraises the event (likely by tweaking the call shape or following with a no-op camera move).tests/playwright/but is a Node script, not a Playwright spec. A follow-up should adapt it into a proper*.spec.jsso it runs in CI. Tracked as a TODO in the harness file's header comment.Test plan
🤖 Generated with Claude Code