Fix search ?q= scroll overriding hash-fragment navigation (#14141)

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 <mark>
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

Author: cderv

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 <a> 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

tests/docs/playwright/html/search-highlight/_quarto.yml

@@ -8,4 +8,6 @@ website:
       - href: index.qmd
         text: Home
 
-format: html
+format:
+  html:
+    toc: true

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.

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);
 

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<boolean>(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).