Merge fix/sheet-aria-modal-gating: sheet a11y gating + test-suite hygiene

Gate always-mounted sheet dialog semantics on open state; fix focus-trap Escape drop and deep-link back navigation; update the e2e suite for shipped magic-byte validation, SSE ticket flow and PIN auto-submit; gate Chromium-only permissions.
2026-06-30 22:12:11 +02:00
parent 1c7495e568 a3c0082c4b
commit 6ccf2d0011
12 changed files with 87 additions and 27 deletions
--- a/e2e/page-objects/recover-page.ts
+++ b/e2e/page-objects/recover-page.ts
@@ -21,7 +21,15 @@ export class RecoverPage {

  async recover(name: string, pin: string) {
    await this.nameInput.fill(name);
+    // Filling the 4th digit fires the form's auto-submit ($effect on
+    // pin.length === 4, see pin-auto-submit.spec). An explicit submit click would
+    // race the ensuing navigation and detach mid-click, so only click as a
+    // fallback if the button is still around (e.g. a partial / failed PIN).
    await this.pinInput.fill(pin);
-    await this.submitButton.click();
+    if (await this.submitButton.isEnabled().catch(() => false)) {
+      await this.submitButton.click().catch(() => {
+        /* auto-submit already navigated — nothing to click */
+      });
+    }
  }
 }
--- a/e2e/playwright.config.ts
+++ b/e2e/playwright.config.ts
@@ -12,6 +12,12 @@ import { defineConfig, devices } from '@playwright/test';
 * engine-level divergences. The rest of the suite only runs against
 * `chromium-desktop` to keep the wall-clock reasonable.
 */
+// camera/microphone/clipboard are Chromium-only permissions; passing them to
+// firefox/webkit projects throws "Unknown permission: camera" and fails the
+// whole test before it runs. Granted per-Chromium-project below instead of in
+// the global `use` block.
+const CHROMIUM_PERMISSIONS = ['camera', 'microphone', 'clipboard-read', 'clipboard-write'];
+
 export default defineConfig({
  testDir: './specs',
  outputDir: './test-results',
@@ -35,9 +41,8 @@ export default defineConfig({
    video: 'retain-on-failure',
    actionTimeout: 10_000,
    navigationTimeout: 30_000,
-    // Camera/mic permissions granted by default; the fake-media launch args
-    // (set per-project below for Chromium) supply the actual stream.
-    permissions: ['camera', 'microphone', 'clipboard-read', 'clipboard-write'],
+    // No camera/mic/clipboard here — those are Chromium-only and are granted on
+    // the Chromium projects below (see CHROMIUM_PERMISSIONS).
  },

  projects: [
@@ -49,6 +54,7 @@ export default defineConfig({
      testIgnore: ['**/09-mobile/**'],
      use: {
        ...devices['Desktop Chrome'],
+        permissions: CHROMIUM_PERMISSIONS,
        launchOptions: {
          args: [
            '--use-fake-ui-for-media-stream',
@@ -68,13 +74,13 @@ export default defineConfig({
    {
      name: 'chromium-mobile',
      testMatch: ['**/09-mobile/**/*.spec.ts'],
-      use: { ...devices['Pixel 7'] },
+      use: { ...devices['Pixel 7'], permissions: CHROMIUM_PERMISSIONS },
    },

    // ── Mobile UA smoke matrix (runs only @smoke specs in CI) ────────────
    {
      name: 'chromium-pixel7',
-      use: { ...devices['Pixel 7'] },
+      use: { ...devices['Pixel 7'], permissions: CHROMIUM_PERMISSIONS },
      grep: /@smoke/,
    },
    {
@@ -84,6 +90,7 @@ export default defineConfig({
        viewport: { width: 360, height: 780 },
        userAgent:
          'Mozilla/5.0 (Linux; Android 14; SM-S911B) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Mobile Safari/537.36',
+        permissions: CHROMIUM_PERMISSIONS,
      },
      grep: /@smoke/,
    },
@@ -94,6 +101,7 @@ export default defineConfig({
        viewport: { width: 360, height: 780 },
        userAgent:
          'Mozilla/5.0 (Linux; Android 14; SM-S911B) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/24.0 Chrome/124.0.0.0 Mobile Safari/537.36',
+        permissions: CHROMIUM_PERMISSIONS,
      },
      grep: /@smoke/,
    },
@@ -103,6 +111,7 @@ export default defineConfig({
        ...devices['Pixel 7'],
        userAgent:
          'Mozilla/5.0 (Linux; Android 14; Pixel 7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Mobile Safari/537.36 EdgA/124.0.0.0',
+        permissions: CHROMIUM_PERMISSIONS,
      },
      grep: /@smoke/,
    },
@@ -125,6 +134,9 @@ export default defineConfig({
      use: {
        ...devices['Pixel 7'],
        defaultBrowserType: 'firefox',
+        // Firefox rejects `isMobile` (Chromium-only). Keep the phone viewport +
+        // Android UA for coverage, but drop the unsupported flag.
+        isMobile: false,
        userAgent:
          'Mozilla/5.0 (Android 14; Mobile; rv:124.0) Gecko/124.0 Firefox/124.0',
      },
--- a/e2e/specs/01-auth/join.spec.ts
+++ b/e2e/specs/01-auth/join.spec.ts
@@ -54,9 +54,13 @@ test.describe('Auth — join flow', () => {
    await expect(join.recoveryPinInput).toBeVisible();
    await expect(page.getByText(/Charlie.*bereits vergeben/)).toBeVisible();

-    // Type correct PIN → land on /feed with a new JWT
+    // Type correct PIN → land on /feed with a new JWT. Filling the 4th digit
+    // auto-submits (see pin-auto-submit.spec), so an explicit submit click would
+    // race the navigation; click only as a fallback if the button is still around.
    await join.recoveryPinInput.fill(original.pin);
-    await join.recoverySubmit.click();
+    if (await join.recoverySubmit.isEnabled().catch(() => false)) {
+      await join.recoverySubmit.click().catch(() => {});
+    }
    await page.waitForURL('**/feed');

    const storage = await readStorage(page);
--- a/e2e/specs/02-upload/rate-limit.spec.ts
+++ b/e2e/specs/02-upload/rate-limit.spec.ts
@@ -6,8 +6,12 @@
 */
 import { test, expect } from '../../fixtures/test';
 import { join } from 'node:path';
+import { readFileSync } from 'node:fs';

 const SAMPLE_JPG = join(process.cwd(), 'fixtures', 'media', 'sample.jpg');
+// A real, decodable JPEG — the upload handler validates magic bytes, so a
+// zero-filled buffer would be rejected with 400 before the rate limiter is reached.
+const SAMPLE_BYTES = readFileSync(SAMPLE_JPG);

 test.describe('Upload — rate limit', () => {
  test('4th upload in one hour returns 429 with Retry-After', async ({ api, adminToken, guest }) => {
@@ -24,7 +28,7 @@ test.describe('Upload — rate limit', () => {
    // Hit the API directly for speed — UI behavior is asserted in gallery-path.spec.
    const upload = async (n: number) => {
      const form = new FormData();
-      const blob = new Blob([new Uint8Array(640)], { type: 'image/jpeg' });
+      const blob = new Blob([SAMPLE_BYTES], { type: 'image/jpeg' });
      form.append('file', blob, `file${n}.jpg`);
      form.append('content_type', 'image/jpeg');
      return fetch((process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101') + '/api/v1/upload', {
@@ -47,7 +51,6 @@ test.describe('Upload — rate limit', () => {
    // The 429 response carries Retry-After.
    const limited = responses.find((r) => r.status === 429)!;
    expect(limited.headers.get('retry-after')).toBeTruthy();
-    void SAMPLE_JPG;
  });

  test('flipping upload_rate_enabled off bypasses the limit', async ({ api, adminToken, guest }) => {
@@ -56,7 +59,7 @@ test.describe('Upload — rate limit', () => {
    const h = await guest('NoQuota');
    const upload = async (n: number) => {
      const form = new FormData();
-      const blob = new Blob([new Uint8Array(640)], { type: 'image/jpeg' });
+      const blob = new Blob([SAMPLE_BYTES], { type: 'image/jpeg' });
      form.append('file', blob, `file${n}.jpg`);
      form.append('content_type', 'image/jpeg');
      return fetch((process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101') + '/api/v1/upload', {
--- a/e2e/specs/07-adversarial/ddos.spec.ts
+++ b/e2e/specs/07-adversarial/ddos.spec.ts
@@ -46,9 +46,21 @@ test.describe('Adversarial — small-scale abuse', () => {

  test('SSE: 10 concurrent streams from one user do not crash the server', async ({ guest }) => {
    const g = await guest('SseFlood');
-    const controllers = Array.from({ length: 10 }, () => new AbortController());
-    const requests = controllers.map((c) =>
-      fetch(`${BASE}/api/v1/stream?token=${encodeURIComponent(g.jwt)}`, { signal: c.signal })
+    // The stream endpoint authenticates via single-use tickets (POST /stream/ticket),
+    // not the raw JWT — a `?token=` open is rejected with 400. Mint one ticket per stream.
+    const mintTicket = async () => {
+      const res = await fetch(`${BASE}/api/v1/stream/ticket`, {
+        method: 'POST',
+        headers: { Authorization: `Bearer ${g.jwt}` },
+      });
+      const json: any = await res.json();
+      return json.ticket as string;
+    };
+    const tickets = await Promise.all(Array.from({ length: 10 }, mintTicket));
+
+    const controllers = tickets.map(() => new AbortController());
+    const requests = tickets.map((ticket, i) =>
+      fetch(`${BASE}/api/v1/stream?ticket=${encodeURIComponent(ticket)}`, { signal: controllers[i].signal })
    );
    const responses = await Promise.all(requests);
    // All accepted (or some rate-limited — both fine).
--- a/e2e/specs/07-adversarial/file-upload-attacks.spec.ts
+++ b/e2e/specs/07-adversarial/file-upload-attacks.spec.ts
--- a/e2e/specs/09-mobile/gestures-doubletap.spec.ts
+++ b/e2e/specs/09-mobile/gestures-doubletap.spec.ts
@@ -75,10 +75,11 @@ test.describe('Mobile — double-tap gesture', () => {
    await expect(imageButton).toBeVisible({ timeout: 10_000 });
    await imageButton.click();

-    // LightboxModal is `role="dialog"` (no aria-modal). The other dialog on the
-    // page is the ContextSheet which has `aria-modal="true"` even when closed,
-    // so scope to NOT-aria-modal to pick the lightbox specifically.
-    const lightbox = page.locator('[role="dialog"]:not([aria-modal])');
+    // LightboxModal is the only dialog labelled by #lightbox-title, so target it
+    // directly. (Closed sheets no longer expose role=dialog — that semantics is
+    // gated on `open` — so a plain [role=dialog] match would be ambiguous only
+    // while a sheet is open; the labelledby scope keeps this unambiguous.)
+    const lightbox = page.locator('[role="dialog"][aria-labelledby="lightbox-title"]');
    await expect(lightbox).toBeVisible();

    // Find the inner image element to tap.
--- a/e2e/specs/09-mobile/sheet-escape.spec.ts
+++ b/e2e/specs/09-mobile/sheet-escape.spec.ts
@@ -12,7 +12,9 @@ test.describe('Mobile a11y — sheets dismiss on Escape', () => {
    await page.goto('/account');

    // Click the "Original" radio in the Datennutzung section to open the warning sheet.
-    const originalRadio = page.getByRole('radio', { name: /Original$/i });
+    // The radio's accessible name is its title + description ("Original Lädt die
+    // Originaldateien…"), so anchor on the start, not the end.
+    const originalRadio = page.getByRole('radio', { name: /^Original/i });
    await originalRadio.click();

    const sheet = page.locator('[role="dialog"][aria-labelledby="data-mode-title"]');
--- a/frontend/src/lib/actions/focus-trap.ts
+++ b/frontend/src/lib/actions/focus-trap.ts
@@ -64,11 +64,16 @@ export function focusTrap(
 	node.addEventListener('keydown', onKeyDown);

 	if (opts.autoFocus !== false) {
-		// Defer one frame so the element is fully laid out (sheets animate in).
+		// Focus the container synchronously on mount so the trap owns the keyboard
+		// immediately — otherwise an Escape pressed before the deferred focus below
+		// lands on an element *outside* the node, where this node-scoped listener
+		// never sees it (the keystroke is silently dropped). Then defer one frame to
+		// move focus onto the first control once the sheet has laid out / animated in.
+		if (!node.hasAttribute('tabindex')) node.setAttribute('tabindex', '-1');
+		node.focus({ preventScroll: true });
 		requestAnimationFrame(() => {
 			const list = focusables(node);
 			const target = list[0] ?? node;
-			if (!node.hasAttribute('tabindex')) node.setAttribute('tabindex', '-1');
 			target.focus({ preventScroll: true });
 		});
 	}
--- a/frontend/src/lib/components/ContextSheet.svelte
+++ b/frontend/src/lib/components/ContextSheet.svelte
@@ -103,8 +103,10 @@
 	class:translate-y-full={!open}
 	class:translate-y-0={open}
 	style="padding-bottom: env(safe-area-inset-bottom)"
-	role="dialog"
-	aria-modal="true"
+	role={open ? 'dialog' : undefined}
+	aria-modal={open ? 'true' : undefined}
+	aria-hidden={!open}
+	inert={!open}
 	tabindex="-1"
 >
 	<div class="flex justify-center pt-3 pb-1">
--- a/frontend/src/lib/components/UploadSheet.svelte
+++ b/frontend/src/lib/components/UploadSheet.svelte
@@ -136,9 +136,11 @@
 	class:translate-y-full={!open}
 	class:translate-y-0={open}
 	style="padding-bottom: env(safe-area-inset-bottom)"
-	role="dialog"
-	aria-modal="true"
+	role={open ? 'dialog' : undefined}
+	aria-modal={open ? 'true' : undefined}
 	aria-label="Hochladen"
+	aria-hidden={!open}
+	inert={!open}
 	tabindex="-1"
 >
 	<!-- Drag handle -->
--- a/frontend/src/routes/recover/+page.svelte
+++ b/frontend/src/routes/recover/+page.svelte
@@ -1,14 +1,23 @@
 <script lang="ts">
-	import { goto } from '$app/navigation';
+	import { goto, afterNavigate } from '$app/navigation';
 	import { api, ApiError } from '$lib/api';
 	import { setAuth, getPin, getToken } from '$lib/auth';
 	import { browser } from '$app/environment';
 	import IconButton from '$lib/components/IconButton.svelte';

+	// `from` is non-null only when we arrived here via in-app (client-side)
+	// navigation; on a full-page load (deep link, new tab) it's null. Using
+	// history.length is unreliable — a fresh tab keeps `about:blank` as the prior
+	// entry, so history.back() would land there instead of inside the app.
+	let cameFromApp = $state(false);
+	afterNavigate(({ from }) => {
+		cameFromApp = from !== null;
+	});
+
 	function goBack() {
 		// Prefer the actual previous page (most users land here from /join or /account).
 		// Fall back to a sensible default based on auth state for deep-linked users.
-		if (browser && window.history.length > 1) {
+		if (cameFromApp) {
 			window.history.back();
 			return;
 		}