Skip to content

fix(search): use URL mode per-tab instead of shared localStorage#12028

Open
ManthanNimodiya wants to merge 2 commits intointernetarchive:masterfrom
ManthanNimodiya:fix/search-mode-cross-tab-pollution
Open

fix(search): use URL mode per-tab instead of shared localStorage#12028
ManthanNimodiya wants to merge 2 commits intointernetarchive:masterfrom
ManthanNimodiya:fix/search-mode-cross-tab-pollution

Conversation

@ManthanNimodiya
Copy link

Summary

Fixes #11960

The search mode (?mode=everything / ?mode=ebooks) was being written to localStorage on every page load in ol.js:

if (urlParams.mode) {
    searchMode.write(urlParams.mode); // pollutes shared localStorage
}

Changes

  • ol.js — Removed searchMode.write(urlParams.mode); passes urlParams down to SearchPage
  • SearchBar.js — Added this._activeMode (URL mode takes priority, falls back to localStorage for homepage); replaced eager mode.sync with a deferred sync that only fires on explicit user radio interaction; composeSearchUrl gets an optional mode param; submitForm and renderAutocompletionResults use this._activeMode
  • SearchPage.js — Accepts urlParams, stores _urlMode; updateModeInputs prefers _urlMode until user clicks a radio button, then hands back to localStorage naturally

Testing

  1. Open two tabs: Tab A /search?q=python&mode=everything, Tab B /search?q=python&mode=ebooks
  2. Each tab's radio must reflect its own URL — switching tabs must not affect the other
  3. Submit from Tab A without touching the radio → result URL keeps mode=everything
  4. Homepage (no ?mode= in URL): radio reflects localStorage fallback as before
  5. Click radio → localStorage updates → next search on that tab uses the new value

Fixes internetarchive#11960

Signed-off-by: ManthanNimodiya <manthannimodiya989898@gmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a cross-tab state leak where mode (ebooks vs everything) could be overwritten via shared localStorage, causing mismatched radio state and inconsistent search URLs across tabs.

Changes:

  • Stop writing ?mode= from the URL into shared localStorage on page load.
  • Make SearchBar and SearchPage prefer URL-provided mode per-tab until the user explicitly changes the radio button, then allow localStorage to take over naturally.
  • Extend composeSearchUrl to accept an optional mode override so submissions/autocomplete preserve per-tab mode.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
openlibrary/plugins/openlibrary/js/ol.js Removes eager localStorage write on load; passes parsed URL params into SearchBar/SearchPage.
openlibrary/plugins/openlibrary/js/SearchPage.js Adds per-tab URL-mode override logic for mode inputs until user interaction.
openlibrary/plugins/openlibrary/js/SearchBar.js Tracks _activeMode (URL-first) and uses it for submits/autocomplete; defers mode.sync init firing.

Addresses PR review: normalizeMode() in SearchUtils guards SearchBar
and SearchPage against invalid/unsanitized urlParams.mode values.
Also adds JSDoc for composeSearchUrl mode param.

Signed-off-by: ManthanNimodiya <manthannimodiya989898@gmail.com>
Copy link
Contributor

@akramcodez akramcodez left a comment

Choose a reason for hiding this comment

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

LGTM

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.

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

4 participants