close
Skip to content

chore(search): remove dead search-mode localStorage plumbing - #13124

Merged
cdrini merged 1 commit into
internetarchive:masterfrom
lokesh:11960/chore/remove-dead-search-mode
Jul 10, 2026
Merged

chore(search): remove dead search-mode localStorage plumbing#13124
cdrini merged 1 commit into
internetarchive:masterfrom
lokesh:11960/chore/remove-dead-search-mode

Conversation

@lokesh

@lokesh lokesh commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #11960

Technical

The mode (everything/ebooks) value is a leftover from the legacy search bar and is now fully dead:

  • The everything/ebooks radio buttons no longer exist — the search page uses the availability toggle + language filter, which keep their state in per-tab sessionStorage with the URL as source of truth.
  • The mode query param has never been read server-side (verified in git history — even before the new search UI, worksearch/code.py had no mode handling). The mode→has_fulltext translation always lived client-side in the legacy SearchBar.js, which was removed in ac18f7a. External API clients that still send mode= to /search.json are unaffected — the param was already ignored.

But three vestiges survived and still leaked a stale mode value across tabs via localStorage (harmless, since nothing reads it — just confusing URL noise). This PR removes them:

  • SearchUtils.js deleted — its PersistentValue class was only used for mode (+ its test file)
  • ol.js no longer mirrors ?mode= into localStorage on every page load
  • SearchModal.js no longer appends mode= to the /search and /search.json URLs it builds

Net: −205 lines, and search URLs stop carrying a mode param that could contradict the actual filter state — the last remnant of the cross-tab bug in #11960.

Note: stale mode keys already in patrons' localStorage are simply left behind — nothing reads them, so no migration/cleanup code is needed.

Safety checks (static)

  • Repo-wide grep: zero remaining references to SearchUtils, searchMode, or PersistentValue
  • No other JS reads the mode localStorage key
  • No template or Python code emits mode= URLs
  • npm run test:js — 457 tests pass; eslint + pre-commit clean

Testing (live, local dev)

Rebuilt the JS bundle on this branch and exercised the full search flow in the browser:

  1. Seeded localStorage.mode = 'ebooks' (simulating the stale cross-tab value from ebook vs everything radio button status not always correctly displayed on search page #11960), then loaded /?mode=everything → localStorage untouched (old code would have overwritten it)
  2. Opened the header search modal, typed a query → both /search.json requests (results + readable count) returned 200 with no mode param, and the stale localStorage value did not leak into them; results rendered normally
  3. Pressed Enter → landed on a clean /search?q=... URL (no mode), results page rendered with the filter row
  4. Flipped "Readable Only" → navigated to /search?q=...&has_fulltext=true as expected
  5. Backward compat: loaded a legacy bookmark-style URL /search?q=...&mode=ebooks&has_fulltext=true → page renders fine, toggle correctly shows ON (derived from has_fulltext, mode ignored)
  6. No new console errors (one pre-existing my-books.js dropper exception, unrelated)

Stakeholders

@cdrini

The `mode` (everything/ebooks) value is a leftover from the legacy
search bar. Nothing reads it anymore: the radio buttons are gone,
ebook filtering happens via explicit availability params, and the
server ignores the `mode` query param entirely.

Remove the vestiges:
- SearchUtils.js (PersistentValue was only used for `mode`) and its test
- ol.js no longer mirrors `?mode=` into localStorage on page load
- SearchModal no longer appends `mode=` to /search and /search.json URLs

This also stops search URLs from carrying a stale cross-tab `mode`
value, the last confusing remnant of internetarchive#11960.

Closes internetarchive#11960
@lokesh lokesh added Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed] Needs: Review This issue/PR needs to be reviewed in order to be closed or merged (see comments). [managed] labels Jul 6, 2026

@cdrini cdrini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice lgtm! I know the authors page still shows ?mode=ebooks, but it looks like its not wired with this in any way.

@cdrini
cdrini merged commit 6b1afb4 into internetarchive:master Jul 10, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Review This issue/PR needs to be reviewed in order to be closed or merged (see comments). [managed] Type: Refactor/Clean-up Issues related to reorganization/clean-up of data or code (e.g. for maintainability). [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ebook vs everything radio button status not always correctly displayed on search page

2 participants