Compare commits

...

8 Commits

Author SHA1 Message Date
Marcel
49443ad16a docs(PersonMentionEditor): document client-side fetch exception inline
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 3m20s
CI / OCR Service Tests (pull_request) Successful in 35s
CI / Backend Unit Tests (pull_request) Failing after 3m4s
CI / Unit & Component Tests (push) Failing after 3m44s
CI / OCR Service Tests (push) Successful in 40s
CI / Backend Unit Tests (push) Failing after 3m14s
Per Markus #5616, the leaf-component fetch in the Tiptap suggestion plugin
violates the project-wide rule from frontend/CLAUDE.md ("Data flows from
+page.server.ts via props — never client-side API fetch"). Add an inline
block-comment explaining why this exception is justified (suggestion runs
client-side per keystroke; same auth surface; no server-side reshape
benefit) and points future readers at the open ADR follow-up plus Nora's
PersonSummaryDTO response-shape audit.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:21:33 +02:00
Marcel
e6844c403c feat(MentionDropdown): restore "Neue Person anlegen" empty-state link
The Tiptap rewrite dropped the inline "create new person" affordance the
textarea-era component used to render. Without it the workflow regresses:
transcriber must close the dropdown, navigate to /persons/new, come back,
re-type the query. The m.person_mention_create_new() key is still in all
three locale files — add the link back as a 44px-tall row with a top
border separating it from the empty-state message.

target=_blank keeps document/editor state intact; rel=noopener prevents
reverse-tabnabbing. mousedown preventDefault keeps the editor focused
(the dropdown row pattern used for option rows).

Test: empty-state renders a link to /persons/new with the localised label.

Leonie #5621 (Major) + Elicit OQ-373-04.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:20:42 +02:00
Marcel
f1932fd5f6 fix(person-mention): WCAG 1.4.11 contrast for mention pill and dropdown ring
Two non-text-contrast failures, both flagged by Leonie #5621:

1. PersonMentionEditor mention pill: decoration-brand-mint (#A6DAD8) on
   white is ≈1.7:1 — fails the 3:1 minimum for meaningful UI indicators.
   Switch to decoration-ink/50, which matches the read-mode .person-mention
   rule (≈6.4:1) and keeps a unified underline language across modes.

2. MentionDropdown highlighted-row ring: ring-brand-mint on bg-brand-mint/20
   is ≈2.5:1 — same failure class. Switch to ring-brand-navy (≈14.5:1
   against the highlight background) so keyboard-driven selection has a
   clearly visible indicator.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:19:34 +02:00
Marcel
ba88febc77 fix(PersonMentionEditor): guard setEditable effect against re-entry loop
The disabled-state effect calls editor.setEditable, which triggers a
ProseMirror transaction → onUpdate → bind:value/mentionedPersons writes →
host re-render → child prop pass-through → effect re-fires. Without an
idempotence check, this exceeds Svelte's effect_update_depth and crashes
every consuming spec (TranscriptionBlock 22/22). Compare editor.isEditable
against the desired value first; only call setEditable when it actually
needs to change.

Follow-up to 6ef888a1.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:18:40 +02:00
Marcel
fa7b97acdc test(PersonMentionEditor): assert no HTML injection via mention displayName
Adds a CWE-79 regression test: a sidecar entry whose displayName contains
an <img onerror=alert(1)> payload must round-trip through deserialize and
the Tiptap renderHTML without producing a real <img> element in the editor
DOM. Locks down the "renderHTML's third tuple entry is a text node, never
parsed as HTML" invariant so a future "use innerHTML for performance"
refactor cannot silently regress.

Nora #5618 detection-gap concern.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:14:19 +02:00
Marcel
6ef888a128 fix(PersonMentionEditor): enforce disabled state on the contenteditable
Wrapping the editor with pointer-events-none was visual-only — keyboard users
could still tab into the contenteditable and type. Wire `editable: !disabled`
on the Tiptap Editor and a reactive `$effect` that calls setEditable when the
prop flips after mount; expose `aria-disabled="true"` on the wrapper so
screen readers announce the deactivated state.

Tests assert contenteditable=false and aria-disabled=true when disabled;
contenteditable=true otherwise.

Closes WCAG 2.1.1 / 4.1.2 — Felix #5615 + Leonie #5621 + Nora #5618 BLOCKER.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:13:32 +02:00
Marcel
94d0733412 chore(i18n): remove orphaned error_person_rename_conflict translation key
errors.ts no longer references this code (the rename-propagation listener
was deleted) and the matching ErrorCode value is gone from the backend.
The Paraglide-compiled message helpers should not include strings nothing
calls — drop the entries from de/en/es to keep the i18n surface honest.

Felix #5615 + Elicit #5624 blocker.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:10:36 +02:00
Marcel
4ac94b2feb refactor(frontend): delete orphaned personMention.ts after Tiptap migration
The textarea-era detectPersonMention helper has no production callers since
the suggestion plugin's char: '@' mechanism replaced it. Per "Dead code is
deleted, not commented out", remove the source file and its spec — the spec
was running but tested a function nobody calls.

Felix #5615 blocker.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-29 16:09:53 +02:00
9 changed files with 151 additions and 95 deletions

View File

@@ -551,7 +551,6 @@
"person_alias_btn_delete": "Entfernen",
"error_alias_not_found": "Der Namensalias wurde nicht gefunden.",
"error_invalid_person_type": "Der angegebene Personentyp ist ungültig.",
"error_person_rename_conflict": "Eine andere Bearbeitung hat einen verknüpften Transkriptionsblock gleichzeitig geändert. Bitte erneut versuchen.",
"validation_last_name_required": "Nachname ist Pflichtfeld.",
"validation_first_name_required": "Vorname ist Pflichtfeld.",
"error_ocr_service_unavailable": "Der OCR-Dienst ist nicht verfügbar.",

View File

@@ -551,7 +551,6 @@
"person_alias_btn_delete": "Remove",
"error_alias_not_found": "The name alias was not found.",
"error_invalid_person_type": "The specified person type is not valid.",
"error_person_rename_conflict": "Another edit modified a referenced transcription block at the same time. Please try again.",
"validation_last_name_required": "Last name is required.",
"validation_first_name_required": "First name is required.",
"error_ocr_service_unavailable": "The OCR service is not available.",

View File

@@ -551,7 +551,6 @@
"person_alias_btn_delete": "Eliminar",
"error_alias_not_found": "No se encontro el alias de nombre.",
"error_invalid_person_type": "El tipo de persona especificado no es válido.",
"error_person_rename_conflict": "Otra edición modificó un bloque de transcripción referenciado al mismo tiempo. Por favor, inténtalo de nuevo.",
"validation_last_name_required": "El apellido es obligatorio.",
"validation_first_name_required": "El nombre es obligatorio.",
"error_ocr_service_unavailable": "El servicio OCR no está disponible.",

View File

@@ -122,12 +122,31 @@ function selectItem(item: Person) {
<p class="px-3 py-2.5 font-sans text-sm text-ink-3">
{m.person_mention_popup_empty()}
</p>
<!--
Empty-state escape hatch — without it the transcriber has to close
the dropdown, navigate to /persons/new, come back, and re-type the
query. target=_blank keeps the document and editor state intact;
rel=noopener prevents reverse-tabnabbing on the new tab. Leonie #5621.
-->
<a
href="/persons/new"
target="_blank"
rel="noopener"
class="flex min-h-[44px] items-center gap-2 border-t border-line px-3 py-2.5 font-sans text-sm font-medium text-brand-navy hover:bg-canvas focus:bg-canvas focus:outline-none"
onmousedown={(e) => e.preventDefault()}
>
{m.person_mention_create_new()}
<span aria-hidden="true"></span>
</a>
{:else}
{#each model.items as person, i (person.id)}
<div
class={[
'flex min-h-[44px] cursor-pointer flex-col gap-1 px-3 py-2.5 text-left hover:bg-canvas',
i === highlightedIndex && 'bg-brand-mint/20 ring-2 ring-brand-mint ring-inset'
// brand-mint ring (≈2.5:1 on white) fails WCAG 1.4.11 Non-Text
// Contrast for a meaningful keyboard-highlight indicator. brand-navy
// gives ~14.5:1 against the bg-brand-mint/20 row. Leonie #5621.
i === highlightedIndex && 'bg-brand-mint/20 ring-2 ring-brand-navy ring-inset'
]}
role="option"
aria-selected={i === highlightedIndex}

View File

@@ -82,6 +82,11 @@ onMount(() => {
editor = new Editor({
element: editorEl,
// Initial editable state honors the `disabled` prop. The reactive
// $effect below keeps it in sync if the prop flips after mount —
// without this, a keyboard user can tab into the contenteditable
// even when the wrapper has pointer-events-none (WCAG 2.1.1).
editable: !disabled,
extensions: [
StarterKit.configure({
heading: false,
@@ -98,6 +103,9 @@ onMount(() => {
}),
CustomMention.configure({
renderHTML({ node }) {
// Underline color matches the read-mode .person-mention rule
// (ink at ~50% alpha) — brand-mint on white fails WCAG 1.4.11
// Non-Text Contrast (≈1.7:1, needs 3:1). Leonie #5621.
return [
'span',
{
@@ -105,7 +113,7 @@ onMount(() => {
'data-person-id': node.attrs.personId,
'data-display-name': node.attrs.displayName,
class:
'mention-token underline decoration-brand-mint underline-offset-2 text-brand-navy font-medium'
'mention-token underline decoration-ink/50 underline-offset-2 text-brand-navy font-medium'
},
`@${node.attrs.displayName}`
];
@@ -115,6 +123,20 @@ onMount(() => {
},
suggestion: {
char: '@',
// ─────────────────────────────────────────────────────────────
// EXCEPTION to frontend/CLAUDE.md "no client-side API fetch":
// Tiptap's suggestion plugin lives entirely on the client and
// fires on every keystroke after `@`. Routing each query through
// a SvelteKit form action would round-trip through SSR for a
// dropdown that needs to feel instantaneous, and a +server.ts
// endpoint would only proxy the same call. Auth flows through
// the Vite proxy in dev and Caddy in prod (cookie-based), so the
// network surface is identical to a server-driven call.
// Markus #5616: an ADR will formalise this. Open follow-up:
// "ADR: client-side fetch exception for editor suggestion plugins."
// Nora #5618 #3 — separate issue tracks the GET /api/persons
// response-shape audit (PersonSummaryDTO leaks `notes`).
// ─────────────────────────────────────────────────────────────
items: async ({ query }: { query: string }) => {
if (!query) return [];
try {
@@ -232,11 +254,26 @@ onMount(() => {
onDestroy(() => {
editor?.destroy();
});
// Keep editor in sync with the reactive `disabled` prop. Tiptap's setEditable
// flips contenteditable on the inner DOM and stops accepting input — matches
// the textarea's old `disabled` semantics for keyboard users (WCAG 2.1.1).
//
// Guard: setEditable triggers a ProseMirror transaction which fires onUpdate;
// onUpdate writes through bind:value / bind:mentionedPersons. Without this
// idempotence check, the effect would loop on every prop pass-through.
$effect(() => {
const shouldBeEditable = !disabled;
if (editor && editor.isEditable !== shouldBeEditable) {
editor.setEditable(shouldBeEditable);
}
});
</script>
<div
class="relative rounded-sm border border-transparent focus-within:border-brand-mint focus-within:ring-2 focus-within:ring-brand-mint/40"
class:opacity-50={disabled}
class:pointer-events-none={disabled}
aria-disabled={disabled ? 'true' : undefined}
bind:this={editorEl}
></div>

View File

@@ -47,7 +47,9 @@ function mockFetchEmpty() {
type Snapshot = { value: string; mentionedPersons: PersonMention[] };
function renderHost(initial: { value?: string; mentionedPersons?: PersonMention[] } = {}) {
function renderHost(
initial: { value?: string; mentionedPersons?: PersonMention[]; disabled?: boolean } = {}
) {
let snapshot: Snapshot = {
value: initial.value ?? '',
mentionedPersons: initial.mentionedPersons ?? []
@@ -55,6 +57,7 @@ function renderHost(initial: { value?: string; mentionedPersons?: PersonMention[
render(PersonMentionEditorHost, {
initialValue: initial.value ?? '',
initialMentions: initial.mentionedPersons ?? [],
disabled: initial.disabled ?? false,
onChange: (snap: Snapshot) => {
snapshot = snap;
}
@@ -142,6 +145,19 @@ describe('PersonMentionEditor — typeahead', () => {
await expect.element(page.getByText('Keine Personen gefunden')).toBeInTheDocument();
});
});
it('offers a "create new person" link in the empty state', async () => {
mockFetchEmpty();
renderHost();
await userEvent.type(page.getByRole('textbox'), '@xyz');
await vi.waitFor(async () => {
const link = page.getByRole('link', { name: /Neue Person anlegen/ });
await expect.element(link).toBeVisible();
await expect.element(link).toHaveAttribute('href', '/persons/new');
});
});
});
// ─── AC-1: typed text becomes displayName, not DB name ───────────────────────
@@ -280,6 +296,73 @@ describe('PersonMentionEditor — keyboard navigation', () => {
});
});
// ─── Disabled state (WCAG 2.1.1 — keyboard users) ────────────────────────────
describe('PersonMentionEditor — disabled state', () => {
it('sets contenteditable=false on the editor when disabled', async () => {
renderHost({ value: 'Bestehender Text', disabled: true });
await vi.waitFor(() => {
const textbox = document.querySelector('[role="textbox"]') as HTMLElement | null;
expect(textbox).not.toBeNull();
expect(textbox!.getAttribute('contenteditable')).toBe('false');
});
});
it('exposes aria-disabled=true on the editor wrapper when disabled', async () => {
renderHost({ disabled: true });
await vi.waitFor(() => {
const wrapper = document.querySelector('[aria-disabled="true"]');
expect(wrapper).not.toBeNull();
});
});
it('keeps the editor editable (contenteditable=true) when not disabled', async () => {
renderHost({ disabled: false });
await vi.waitFor(() => {
const textbox = document.querySelector('[role="textbox"]') as HTMLElement | null;
expect(textbox).not.toBeNull();
expect(textbox!.getAttribute('contenteditable')).toBe('true');
});
});
});
// ─── Security — XSS in displayName (CWE-79) ──────────────────────────────────
describe('PersonMentionEditor — XSS resistance', () => {
it('renders a malicious displayName as text, not as HTML elements', async () => {
// A historical sidecar entry whose displayName contains an HTML payload
// that would execute if interpolated as raw HTML. Tiptap's renderHTML
// returns the @-prefixed string as the third tuple entry, which
// ProseMirror's DOMSerializer treats as a Text node — escaping it.
const maliciousMention: PersonMention = {
personId: '00000000-0000-0000-0000-000000000001',
displayName: '<img src=x onerror=alert(1)>'
};
renderHost({
value: '@<img src=x onerror=alert(1)>',
mentionedPersons: [maliciousMention]
});
await vi.waitFor(() => {
const textbox = document.querySelector('[role="textbox"]') as HTMLElement | null;
expect(textbox).not.toBeNull();
// No element from the malicious payload should have appeared as a real
// DOM node. (Tiptap inserts its own ProseMirror-separator <img> in empty
// paragraphs — that is internal markup and never carries user attrs;
// guard against the injection by checking the user-controlled attrs.)
expect(textbox!.querySelector('img[onerror]')).toBeNull();
expect(textbox!.querySelector('img[src="x"]')).toBeNull();
expect(textbox!.querySelector('script')).toBeNull();
// The payload should appear as visible text content instead.
expect(textbox!.textContent ?? '').toContain('<img src=x onerror=alert(1)>');
});
});
});
// ─── Touch target (WCAG 2.2 AA) ──────────────────────────────────────────────
describe('PersonMentionEditor — touch target', () => {

View File

@@ -9,10 +9,17 @@ type Props = {
initialValue?: string;
initialMentions?: PersonMention[];
placeholder?: string;
disabled?: boolean;
onChange: (snapshot: { value: string; mentionedPersons: PersonMention[] }) => void;
};
let { initialValue = '', initialMentions = [], placeholder, onChange }: Props = $props();
let {
initialValue = '',
initialMentions = [],
placeholder,
disabled = false,
onChange
}: Props = $props();
// initial* props seed mount-time state; reading them inside untrack signals
// the intentional one-shot capture and silences state_referenced_locally.
@@ -28,4 +35,5 @@ $effect(() => {
bind:value={value}
bind:mentionedPersons={mentionedPersons}
placeholder={placeholder}
disabled={disabled}
/>

View File

@@ -1,65 +0,0 @@
import { describe, it, expect } from 'vitest';
import { detectPersonMention } from './personMention';
describe('detectPersonMention', () => {
it('returns null when text has no @', () => {
expect(detectPersonMention('hello world', 11)).toBeNull();
});
it('returns null when @ is preceded by a non-whitespace character (email pattern)', () => {
expect(detectPersonMention('user@example', 12)).toBeNull();
});
it('returns query for @ at the very start of string', () => {
expect(detectPersonMention('@Aug', 4)).toBe('Aug');
});
it('returns empty string immediately after @', () => {
expect(detectPersonMention('@', 1)).toBe('');
});
it('returns single-word query', () => {
expect(detectPersonMention('hi @Auguste', 11)).toBe('Auguste');
});
it('keeps the trigger active when the query has a trailing space', () => {
expect(detectPersonMention('hi @Auguste ', 12)).toBe('Auguste ');
});
it('returns multi-word query (spaces allowed)', () => {
expect(detectPersonMention('hi @Auguste Raddatz', 19)).toBe('Auguste Raddatz');
});
it('returns single-character query', () => {
expect(detectPersonMention('@M', 2)).toBe('M');
});
it('returns null when the query crosses a newline', () => {
expect(detectPersonMention('@Aug\nfoo', 8)).toBeNull();
});
it('returns null when a second @ appears in the query (next mention starts)', () => {
expect(detectPersonMention('@Aug@bar', 8)).toBeNull();
});
it('uses the most recent @ when separated by whitespace', () => {
// '@Aug @Bert' with cursor at end — the second @ is the trigger.
expect(detectPersonMention('@Aug @Bert', 10)).toBe('Bert');
});
it('returns the query when the cursor sits exactly at a newline boundary', () => {
// '@Aug\nfoo' with cursor at index 4 — right at the newline before it
// is consumed. The query is still 'Aug' because nothing past the cursor
// counts.
expect(detectPersonMention('@Aug\nfoo', 4)).toBe('Aug');
});
it('returns null when cursor is before the @', () => {
expect(detectPersonMention('@Hans', 0)).toBeNull();
});
it('uses the most recent @ in the text', () => {
// cursor is just after the second @ + a few chars
expect(detectPersonMention('hi @Anna and @Bert', 18)).toBe('Bert');
});
});

View File

@@ -1,23 +0,0 @@
/**
* Given the current textarea value and cursor position, returns the
* @-person-mention query being typed (the text after the last triggering @),
* or null if no person-mention is active.
*
* Rules — distinct from comment-mentions in `mention.ts`:
* - @ must be at the start of the string or preceded by whitespace
* - The query may contain spaces (historical persons commonly have multi-word
* display names — "Auguste Raddatz", "Maria von Müller-Schultz")
* - The query stops at a newline or at a second @ (the next mention starts)
*/
export function detectPersonMention(text: string, cursorPos: number): string | null {
const before = text.slice(0, cursorPos);
const atIndex = before.lastIndexOf('@');
if (atIndex === -1) return null;
if (atIndex > 0 && !/\s/.test(before[atIndex - 1])) return null;
const query = before.slice(atIndex + 1);
if (query.includes('\n') || query.includes('@')) return null;
return query;
}