diff --git a/frontend/editor/src/core/pages/HomePage.tsx b/frontend/editor/src/core/pages/HomePage.tsx index 305cb52eb2..df98f3695d 100644 --- a/frontend/editor/src/core/pages/HomePage.tsx +++ b/frontend/editor/src/core/pages/HomePage.tsx @@ -148,6 +148,7 @@ export default function HomePage() { syncFromUrl(); window.addEventListener("popstate", syncFromUrl); return () => window.removeEventListener("popstate", syncFromUrl); + // location.pathname is a trigger only - it marks a router commit. }, [location.pathname]); useEffect(() => { @@ -160,9 +161,8 @@ export default function HomePage() { // when opened directly on a /settings URL (deep link) - close falls back to // the editor root. const settingsOriginRef = useRef(null); - // Last route outside /settings. Openers that navigate straight to - // /settings/* (super search) have already replaced the URL by the time the - // modal opens, so the origin has to be remembered on the way past. + // Last route outside /settings. Super search pushes /settings/* through the + // router, so the app is already in settings when the modal opens. const lastNonSettingsPathRef = useRef(null); useEffect(() => { if (!location.pathname.startsWith("/settings")) { @@ -172,11 +172,9 @@ export default function HomePage() { const wasConfigOpenRef = useRef(false); useEffect(() => { if (configModalOpen && !wasConfigOpenRef.current) { - // Already in /settings when the modal opened: either a nav-driven open - // (fall back to the route we came from) or a genuine deep link (null). - settingsOriginRef.current = isInSettings() - ? lastNonSettingsPathRef.current - : location.pathname; + // One clock only: location.pathname can still hold a stale /settings + // path here, and storing it makes the next close re-open the modal. + settingsOriginRef.current = lastNonSettingsPathRef.current; } wasConfigOpenRef.current = configModalOpen; }, [configModalOpen, location.pathname]); diff --git a/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts b/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts index 020e0dbd1e..94d6ffcaa9 100644 --- a/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/console-clean.spec.ts @@ -82,6 +82,12 @@ async function attachInPageErrorCapture(page: Page): Promise { if (value instanceof Error) { return value.stack || `${value.name}: ${value.message}`; } + if (value instanceof Element) { + // JSON.stringify renders a DOM node as "{}" without throwing, so the + // object branch below would swallow the one detail that matters. + const el = value as HTMLImageElement & HTMLLinkElement; + return `${value.tagName}[${el.src || el.href || ""}]`; + } if (value && typeof value === "object") { try { return JSON.stringify(value); @@ -91,17 +97,20 @@ async function attachInPageErrorCapture(page: Page): Promise { } return String(value); }; - window.addEventListener("error", (event) => { - // Resource-load failures bubble here with no `error` object and never - // reach `pageerror`, so record where they came from rather than - // pretending they were throws. - const origin = event.filename - ? `${event.filename}:${event.lineno}:${event.colno}` - : describe(event.target); - sink.push( - `[window.onerror] ${event.message || "(sanitised)"} @ ${origin} :: ${describe(event.error)}`, - ); - }); + window.addEventListener( + "error", + (event) => { + // Resource-load failures only reach window in the capture phase and + // never reach `pageerror`, so record where they came from. + const origin = event.filename + ? `${event.filename}:${event.lineno}:${event.colno}` + : describe(event.target); + sink.push( + `[window.onerror] ${event.message || "(sanitised)"} @ ${origin} :: ${describe(event.error)}`, + ); + }, + true, + ); window.addEventListener("unhandledrejection", (event) => { sink.push(`[unhandledrejection] ${describe(event.reason)}`); }); diff --git a/frontend/editor/src/core/tests/stubbed/settings.spec.ts b/frontend/editor/src/core/tests/stubbed/settings.spec.ts index 198e9efa26..029a2b3a8a 100644 --- a/frontend/editor/src/core/tests/stubbed/settings.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/settings.spec.ts @@ -250,6 +250,19 @@ test.describe("Settings dialog", () => { .toBe(originPath); }); + test("close from a deep link leaves /settings", async ({ page }) => { + // Nothing to restore on a cold deep link, so close falls back to the + // editor root rather than pinning the URL in /settings. + await page.goto("/settings/general", { waitUntil: "domcontentloaded" }); + await expect(page.locator(".modal-container")).toBeVisible({ + timeout: 5_000, + }); + await closeSettings(page); + await expect + .poll(() => new URL(page.url()).pathname, { timeout: 5_000 }) + .not.toMatch(/\/settings/); + }); + test("close returns to origin URL when settings was opened from super search", async ({ page, }) => {