Fix listing page hash lost on a click right after a page change (#15233)

list.js rebuilds the pagination links on every update, and the handler
that writes the page hash (and blocks the links' href="#") was only
re-attached in a setTimeout after each update. A click on a rebuilt link
before that timer fired changed the page through list.js but followed
href="#", leaving the URL at index.html# with no page hash. This showed
up as an intermittent firefox failure of the page hash Playwright test on
the macOS nightly run.

The deferral could not simply be dropped: list.js dispatches "updated"
handlers in reverse registration order, so our handler runs before the
pagination plugin rebuilds the links. Instead, attach one click listener
per listing on its ul.pagination, which list.js keeps across updates,
so there is no window without a handler.

The new test clicks page 2 then page 3 in a single task, so no timer can
run between the update and the second click; it failed in chromium,
firefox and webkit before this change.

Author: cderv

src/resources/projects/website/listing/quarto-listing.js

@@ -48,19 +48,16 @@ window["quarto-listing-loaded"] = () => {
     // The actual list
     const list = window["quarto-listings"][listingId];
 
-    // Update the handlers for pagination events
-    refreshPaginationHandlers(listingId);
+    // Record the page hash on pagination clicks
+    attachPaginationHandler(listingId);
 
     // Render any visible items that need it
     renderVisibleProgressiveImages(list);
 
-    // Whenever the list is updated, we also need to
-    // attach handlers to the new pagination elements
-    // and refresh any newly visible items.
+    // Whenever the list is updated, refresh any newly visible items.
     list.on("updated", function () {
       renderVisibleProgressiveImages(list);
       window.quartoDecorateExternalLinks?.(list.list);
-      setTimeout(() => refreshPaginationHandlers(listingId));
 
       // Show or hide the no matching message
       toggleNoMatchingMessage(list);
@@ -130,19 +127,26 @@ function getListingPageKey(listingId) {
   return `${listingId}-page`;
 }
 
-function refreshPaginationHandlers(listingId) {
+function attachPaginationHandler(listingId) {
+  // list.js rebuilds the page links on every update but keeps their
+  // container, so a single listener there covers every link.
   const listingEl = window.document.getElementById(listingId);
-  const paginationEls = listingEl.querySelectorAll(
-    ".pagination li.page-item:not(.disabled) .page.page-link"
-  );
-  for (const paginationEl of paginationEls) {
+  const paginationEl = listingEl.querySelector(".pagination");
+  if (!paginationEl) {
+    return;
+  }
+  paginationEl.addEventListener("click", (event) => {
+    const pageLink = event.target.closest(
+      "li.page-item:not(.disabled) .page.page-link"
+    );
+    if (!pageLink) {
+      return;
+    }
     // list.js's pagination plugin shows the page from its own click
     // listener on the pagination list; this handler only records the hash.
-    paginationEl.onclick = (sender) => {
-      setPageHash(listingId, sender.target.getAttribute("data-i"));
-      return false;
-    };
-  }
+    event.preventDefault();
+    setPageHash(listingId, pageLink.getAttribute("data-i"));
+  });
 }
 
 function renderVisibleProgressiveImages(list) {

tests/integration/playwright/tests/listing-interactions.spec.ts

@@ -80,6 +80,23 @@ test.describe("Listing pagination", () => {
     await expect(page).toHaveURL(/#listing-second-listing-page=3$/);
   });
 
+  test("page link clicked right after an update writes the page hash", async ({ page }) => {
+    await page.goto(fixture);
+
+    // Click page 2 then page 3 in one task, so no timer runs between the
+    // update that rebuilds the links and the click on a rebuilt link.
+    await page.evaluate(() => {
+      const link = (n: string) =>
+        [...document.querySelectorAll<HTMLElement>("#second-listing-pagination .page.page-link")]
+          .find((el) => el.textContent?.trim() === n)!;
+      link("2").click();
+      link("3").click();
+    });
+
+    await expect(titles(page, "second-listing")).toHaveText(["Post 3"]);
+    await expect(page).toHaveURL(/#listing-second-listing-page=3$/);
+  });
+
   test("back restores the previous page", async ({ page }) => {
     await page.goto(fixture);
     await pageLink(page, "second-listing", "2").click();