From 9691ecad8dba55e50eed86373ba93759fbe57e5b Mon Sep 17 00:00:00 2001 From: ael-dev3 <183124839+ael-dev3@users.noreply.github.com> Date: Sat, 24 Oct 2026 20:26:42 +0200 Subject: [PATCH] fix(web): ignore queued popover closes after navigation --- CHANGELOG.md | 4 ++ crates/allthing-web/src/components/popover.rs | 11 ++++- e2e/allthing.spec.ts | 9 +++- e2e/popover-lifecycle.fixture.spec.ts | 48 +++++++++++++++++-- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fb2cb2d..34adcdd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ All notable changes to Allthing are recorded here. The format follows [Keep a Ch ### Fixed +- A late popover-close event after navigation ignores disposed state, and cancelling a preview timer is idempotent. + - A video sound badge handles Space release explicitly after touch input, retaining focus and playback without depending on a synthesized native click. Held-key repeats do not toggle sound, and Enter retains its native button action. - Changing presence visibility or session while a heartbeat is in flight queues one refresh with the latest state, so the update is not lost behind the older response. - Write buttons retain their dialog accessibility attributes during permission discovery and key approval. A follow confirmed on the first hand-off check preserves its new follower count when the reader was known not to follow beforehand. @@ -38,6 +40,8 @@ All notable changes to Allthing are recorded here. The format follows [Keep a Ch ### Development +- Live reaction-list and feed-reply previews run as separate workflows, retaining every assertion and the strict per-test API request allowance while allowing the existing between-test pacing to drain speculative profile reads. + - Live sort checks wait for the interactive app; deep Back checks use Past week so posts aging out of a short range cannot masquerade as lost retained cards. - Profile website checks retain bounded browser errors and failed-request diagnostics when the app fails to leave its boot shell. - Post-opening performance checks retain bounded, sanitized hover, resource, frame, long-task and animation-frame diagnostics for each attempt, including failures. The click-to-post measurement, prefetch wait, sample count and strict 100 ms limit are unchanged. diff --git a/crates/allthing-web/src/components/popover.rs b/crates/allthing-web/src/components/popover.rs index 42463a97..43f84959 100644 --- a/crates/allthing-web/src/components/popover.rs +++ b/crates/allthing-web/src/components/popover.rs @@ -233,9 +233,11 @@ pub fn Popover( // or replace. The end of the closing animation has a timer of its own, // so nothing leaves a panel invisible but still in the page. let cancel = move || { - if let Some(handle) = timer.get_value() { + // Navigation can dispose a nested preview while an event that + // closes it is still being delivered. Taking a live timer is + // idempotent; a timer whose owner has gone needs no cancellation. + if let Some(Some(handle)) = timer.try_update_value(Option::take) { handle.clear(); - timer.set_value(None); } }; let schedule = move |delay: Duration, f: Box| { @@ -251,6 +253,11 @@ pub fn Popover( }; let open = move || open_by(false); let close = move || { + // An event already being dispatched can outlive the panel that + // registered it. Never touch that panel's other stored values. + if state.try_get_untracked().is_none() { + return; + } cancel(); pressing.set_value(false); if state.get_untracked() == State::Open { diff --git a/e2e/allthing.spec.ts b/e2e/allthing.spec.ts index e7a61cab..fb735042 100644 --- a/e2e/allthing.spec.ts +++ b/e2e/allthing.spec.ts @@ -838,7 +838,7 @@ test.describe("threads and navigation", () => { expect(Math.abs((await top()) - before)).toBeLessThan(30); }); - test("hovering likes, recasts and replies previews them; clicking opens the full list", async ({ page }) => { + test("hovering likes and recasts previews them; clicking opens the full list", async ({ page }) => { await page.goto(at(`/p/${NESTED_THREAD}`)); const foot = page.locator(".post .engagement"); await expect(foot).toBeVisible(); @@ -872,9 +872,14 @@ test.describe("threads and navigation", () => { await foot.locator(".stat-recasts .stat-count").focus(); await expect(page.getByRole("dialog", { name: "Recasted by" })).toBeVisible(); await page.keyboard.press("Escape"); + }); - // Replies preview from a card on the front page. + // A separate visit keeps this workflow inside the same strict per-test + // request allowance: reaction lists preload their people's profile facts. + // No requests, assertions or quota checks are removed. + test("hovering a replies count previews its replies clear of the header", async ({ page }) => { await page.goto(at("/")); + await appCame(page); const withReplies = page.locator(".cast-card").filter({ has: page.locator("button.stat-replies") }).first(); await expect(withReplies).toBeVisible(); await settleImages(page); diff --git a/e2e/popover-lifecycle.fixture.spec.ts b/e2e/popover-lifecycle.fixture.spec.ts index c36a4961..0b4ec748 100644 --- a/e2e/popover-lifecycle.fixture.spec.ts +++ b/e2e/popover-lifecycle.fixture.spec.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-or-later import { expect, test } from "@playwright/test"; import { NOW } from "./fixture-time"; +import { appCame } from "./helpers/app"; test.skip(Boolean(process.env.E2E_PAGES), "uses the Worker API contract"); const hash = `0x${"a".repeat(40)}`; @@ -39,7 +40,11 @@ test("Escape closes a keyboard popover and returns focus to its trigger", async await expect(panel).toHaveCount(0, { timeout: 1500 }); }); -test("navigating from an open popover removes its window click listener", async ({ page }) => { +test("navigating from an open popover removes its window click listener", async ({ page }, info) => { + const consoleErrors: string[] = []; + page.on("console", message => { + if (message.type() === "error" && consoleErrors.length < 12) consoleErrors.push(message.text().slice(0, 8000)); + }); const errors: string[] = []; page.on("pageerror", (error) => errors.push(error.stack ?? error.message)); await page.addInitScript(() => { @@ -56,6 +61,7 @@ test("navigating from an open popover removes its window click listener", async }) as typeof window.removeEventListener; }); await page.goto("/"); + await appCame(page); const trigger = page.getByRole("button", { name: "10 likes: show who", exact: true }); await expect(trigger).toBeVisible(); const before = await page.evaluate(() => { @@ -65,14 +71,14 @@ test("navigating from an open popover removes its window click listener", async }); await trigger.hover(); const panel = page.getByRole("dialog", { name: "Liked by", exact: true }); - await expect(panel.getByRole("link")).toBeVisible(); + await expect(panel.locator("a.who-link")).toBeVisible(); await expect.poll(() => page.evaluate(() => (window as TrackedWindow).activeClickListeners.size)).toBe(before + 1); await page.evaluate(() => { const tracked = window as TrackedWindow; tracked.popoverClickListeners = [...tracked.activeClickListeners].filter((listener) => !tracked.initialClickListeners.has(listener)); }); const document = await page.locator("html").elementHandle(); - await panel.getByRole("link").click(); + await panel.locator("a.who-link").click(); await expect(page).toHaveURL(/\/u\/reader$/); expect(await document!.evaluate((root) => root.isConnected).catch(() => false)).toBe(true); await expect(trigger).toHaveCount(0); @@ -81,6 +87,7 @@ test("navigating from an open popover removes its window click listener", async return tracked.popoverClickListeners.filter((listener) => tracked.activeClickListeners.has(listener)).length; }), { timeout: 1500 }).toBe(0); await page.locator("body").click({ position: { x: 5, y: 5 } }); + if (errors.length) await info.attach("popover-panic-console", { contentType: "application/json", body: JSON.stringify(consoleErrors) }); expect(errors).toEqual([]); }); @@ -135,3 +142,38 @@ test("Back after leaving nested previews does not wait for their closing animati await page.locator("body").click({ position: { x: 5, y: 5 } }); expect(errors).toEqual([]); }); + +// A close already queued by pointer leave can be delivered after navigation +// has cancelled its timer. Retain those callbacks to exercise that ordering. +test("a queued nested-preview close delivered after navigation is harmless", async ({ page }) => { + const errors: string[] = []; + page.on("pageerror", error => errors.push(error.stack ?? error.message)); + await page.addInitScript(() => { + const schedule = window.setTimeout.bind(window); + const clear = window.clearTimeout.bind(window); + const pending: { id: number; deliver: () => void }[] = []; + window.setTimeout = ((handler: TimerHandler, delay?: number, ...args: unknown[]) => { + if (delay !== 180 || typeof handler !== "function") return schedule(handler, delay, ...args); + const id = schedule(() => handler(...args), 60_000); + pending.push({ id, deliver: () => { clear(id); handler(...args); } }); + return id; + }) as typeof window.setTimeout; + (window as Window & { queuedPreviewClosings: () => number; deliverPreviewClosings: () => void }).queuedPreviewClosings = () => pending.length; + (window as Window & { deliverPreviewClosings: () => void }).deliverPreviewClosings = () => { + for (const callback of pending.splice(0)) callback.deliver(); + }; + }); + await page.goto("/"); + await page.getByRole("button", { name: "10 likes: show who", exact: true }).hover(); + const row = page.getByRole("dialog", { name: "Liked by", exact: true }).locator("a.who-link"); + await row.hover(); + await expect(page.getByRole("dialog", { name: "Profile: reader (@reader)", exact: true })).toBeVisible(); + await page.mouse.move(5, 5); + await expect.poll(() => page.evaluate(() => (window as Window & { queuedPreviewClosings: () => number }).queuedPreviewClosings())).toBeGreaterThanOrEqual(2); + await row.click(); + await expect(page).toHaveURL(/\/u\/reader$/); + await expect(page.getByRole("region", { name: "Profile", exact: true })).toBeVisible(); + await page.evaluate(() => (window as Window & { deliverPreviewClosings: () => void }).deliverPreviewClosings()); + await page.locator("body").click({ position: { x: 5, y: 5 } }); + expect(errors).toEqual([]); +});