diff --git a/src/components/LocationProvider/LocationProvider.tsx b/src/components/LocationProvider/LocationProvider.tsx index bd729298..7243a183 100644 --- a/src/components/LocationProvider/LocationProvider.tsx +++ b/src/components/LocationProvider/LocationProvider.tsx @@ -83,7 +83,13 @@ export function LocationProvider({ children }: ILocationProviderProps) { section, split: resolvedSplit, })); - handleSetLocationParams({ version, search, section, split: resolvedSplit }); + // Redirect to the canonical URL, but keep ?search/?section so the second + // hashchange (triggered by the rewrite) doesn't wipe state before the + // Search input and Outline scroll have a chance to consume them. + const redirectHash = locationParamsToHash({ version, search, section, split: resolvedSplit }, metadata, { + includeSearchSection: true, + }); + window.location.hash = redirectHash; return; } @@ -101,7 +107,10 @@ export function LocationProvider({ children }: ILocationProviderProps) { section, split: resolvedSplit, })); - handleSetLocationParams({ version, search, section, split: resolvedSplit }); + const redirectHash = locationParamsToHash({ version, search, section, split: resolvedSplit }, metadata, { + includeSearchSection: true, + }); + window.location.hash = redirectHash; return; } @@ -143,7 +152,7 @@ export function LocationProvider({ children }: ILocationProviderProps) { } return params; }); - }, [handleSetLocationParams, metadata, resolveFullVersion]); + }, [metadata, resolveFullVersion]); const synctexBlocksToSelectionParams: ILocationContext["synctexBlocksToSelectionParams"] = useCallback((blocks) => { const blockIds = blocks.map((block) => ({ pageNumber: block.pageNumber, index: block.index })); diff --git a/src/components/LocationProvider/utils/locationParamsToHash.ts b/src/components/LocationProvider/utils/locationParamsToHash.ts index 74f7506d..4a1b0b75 100644 --- a/src/components/LocationProvider/utils/locationParamsToHash.ts +++ b/src/components/LocationProvider/utils/locationParamsToHash.ts @@ -8,7 +8,11 @@ import { } from "./constants"; import { encodePageNumberAndIndex } from "./encodePageNumberAndIndex"; -export const locationParamsToHash = (params: ILocationParams, metadata: IMetadataContext["metadata"]) => { +export const locationParamsToHash = ( + params: ILocationParams, + metadata: IMetadataContext["metadata"], + options: { includeSearchSection?: boolean } = {}, +) => { const fullVersion = params.version; const version = fullVersion ? fullVersion.substring(0, SHORT_COMMIT_HASH_LENGTH) @@ -29,13 +33,20 @@ export const locationParamsToHash = (params: ILocationParams, metadata: IMetadat ].join(""); } - // we never put search/section to the URL, - // yet we keep them in `locationParams`. + // search/section are URL-only "intents" consumed by Search input and Outline scroll. + // They are normally dropped from the serialized URL once consumed; the redirect path + // (when the URL had no resolvable version) opts in via `includeSearchSection` so the + // pre-consumption values survive the rewrite. const finalParamsToSerialize: SearchParams = { v: versionName, rest: `${SEGMENT_SEPARATOR}${stringifiedParams.join(SEGMENT_SEPARATOR)}`, }; + if (options.includeSearchSection) { + if (params.search) finalParamsToSerialize.search = params.search; + if (params.section) finalParamsToSerialize.section = params.section; + } + if (params.split) { const splitName = metadata.versions[params.split]?.name ?? (params.split === metadata.nightly?.hash ? "nightly" : null); diff --git a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-options-menu-dark-mode-linux.png b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-options-menu-dark-mode-linux.png index f0767c2e..132c51d9 100644 Binary files a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-options-menu-dark-mode-linux.png and b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-options-menu-dark-mode-linux.png differ diff --git a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-version-dropdown-light-mode-linux.png b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-version-dropdown-light-mode-linux.png index 007bca10..17177e57 100644 Binary files a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-version-dropdown-light-mode-linux.png and b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-header-version-dropdown-light-mode-linux.png differ diff --git a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-closed-dark-mode-linux.png b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-closed-dark-mode-linux.png index 2308ad95..6492616c 100644 Binary files a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-closed-dark-mode-linux.png and b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-closed-dark-mode-linux.png differ diff --git a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-dark-mode-linux.png b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-dark-mode-linux.png index d44fdcba..78ed4a62 100644 Binary files a/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-dark-mode-linux.png and b/tools/snapshot-tests/tests/split-screen.spec.ts-snapshots/split-sidebar-overlay-dark-mode-linux.png differ diff --git a/tools/snapshot-tests/tests/url-params.spec.ts b/tools/snapshot-tests/tests/url-params.spec.ts new file mode 100644 index 00000000..746eccbe --- /dev/null +++ b/tools/snapshot-tests/tests/url-params.spec.ts @@ -0,0 +1,69 @@ +import { expect, test } from "@playwright/test"; +import { waitForPdfReady } from "./utils/wait-for-pdf"; + +const port = process.env.PLAYWRIGHT_PORT || "5173"; +const host = process.env.PLAYWRIGHT_HOST || "localhost"; +const origin = `http://${host}:${port}`; + +const SEARCH_INPUT = 'input[placeholder*="search"]'; +const PDF_SCROLL_CONTAINER = ".pdfViewer"; + +async function getScrollTop(page: import("@playwright/test").Page) { + return await page.evaluate(() => { + const viewer = document.querySelector(".pdfViewer"); + return viewer?.parentElement?.scrollTop ?? 0; + }); +} + +test.describe("URL ?search and ?section parsing", () => { + test("fills search input when URL has ?search (no version segment)", async ({ page }) => { + await page.goto(`${origin}/#/?search=protocol`, { waitUntil: "networkidle" }); + await waitForPdfReady(page); + + const input = page.locator(SEARCH_INPUT).first(); + await expect(input).toHaveValue("protocol"); + }); + + test("fills search input when URL has ?search with a version segment", async ({ page }) => { + await page.goto(`${origin}/#/0.7.2?search=protocol`, { waitUntil: "networkidle" }); + await waitForPdfReady(page); + + const input = page.locator(SEARCH_INPUT).first(); + await expect(input).toHaveValue("protocol"); + }); + + test("scrolls to section when URL has ?section (no version segment)", async ({ page }) => { + await page.goto(`${origin}/#/?section=Header`, { waitUntil: "networkidle" }); + await waitForPdfReady(page); + await page.locator(PDF_SCROLL_CONTAINER).waitFor({ state: "visible" }); + + await expect.poll(() => getScrollTop(page), { timeout: 8_000, intervals: [200] }).toBeGreaterThan(1_000); + }); + + test("scrolls to section AND fills search when URL has both", async ({ page }) => { + await page.goto(`${origin}/#/?search=hash§ion=Header`, { waitUntil: "networkidle" }); + await waitForPdfReady(page); + + const input = page.locator(SEARCH_INPUT).first(); + await expect(input).toHaveValue("hash"); + + await expect.poll(() => getScrollTop(page), { timeout: 8_000, intervals: [200] }).toBeGreaterThan(1_000); + }); + + test("subsequent navigation drops ?search/?section from the URL", async ({ page }) => { + // Open with search/section so they end up in URL after the redirect. + await page.goto(`${origin}/#/?search=protocol§ion=Header`, { waitUntil: "networkidle" }); + await waitForPdfReady(page); + await expect(page).toHaveURL(/[?&]search=protocol/); + await expect(page).toHaveURL(/[?&]section=Header/); + + // Simulate the user navigating to a "plain" URL within the same SPA session + // (e.g. version change from the UI). The next URL must not retain search/section. + await page.evaluate(() => { + window.location.hash = "#/0.7.2?v=0.7.2"; + }); + + await expect(page).not.toHaveURL(/[?&]search=/); + await expect(page).not.toHaveURL(/[?&]section=/); + }); +});