From b69b787d636d58f4b6f01d33c1aa98e1cb1b79f2 Mon Sep 17 00:00:00 2001
From: James Brunton <jbrunton96@gmail.com>
Date: Mon, 6 Jul 2026 10:25:46 +0100
Subject: [PATCH] Fix Playwright tests in Firefox and Safari (#6868)

# Description of Changes
Playwright tests currently fail in Firefox and Safari because of
inconsistent behaviour across the browsers. This is causing the
nightlies to fail every night. This PR fixes the test behaviour to work
consistently across browsers (most of the issues were to do with the
tests opening the file picker, which was being automatically suppressed
in Chromium, but not the other browsers).
---
 .../src/core/tests/helpers/stub-test-base.ts  |  2 +
 .../src/core/tests/helpers/test-base.ts       |  2 +
 .../src/core/tests/helpers/ui-helpers.ts      | 46 +++++++++++++++++--
 .../live/encrypted-unlock-then-tool.spec.ts   |  3 ++
 .../stubbed/certificate-validation.spec.ts    |  5 ++
 .../src/core/tests/stubbed/convert.spec.ts    | 14 ++++--
 .../stubbed/encrypted-pdf-unlock.spec.ts      | 12 ++++-
 .../stubbed/files-page-screenshots.spec.ts    | 13 +++++-
 .../tests/stubbed/pdf-text-search.spec.ts     |  5 +-
 .../stubbed/viewer-text-selection.spec.ts     | 16 +++++++
 10 files changed, 105 insertions(+), 13 deletions(-)

diff --git a/frontend/editor/src/core/tests/helpers/stub-test-base.ts b/frontend/editor/src/core/tests/helpers/stub-test-base.ts
index 5752108d42..e6ae98c6f7 100644
--- a/frontend/editor/src/core/tests/helpers/stub-test-base.ts
+++ b/frontend/editor/src/core/tests/helpers/stub-test-base.ts
@@ -6,6 +6,7 @@ import {
   skipOnboarding,
   type MockAppApiOptions,
 } from "@app/tests/helpers/api-stubs";
+import { suppressNativeFilePicker } from "@app/tests/helpers/ui-helpers";
 
 /**
  * Custom Playwright fixture for backend-free specs.
@@ -57,6 +58,7 @@ export const test = base.extend<StubFixtures>({
   seedJwt: [false, { option: true }],
 
   page: async ({ page, stubOptions, autoGoto, seedJwt }, use) => {
+    suppressNativeFilePicker(page);
     await seedCookieConsent(page);
     if (seedJwt) {
       // Logged-in users hit the orchestrator path that surfaces the
diff --git a/frontend/editor/src/core/tests/helpers/test-base.ts b/frontend/editor/src/core/tests/helpers/test-base.ts
index 1659bc3abd..90de9432f9 100644
--- a/frontend/editor/src/core/tests/helpers/test-base.ts
+++ b/frontend/editor/src/core/tests/helpers/test-base.ts
@@ -1,6 +1,7 @@
 import { test as base, expect } from "@playwright/test";
 import * as fs from "node:fs/promises";
 import * as path from "node:path";
+import { suppressNativeFilePicker } from "@app/tests/helpers/ui-helpers";
 
 /**
  * Custom test fixture that:
@@ -40,6 +41,7 @@ const COVERAGE_DIR = path.resolve(
 
 export const test = base.extend({
   page: async ({ page }, use, testInfo) => {
+    suppressNativeFilePicker(page);
     await page.context().addCookies([
       {
         name: "cc_cookie",
diff --git a/frontend/editor/src/core/tests/helpers/ui-helpers.ts b/frontend/editor/src/core/tests/helpers/ui-helpers.ts
index 7c08f93f60..353037c106 100644
--- a/frontend/editor/src/core/tests/helpers/ui-helpers.ts
+++ b/frontend/editor/src/core/tests/helpers/ui-helpers.ts
@@ -11,6 +11,37 @@ import { expect, type Page, type Locator } from "@playwright/test";
 
 const MANTINE_MODAL_OVERLAY = ".mantine-Modal-overlay";
 
+/**
+ * Suppress the native OS file picker for the whole page, on every browser.
+ *
+ * Several upload entry points (the FileSidebar "Open from computer" button,
+ * the Mantine `<FileInput>`, AddFileCard, etc.) open a file dialog by clicking
+ * a hidden `<input type="file">`. On firefox/webkit Playwright only intercepts
+ * that dialog while the page has a `filechooser` listener - it toggles
+ * `Page.setInterceptFileChooserDialog` off the event subscription. With no
+ * listener the real OS picker leaks onto the host and hangs the nightly run.
+ *
+ * Registering a (no-op) `filechooser` listener flips that interception on for
+ * every browser, so the dialog is suppressed at the browser level however it
+ * was triggered - a programmatic `.click()`, a `<label>` activation, or
+ * Playwright's own click. We deliberately don't set files in the handler: specs
+ * still drive uploads explicitly via `setInputFiles()`, which sets files
+ * through the protocol regardless of the pending intercepted chooser. This lets
+ * a spec click the real entry-point button while the picker stays mocked
+ * cross-browser - unlike a global `HTMLInputElement.prototype.click` override,
+ * which misses `<label>`-triggered pickers and never enables Playwright's own
+ * interception.
+ *
+ * Installed once per page by the shared test fixtures (stub + live), so no spec
+ * has to opt in.
+ */
+export function suppressNativeFilePicker(page: Page): void {
+  page.on("filechooser", () => {
+    // Interception alone suppresses the native dialog; specs provide the files
+    // themselves via setInputFiles() on the hidden input.
+  });
+}
+
 /**
  * Wait for a Mantine Modal overlay to appear or disappear. Most file pickers,
  * settings dialogs, encrypted-PDF unlock prompts and so on render through
@@ -39,8 +70,10 @@ export async function waitForModalClose(
 /**
  * Upload one or more files through the FileSidebar's "Open from computer"
  * action. The button is always rendered (collapsed or expanded sidebar) and
- * triggers the hidden `data-testid="file-input"` native picker directly -
- * there is no modal to wait for under the post-refactor design.
+ * fires the hidden `data-testid="file-input"`. Its native OS picker is mocked
+ * globally by `suppressNativeFilePicker` (installed by the test fixtures), so
+ * the click is safe on every browser; we then set the files directly on the
+ * input via `setInputFiles`.
  *
  * `setInputFiles` doesn't await the input's async onChange (which writes to
  * IndexedDB via `addFiles`), so without a sync point a caller that follows
@@ -73,8 +106,15 @@ export async function switchToEditorIfViewerMode(page: Page): Promise<void> {
   const goToEditor = page.getByRole("button", {
     name: /go to file editor/i,
   });
+  // The affordance only exists while the workbench is transiently in viewer
+  // mode after an upload. The app can auto-leave viewer mode and detach the
+  // button between our visibility check and the click - the transition timing
+  // differs on firefox/webkit, where the detached button hangs a plain
+  // `click()` for the full actionability timeout. Treat a vanished button as
+  // "already in editor mode": swallow the click failure and let the caller's
+  // run-button assertion catch any genuine regression.
   if (await goToEditor.isVisible({ timeout: 1_000 }).catch(() => false)) {
-    await goToEditor.click();
+    await goToEditor.click({ timeout: 5_000 }).catch(() => {});
   }
 }
 
diff --git a/frontend/editor/src/core/tests/live/encrypted-unlock-then-tool.spec.ts b/frontend/editor/src/core/tests/live/encrypted-unlock-then-tool.spec.ts
index ef03c54369..de9d803d25 100644
--- a/frontend/editor/src/core/tests/live/encrypted-unlock-then-tool.spec.ts
+++ b/frontend/editor/src/core/tests/live/encrypted-unlock-then-tool.spec.ts
@@ -31,6 +31,9 @@ test.describe("Encrypted PDF: unlock then merge", () => {
     await page.goto("/merge");
     await page.waitForLoadState("domcontentloaded");
 
+    // `files-button`'s native picker is mocked globally
+    // (suppressNativeFilePicker), so the click is safe cross-browser; set the
+    // files on the hidden input directly.
     await page.getByTestId("files-button").click();
     await page
       .locator('[data-testid="file-input"]')
diff --git a/frontend/editor/src/core/tests/stubbed/certificate-validation.spec.ts b/frontend/editor/src/core/tests/stubbed/certificate-validation.spec.ts
index 1f8886bab4..075464c587 100644
--- a/frontend/editor/src/core/tests/stubbed/certificate-validation.spec.ts
+++ b/frontend/editor/src/core/tests/stubbed/certificate-validation.spec.ts
@@ -1,5 +1,6 @@
 import { test, expect, type Page } from "@playwright/test";
 import path from "path";
+import { suppressNativeFilePicker } from "@app/tests/helpers/ui-helpers";
 
 // ---------------------------------------------------------------------------
 // Test fixtures — pre-generated keystores in test-fixtures/certs/
@@ -95,6 +96,10 @@ async function uploadCertFile(page: Page, filePath: string) {
 // ---------------------------------------------------------------------------
 test.describe("Certificate Validation — ParticipantView", () => {
   test.beforeEach(async ({ page }) => {
+    // Raw @playwright/test fixture: install the picker suppression directly so
+    // clicking the Mantine <FileInput> button is intercepted cross-browser
+    // (firefox/webkit otherwise leak the native dialog and close the page).
+    suppressNativeFilePicker(page);
     await mockParticipantApis(page);
   });
 
diff --git a/frontend/editor/src/core/tests/stubbed/convert.spec.ts b/frontend/editor/src/core/tests/stubbed/convert.spec.ts
index cd6a60884a..de8fa17a9c 100644
--- a/frontend/editor/src/core/tests/stubbed/convert.spec.ts
+++ b/frontend/editor/src/core/tests/stubbed/convert.spec.ts
@@ -8,6 +8,7 @@
 import { test, expect, type Page } from "@playwright/test";
 import path from "path";
 import { mockAppApis } from "@app/tests/helpers/api-stubs";
+import { suppressNativeFilePicker } from "@app/tests/helpers/ui-helpers";
 
 const FIXTURES_DIR = path.join(__dirname, "../test-fixtures");
 const SAMPLE_PDF = path.join(FIXTURES_DIR, "sample.pdf");
@@ -23,10 +24,10 @@ async function dismissTourTooltip(page: Page) {
 }
 
 // ---------------------------------------------------------------------------
-// Helper: upload a file via the FileSidebar's "Open from computer" action.
-// The button now triggers the native OS picker directly - no modal - and
-// the hidden `data-testid="file-input"` accepts `setInputFiles` in either
-// sidebar state.
+// Helper: upload a file via the FileSidebar's "Open from computer" action. The
+// button's native OS picker is mocked globally by `suppressNativeFilePicker`
+// (test fixtures), so the click is safe cross-browser; the hidden
+// `data-testid="file-input"` then accepts `setInputFiles` in either state.
 // ---------------------------------------------------------------------------
 async function uploadFile(page: Page, filePath: string) {
   await page.getByTestId("files-button").click();
@@ -60,6 +61,11 @@ async function selectToFormat(page: Page, toValue: string) {
 // ---------------------------------------------------------------------------
 test.describe("Convert Tool", () => {
   test.beforeEach(async ({ page }) => {
+    // These specs use the raw @playwright/test fixture, so they don't get the
+    // shared stub-test-base suppression - install it here so the picker click
+    // is intercepted cross-browser (firefox/webkit otherwise leak the native
+    // dialog onto the host and close the page).
+    suppressNativeFilePicker(page);
     await mockAppApis(page);
     await page.goto("/?bypassOnboarding=true");
     await page.waitForSelector('[data-testid="files-button"]', {
diff --git a/frontend/editor/src/core/tests/stubbed/encrypted-pdf-unlock.spec.ts b/frontend/editor/src/core/tests/stubbed/encrypted-pdf-unlock.spec.ts
index 7e220a3bda..2a202b6cfe 100644
--- a/frontend/editor/src/core/tests/stubbed/encrypted-pdf-unlock.spec.ts
+++ b/frontend/editor/src/core/tests/stubbed/encrypted-pdf-unlock.spec.ts
@@ -21,6 +21,7 @@ import { test, expect, type Page } from "@playwright/test";
 import path from "path";
 import fs from "fs";
 import { mockAppApis } from "@app/tests/helpers/api-stubs";
+import { suppressNativeFilePicker } from "@app/tests/helpers/ui-helpers";
 
 const FIXTURES_DIR = path.join(__dirname, "../test-fixtures");
 const ENCRYPTED_PDF = path.join(FIXTURES_DIR, "encrypted.pdf");
@@ -63,8 +64,10 @@ function mockRemovePasswordWrongPassword(page: Page) {
 }
 
 async function uploadEncryptedFile(page: Page, filePath: string) {
+  // `files-button`'s native picker is mocked globally
+  // (suppressNativeFilePicker), so the click is safe cross-browser; set the
+  // files on the hidden input directly.
   await page.getByTestId("files-button").click();
-  // No modal flow - `files-button` triggers the native picker directly.
   await page.locator('[data-testid="file-input"]').setInputFiles(filePath);
 }
 
@@ -76,6 +79,10 @@ test.describe.configure({ mode: "serial" });
 
 test.describe("Encrypted PDF Unlock Modal", () => {
   test.beforeEach(async ({ page }) => {
+    // Raw @playwright/test fixture: install the picker suppression directly so
+    // the files-button click is intercepted cross-browser (firefox/webkit
+    // otherwise leak the native dialog onto the host and close the page).
+    suppressNativeFilePicker(page);
     await mockAppApis(page);
     await page.goto("/?bypassOnboarding=true");
     await page.waitForSelector('[data-testid="files-button"]', {
@@ -153,8 +160,9 @@ test.describe("Encrypted PDF Unlock Modal", () => {
   }) => {
     await mockRemovePasswordSuccess(page);
 
+    // `files-button`'s native picker is mocked globally; click it, then set
+    // the files on the hidden input directly.
     await page.getByTestId("files-button").click();
-    // No modal flow - `files-button` triggers the native picker directly.
     await page.locator('[data-testid="file-input"]').setInputFiles([
       {
         name: "encrypted-a.pdf",
diff --git a/frontend/editor/src/core/tests/stubbed/files-page-screenshots.spec.ts b/frontend/editor/src/core/tests/stubbed/files-page-screenshots.spec.ts
index fffa1b4b65..f4948945fe 100644
--- a/frontend/editor/src/core/tests/stubbed/files-page-screenshots.spec.ts
+++ b/frontend/editor/src/core/tests/stubbed/files-page-screenshots.spec.ts
@@ -445,8 +445,17 @@ test.describe("Files page screenshots", () => {
       localStorage.setItem("i18nextLng", "ar-AR");
       localStorage.setItem("stirling-language", "ar-AR");
       localStorage.setItem("stirling-language-source", "user");
-      document.documentElement.setAttribute("dir", "rtl");
-      document.documentElement.setAttribute("lang", "ar-AR");
+      // On webkit, `document.documentElement` is still null when Playwright
+      // runs init scripts, so calling setAttribute directly throws - and that
+      // uncaught error aborts the *following* init script (the IndexedDB seed
+      // in seedFiles), leaving the grid stuck on skeletons. Guard the access
+      // and defer to DOMContentLoaded when the element isn't there yet.
+      const applyDir = () => {
+        document.documentElement.setAttribute("dir", "rtl");
+        document.documentElement.setAttribute("lang", "ar-AR");
+      };
+      if (document.documentElement) applyDir();
+      else document.addEventListener("DOMContentLoaded", applyDir);
     });
   }
 
diff --git a/frontend/editor/src/core/tests/stubbed/pdf-text-search.spec.ts b/frontend/editor/src/core/tests/stubbed/pdf-text-search.spec.ts
index 5ba1599386..ce4bbce081 100644
--- a/frontend/editor/src/core/tests/stubbed/pdf-text-search.spec.ts
+++ b/frontend/editor/src/core/tests/stubbed/pdf-text-search.spec.ts
@@ -16,8 +16,9 @@ test.describe("Reader - in-document text search", () => {
     await page.goto("/read");
     await page.waitForLoadState("domcontentloaded");
 
-    // Upload a PDF first so the reader has content. `files-button` now
-    // triggers the native picker directly - no modal flow involved.
+    // Upload a PDF first so the reader has content. The `files-button` native
+    // picker is mocked globally (suppressNativeFilePicker), so the click is
+    // safe cross-browser; set the files on the hidden input directly.
     await page.getByTestId("files-button").click();
     await page.locator('[data-testid="file-input"]').setInputFiles(SAMPLE_PDF);
 
diff --git a/frontend/editor/src/core/tests/stubbed/viewer-text-selection.spec.ts b/frontend/editor/src/core/tests/stubbed/viewer-text-selection.spec.ts
index 573789aca3..1a7486e37a 100644
--- a/frontend/editor/src/core/tests/stubbed/viewer-text-selection.spec.ts
+++ b/frontend/editor/src/core/tests/stubbed/viewer-text-selection.spec.ts
@@ -96,7 +96,15 @@ test("hovering over text changes the cursor to an I-beam", async ({ page }) => {
 test("Ctrl+C copies selected text to the clipboard", async ({
   page,
   context,
+  browserName,
 }) => {
+  // Reading the clipboard requires the `clipboard-read` permission, which only
+  // chromium supports via `grantPermissions` (firefox throws "Unknown
+  // permission"; webkit can't expose `navigator.clipboard.readText` in tests).
+  test.skip(
+    browserName !== "chromium",
+    "clipboard read/permissions are chromium-only in Playwright",
+  );
   test.setTimeout(60_000);
   await context.grantPermissions(["clipboard-read", "clipboard-write"]);
   const firstPage = await loadSampleAndOpenViewer(page);
@@ -169,7 +177,15 @@ test("right-click on the page does not surface the browser context menu", async
 test("floating Copy menu appears after drag-select and copies", async ({
   page,
   context,
+  browserName,
 }) => {
+  // Verifying the copy result reads the clipboard, which needs the
+  // `clipboard-read` permission - chromium-only in Playwright. The Copy menu's
+  // appearance is covered cross-browser by the right-click test above.
+  test.skip(
+    browserName !== "chromium",
+    "clipboard read/permissions are chromium-only in Playwright",
+  );
   test.setTimeout(60_000);
   await context.grantPermissions(["clipboard-read", "clipboard-write"]);
   const firstPage = await loadSampleAndOpenViewer(page);