Skip to content

Fix: debounce reorder sign requests, pushState URL updates, proactive subscriptions (fixes #14, #29, #30) - #76

Open
soubarnak wants to merge 1 commit into
freenet:mainfrom
soubarnak:fix/issues-14-29-30
Open

Fix: debounce reorder sign requests, pushState URL updates, proactive subscriptions (fixes #14, #29, #30)#76
soubarnak wants to merge 1 commit into
freenet:mainfrom
soubarnak:fix/issues-14-29-30

Conversation

@soubarnak

Copy link
Copy Markdown
Contributor

Fixes #14, #29, and #30.

Issue #14: swap_page_order now debounces sign requests behind a 500ms timer, coalescing rapid clicks (e.g. held arrow key at 30 Hz) into a single network UPDATE. Prevents updated_at drift from pushing timestamps minutes into the future. Local state updates remain immediate for visual feedback.

Issue #29: update_hash now uses history.pushState instead of replaceState, so the address bar IRI updates and back/forward navigation works for page switches. A popstate listener was added to handle browser back/forward events.

Issue #30: Added subscribe_to_known_sites() which fires always-on Subscribe requests for every known site's contract after the current delegate's KnownSites response arrives. Under the demand-driven hosting model (freenet-core#4642), this prevents published sites from being silently GC'd under peer churn.

Also updated AGENTS.md to note pushState alongside replaceState in the iframe constraints.

…ubscriptions

Fixes freenet#14, freenet#29, and freenet#30.

Issue freenet#14: swap_page_order now debounces sign requests behind a 500ms
timer, coalescing rapid reorder clicks into a single network UPDATE.
Prevents held-arrow-key drift from pushing updated_at minutes into the
future. Local state updates remain immediate for visual feedback.

Issue freenet#29: update_hash now uses history.pushState instead of
replaceState, so the address bar IRI updates and back/forward
navigation works for page switches. A popstate listener was added
to handle browser back/forward button events.

Issue freenet#30: Added subscribe_to_known_sites() which fires always-on
Subscribe requests for every known site's contract after the current
delegate's KnownSites response arrives. Under the demand-driven
hosting model (freenet-core#4642), this prevents published sites
from being silently GC'd under peer churn.

Also updated AGENTS.md to note pushState alongside replaceState in
the iframe constraints.
@sanity

sanity commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Reviewed this alongside #75. #75 is merged; this one I cannot merge yet, and I would rather give you the specifics than just decline.

The headline: it does not compile for the shipping target. cargo check -p delta-ui --target wasm32-unknown-unknown fails with four errors, all in the wasm32 half of queue_reorder_signs in ui/src/state.rs:

  • cannot find value ts_vec — the block binds (titles, contents, orders, timestamps) but the loop iterates &ts_vec
  • use of undeclared type Closurewasm_bindgen::closure::Closure is never imported
  • Some(&order) against Option<u32> — type mismatch
  • REORDER_DEBOUNCE_MS is u32, set_timeout wants i32

cargo test passes 256 tests because all of that sits behind #[cfg(target_arch = "wasm32")]. It also fails clippy --all-targets -- -D warnings and cargo fmt --check.

None of this was your fault to miss: CI had never run on this PR. Fork PRs here wait for maintainer approval, so the WASM build job never executed and you got no signal. That is a process failure on our side and I have approved the workflow.

Three things beyond the build, which matter more because they would survive a green CI:

1. The debounce loses writes. state.rs:

*REORDER_PENDING.write() = pages_owned;   // overwrite

The comment three lines above says the pages "accumulate ... so a single debounced flush covers the full migration set". The code replaces the vector. Swap A/B, then swap C/D within 500 ms: the second click cancels the first timer and overwrites pending with [C, D]. But swap_page_order has already applied A/B's new orders and bumped their timestamps in local state, so A and B are never signed and no UPDATE ships. The reorder looks applied until the next load from the network, then reverts. Two ordinary clicks; no held key needed.

Same shape: REORDER_PENDING and REORDER_TIMER are global rather than per-site, so reordering site X then site Y within the window drops X. And navigating away or closing the tab inside the window loses the reorder entirely, where previously it shipped immediately.

2. #14's actual harm is not addressed. The updated_at bump happens in the immediate local-state mutation, which still runs on every click. A held key still drives the timestamp minutes into the future; the debounce only reduces how many UPDATEs ship. Capping the drift is the half that fixes the stated problem.

3. The back button gets trapped. The new popstate listener calls handle_hash_navigationselect_pageupdate_hashpushState. So Back fires popstate, the app re-selects the page, and pushes a new entry for the destination it just arrived at: [A,B] → Back → [A,B,A] → Back → [A,B,A,B]. Forward history is destroyed and the user cannot leave Delta with Back. With replaceState the same listener was inert, which is presumably why it was replaceState.

Related: #29 asks for a copyable, bookmarkable address, and pushState changes the iframe's URL, not the top-level address bar. Delta runs inside the gateway's sandboxed iframe, which is why set_document_title posts __freenet_shell__ to the parent. A hash fix needs the same shell channel — there is currently only a type:'title' sender. Also, the added comment saying these events fire "from history.pushState calls" is not right: pushState fires neither hashchange nor popstate.

The proactive-subscription idea is sound and its ordering is correct, but it needs a fire-once latch: it sits in the one arm the code documents as firing repeatedly (register_delegate() re-issues load_known_sites() on every reconnect), and both of its immediate neighbours carry latches for exactly that reason. It is also partly redundant, since restore_known_sites already GETs each site and the GET response subscribes — the genuinely new coverage is sites whose GET returns NotFound, which is worth keeping.

Happy to look again whenever you push. Your pushState vs replaceState documentation note is a good addition and I would like to keep it regardless of what happens to the rest.

[AI-assisted - Claude]

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.

Bound updated_at drift on rapid page reorders

2 participants