From d0339575162f823959f0d994b6a38df82a6b5562 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 2 Mar 2026 15:13:00 +0100 Subject: [PATCH] Fix search ?q= scroll overriding hash-fragment navigation When clicking a search result linking to a specific section (e.g., page.html?q=term#section), scrollToFirstVisibleMatch() overrode the browser's hash scroll, jumping to the first near the top. Additionally, quarto-nav.js baked ?q= into all link hrefs during DOMContentLoaded, causing TOC clicks to trigger full-page reloads. Two fixes: - Move replaceState (removing ?q=) to module load time, before any DOMContentLoaded handlers run, so quarto-nav.js resolves link hrefs against a clean URL - Guard scrollToFirstVisibleMatch with a hash check so browser hash navigation is preserved for section-level search results --- .../projects/website/search/quarto-search.js | 27 ++++++++++++------- .../html/search-highlight/_quarto.yml | 4 ++- .../html/search-highlight/index.qmd | 8 ++++++ .../tests/html-search-highlight.spec.ts | 16 +++++++++++ .../tests/html-search-tabsets.spec.ts | 24 +++++++++++++++++ 5 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/resources/projects/website/search/quarto-search.js b/src/resources/projects/website/search/quarto-search.js index 8deeda2f79..437e38f1ac 100644 --- a/src/resources/projects/website/search/quarto-search.js +++ b/src/resources/projects/website/search/quarto-search.js @@ -8,6 +8,18 @@ const kResultsArg = "show-results"; // item is a more item (along with the type) and can be handled appropriately const kItemTypeMoreHref = "0767FDFD-0422-4E5A-BC8A-3BE11E5BBA05"; +// Capture search params and clean ?q= from URL at module load time, before +// any DOMContentLoaded handlers run. quarto-nav.js resolves all hrefs +// against window.location during DOMContentLoaded — if ?q= is still present, +// every link on the page gets the query param baked into its href. +const currentUrl = new URL(window.location); +const kQuery = currentUrl.searchParams.get(kQueryArg); +if (kQuery) { + const replacementUrl = new URL(window.location); + replacementUrl.searchParams.delete(kQueryArg); + window.history.replaceState({}, "", replacementUrl); +} + window.document.addEventListener("DOMContentLoaded", function (_event) { // Ensure that search is available on this page. If it isn't, // should return early and not do anything @@ -37,14 +49,12 @@ window.document.addEventListener("DOMContentLoaded", function (_event) { // Used to determine highlighting behavior for this page // A `q` query param is expected when the user follows a search // to this page - const currentUrl = new URL(window.location); - const query = currentUrl.searchParams.get(kQueryArg); + const query = kQuery; const showSearchResults = currentUrl.searchParams.get(kResultsArg); const mainEl = window.document.querySelector("main"); // highlight matches on the page if (query && mainEl) { - // perform any highlighting highlight(query, mainEl); // Activate tabs on pageshow — after tabsets.js restores localStorage state. @@ -56,14 +66,13 @@ window.document.addEventListener("DOMContentLoaded", function (_event) { for (const mark of mainEl.querySelectorAll("mark")) { openAllTabsetsContainingEl(mark); } - requestAnimationFrame(() => scrollToFirstVisibleMatch(mainEl)); + // Only scroll to first match when there's no hash fragment. + // With a hash, the browser already scrolled to the target section. + if (!currentUrl.hash) { + requestAnimationFrame(() => scrollToFirstVisibleMatch(mainEl)); + } } }, { once: true }); - - // fix up the URL to remove the q query param - const replacementUrl = new URL(window.location); - replacementUrl.searchParams.delete(kQueryArg); - window.history.replaceState({}, "", replacementUrl); } // function to clear highlighting on the page when the search query changes diff --git a/tests/docs/playwright/html/search-highlight/_quarto.yml b/tests/docs/playwright/html/search-highlight/_quarto.yml index 5230178921..628dde7ff4 100644 --- a/tests/docs/playwright/html/search-highlight/_quarto.yml +++ b/tests/docs/playwright/html/search-highlight/_quarto.yml @@ -8,4 +8,6 @@ website: - href: index.qmd text: Home -format: html +format: + html: + toc: true diff --git a/tests/docs/playwright/html/search-highlight/index.qmd b/tests/docs/playwright/html/search-highlight/index.qmd index 42d90d0b6b..73797ca2ee 100644 --- a/tests/docs/playwright/html/search-highlight/index.qmd +++ b/tests/docs/playwright/html/search-highlight/index.qmd @@ -2,6 +2,14 @@ title: "Search Highlight Test" --- +## First Section + This page contains a special keyword that we use for testing search highlighting. +## Second Section + The word special appears multiple times on this page to ensure search highlighting works correctly. + +## Third Section + +More content here for navigation testing. diff --git a/tests/integration/playwright/tests/html-search-highlight.spec.ts b/tests/integration/playwright/tests/html-search-highlight.spec.ts index 5b326751f8..cd4e4426a1 100644 --- a/tests/integration/playwright/tests/html-search-highlight.spec.ts +++ b/tests/integration/playwright/tests/html-search-highlight.spec.ts @@ -32,6 +32,22 @@ test('Search highlights cleared when query changes', async ({ page }) => { await expect(page.locator('main mark')).toHaveCount(0, { timeout: 2000 }); }); +test('TOC links do not retain search query parameter', async ({ page }) => { + await page.goto(`${BASE}?q=special`); + await page.waitForSelector('mark'); + + // Check that sidebar/TOC links don't contain ?q= + const tocLinks = page.locator('#TOC a[href], .sidebar-navigation a[href]'); + const count = await tocLinks.count(); + expect(count).toBeGreaterThan(0); + for (let i = 0; i < count; i++) { + const href = await tocLinks.nth(i).getAttribute('href'); + if (href) { + expect(href).not.toContain('q=special'); + } + } +}); + test('No highlights without search query', async ({ page }) => { await page.goto(BASE); diff --git a/tests/integration/playwright/tests/html-search-tabsets.spec.ts b/tests/integration/playwright/tests/html-search-tabsets.spec.ts index 67bdc9aa2b..4733ecbdba 100644 --- a/tests/integration/playwright/tests/html-search-tabsets.spec.ts +++ b/tests/integration/playwright/tests/html-search-tabsets.spec.ts @@ -104,6 +104,30 @@ test('Search activation overrides localStorage tab preference', async ({ page }) expect(await visibleMarkCount(page)).toBe(1); }); +test('Search with hash fragment scrolls to target section, not first match', async ({ page }) => { + // Use a very small viewport so mark and hash target can't both be visible + await page.setViewportSize({ width: 800, height: 200 }); + // Navigate with ?q= matching near the top AND #hash pointing to section further down + await page.goto(`${BASE}?q=beta-unique-search-term#grouped-tabset`); + + // Marks should exist (highlighting works) + const marks = page.locator('mark'); + await expect(marks.first()).toBeVisible({ timeout: 5000 }); + + // Wait for all scroll behavior to settle (rAF + smooth scroll animation) + await page.waitForFunction(() => { + return new Promise(resolve => { + requestAnimationFrame(() => requestAnimationFrame(() => { + setTimeout(() => resolve(true), 800); + })); + }); + }); + + // The hash target section should still be in viewport (not scrolled away to first mark) + const section = page.locator('#grouped-tabset'); + await expect(section).toBeInViewport(); +}); + test('Search scrolls to first visible match', async ({ page }) => { // Use small viewport so the nested tabset at the bottom is below the fold, // ensuring the test actually exercises scrollIntoView (not trivially passing).