From a909593ddaac755073e0c1cc172e86a4ced0e274 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 21 Jul 2026 23:26:51 +0200 Subject: [PATCH] fix: detect typing inside the QuotedHtml shadow island via composedPath #654 --- components/email/email-composer.tsx | 8 +-- components/layout/sidebar.tsx | 13 ++--- hooks/use-keyboard-shortcuts.ts | 19 ++----- lib/__tests__/keyboard.test.ts | 80 +++++++++++++++++++++++++++++ lib/keyboard.ts | 26 ++++++++++ lib/plugin-sandbox/shortcuts.ts | 11 +--- 6 files changed, 120 insertions(+), 37 deletions(-) create mode 100644 lib/__tests__/keyboard.test.ts create mode 100644 lib/keyboard.ts diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 8236ea1e..df454863 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -14,6 +14,7 @@ import { ContextMenu, ContextMenuItem, ContextMenuSeparator } from "@/components import { sanitizeSignatureHtml, sanitizeSignatureHtmlForDisplay, sanitizeEmailHtml, escapeHtml } from "@/lib/email-sanitization"; import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix"; import { isFilePreviewable } from "@/lib/file-preview"; +import { isEditableEventTarget } from "@/lib/keyboard"; import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html"; import { buildSignatureBlock } from "@/components/email/signature-block"; import { emailHooks, contactHooks } from "@/lib/plugin-hooks"; @@ -1154,10 +1155,9 @@ export function EmailComposer({ useEffect(() => { const handleTemplateKey = (e: KeyboardEvent) => { - const target = e.target as HTMLElement; - const tag = target?.tagName?.toLowerCase(); - if (tag === 'input' || tag === 'textarea' || tag === 'select') return; - if (target?.getAttribute('contenteditable') === 'true') return; + // composedPath-based check so editing inside the QuotedHtml shadow + // island doesn't trigger the picker (#654). + if (isEditableEventTarget(e)) return; if (!templatesEnabled) return; if (e.key === 't' && !e.ctrlKey && !e.metaKey && !e.altKey) { e.preventDefault(); diff --git a/components/layout/sidebar.tsx b/components/layout/sidebar.tsx index 122a4bdb..bc61bcc6 100644 --- a/components/layout/sidebar.tsx +++ b/components/layout/sidebar.tsx @@ -38,6 +38,7 @@ import { } from "lucide-react"; import { cn, buildMailboxTree, MailboxNode } from "@/lib/utils"; import { localizeMailboxName } from "@/lib/mailbox-label"; +import { isEditableEventTarget } from "@/lib/keyboard"; import { Mailbox } from "@/lib/jmap/types"; import { useContextMenu } from "@/hooks/use-context-menu"; import { MailboxContextMenu, type MailboxContextTarget } from "./mailbox-context-menu"; @@ -887,16 +888,8 @@ export function Sidebar({ // window listener, so without this guard typing in a new email (the // contentEditable composer, the subject field, search, etc.) toggled the // selected mailbox's subfolders open/closed on ArrowLeft/ArrowRight. - const target = e.target as HTMLElement | null; - if ( - target && - (target.tagName === 'INPUT' || - target.tagName === 'TEXTAREA' || - target.tagName === 'SELECT' || - target.isContentEditable) - ) { - return; - } + // composedPath-based so it also sees the QuotedHtml shadow island (#654). + if (isEditableEventTarget(e)) return; if (!selectedMailbox || isCollapsed) return; const findNode = (nodes: MailboxNode[]): MailboxNode | null => { diff --git a/hooks/use-keyboard-shortcuts.ts b/hooks/use-keyboard-shortcuts.ts index a203ecca..03134a72 100644 --- a/hooks/use-keyboard-shortcuts.ts +++ b/hooks/use-keyboard-shortcuts.ts @@ -2,6 +2,7 @@ import { useEffect, useCallback, useRef } from "react"; import { Email } from "@/lib/jmap/types"; +import { isEditableEventTarget } from "@/lib/keyboard"; export interface KeyboardShortcutHandlers { // Navigation @@ -43,18 +44,6 @@ export interface UseKeyboardShortcutsOptions { handlers: KeyboardShortcutHandlers; } -// Check if user is typing in an input field -function isInputFocused(): boolean { - const activeElement = document.activeElement; - if (!activeElement) return false; - - const tagName = activeElement.tagName.toLowerCase(); - const isInput = tagName === "input" || tagName === "textarea" || tagName === "select"; - const isContentEditable = activeElement.getAttribute("contenteditable") === "true"; - - return isInput || isContentEditable; -} - // Shortcuts must fire regardless of the active keyboard layout (e.g. Cyrillic, // Greek). Derive the key from the PHYSICAL key (event.code) instead of the // layout-dependent event.key: letters from KeyA..KeyZ, and the symbol shortcuts @@ -94,8 +83,10 @@ export function useKeyboardShortcuts({ const handleKeyDown = useCallback( (event: KeyboardEvent) => { - // Don't trigger shortcuts when typing in inputs - if (isInputFocused()) return; + // Don't trigger shortcuts when typing in inputs. Must be event-based + // (composedPath), not document.activeElement: the QuotedHtml island's + // shadow root retargets activeElement to its plain-div host (#654). + if (isEditableEventTarget(event)) return; const h = handlersRef.current; const key = physicalShortcutKey(event); diff --git a/lib/__tests__/keyboard.test.ts b/lib/__tests__/keyboard.test.ts new file mode 100644 index 00000000..d01bf360 --- /dev/null +++ b/lib/__tests__/keyboard.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect, afterEach } from 'vitest'; + +import { isEditableEventTarget } from '../keyboard'; + +// Regression tests for #654: global single-key shortcuts fired while the user +// was editing inside the QuotedHtml shadow-DOM island, because the shadow +// boundary retargets both document.activeElement and event.target to the +// plain-div host. isEditableEventTarget must rely on composedPath instead. + +// Evaluate from a window-level listener DURING dispatch — composedPath() is +// only populated while the event is being dispatched, matching how the real +// shortcut handlers run. +function dispatchAndCheck(el: HTMLElement): { editable: boolean; target: EventTarget | null } { + let result: { editable: boolean; target: EventTarget | null } | null = null; + const listener = (e: Event) => { + result = { editable: isEditableEventTarget(e), target: e.target }; + }; + window.addEventListener('keydown', listener); + el.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Backspace', bubbles: true, composed: true }) + ); + window.removeEventListener('keydown', listener); + if (!result) throw new Error('keydown never reached window'); + return result; +} + +describe('isEditableEventTarget', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('detects plain inputs and textareas', () => { + for (const tag of ['input', 'textarea', 'select'] as const) { + const el = document.createElement(tag); + document.body.appendChild(el); + expect(dispatchAndCheck(el).editable).toBe(true); + } + }); + + it('detects a contentEditable element', () => { + const el = document.createElement('div'); + el.setAttribute('contenteditable', 'true'); + document.body.appendChild(el); + expect(dispatchAndCheck(el).editable).toBe(true); + }); + + it('returns false for a non-editable element', () => { + const el = document.createElement('div'); + document.body.appendChild(el); + expect(dispatchAndCheck(el).editable).toBe(false); + }); + + it('sees through a shadow boundary to an inner contentEditable (QuotedHtml island)', () => { + // Mirror the structure quoted-html.ts builds: plain-div host, open shadow + // root, inner contentEditable div. + const host = document.createElement('div'); + host.className = 'quoted-html-island'; + document.body.appendChild(host); + const shadow = host.attachShadow({ mode: 'open' }); + const inner = document.createElement('div'); + inner.setAttribute('contenteditable', 'true'); + shadow.appendChild(inner); + + const { editable, target } = dispatchAndCheck(inner); + // Sanity: the shadow boundary retargets the event — the outside listener + // sees the host, which is exactly why a target/activeElement check fails. + expect(target).toBe(host); + expect(editable).toBe(true); + }); + + it('still returns false for a non-editable shadow tree', () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const shadow = host.attachShadow({ mode: 'open' }); + const inner = document.createElement('div'); + shadow.appendChild(inner); + + expect(dispatchAndCheck(inner).editable).toBe(false); + }); +}); diff --git a/lib/keyboard.ts b/lib/keyboard.ts new file mode 100644 index 00000000..749d585b --- /dev/null +++ b/lib/keyboard.ts @@ -0,0 +1,26 @@ +// Shared helper for global key listeners to decide whether a keyboard event +// originated from a typing context (input, textarea, select, contentEditable). +// +// Checking `document.activeElement` / `event.target` is NOT enough: inside a +// shadow root both are retargeted to the host element, so the QuotedHtml +// island's inner contentEditable (components/email/quoted-html.ts) looks like +// a plain
from outside. Single-key mailbox shortcuts then fire while the +// user is editing quoted text — up to and including deleting the open email on +// Backspace (#654). `composedPath()` sees through the shadow boundary and +// starts at the real inner target, so it is the reliable signal. +export function isEditableEventTarget(event: Event): boolean { + const path = + typeof event.composedPath === "function" + ? event.composedPath() + : [event.target]; + return path.some((node) => { + if (!(node instanceof HTMLElement)) return false; + const tag = node.tagName.toLowerCase(); + if (tag === "input" || tag === "textarea" || tag === "select") return true; + if (node.isContentEditable) return true; + // jsdom (tests) doesn't implement isContentEditable; browsers reflect the + // property into the attribute, so this also covers "plaintext-only". + const attr = node.getAttribute("contenteditable"); + return attr === "" || attr === "true" || attr === "plaintext-only"; + }); +} diff --git a/lib/plugin-sandbox/shortcuts.ts b/lib/plugin-sandbox/shortcuts.ts index 73a94b3f..4d8b082a 100644 --- a/lib/plugin-sandbox/shortcuts.ts +++ b/lib/plugin-sandbox/shortcuts.ts @@ -8,6 +8,7 @@ // The listener ignores events when an editable element has focus, matching // the convention in `use-keyboard-shortcuts.ts`. +import { isEditableEventTarget } from '@/lib/keyboard'; import type { SandboxInstance } from './host-bridge'; interface Binding { @@ -62,16 +63,8 @@ function eventMatches(ev: KeyboardEvent, combo: NormalisedCombo): boolean { return ev.key.toLowerCase() === combo.key; } -function isEditableTarget(target: EventTarget | null): boolean { - if (!(target instanceof HTMLElement)) return false; - const tag = target.tagName.toLowerCase(); - if (tag === 'input' || tag === 'textarea' || tag === 'select') return true; - if (target.isContentEditable) return true; - return false; -} - function onKeyDown(ev: KeyboardEvent): void { - if (isEditableTarget(ev.target)) return; + if (isEditableEventTarget(ev)) return; if (bindings.size === 0) return; for (const binding of bindings.values()) { const combo = normaliseCombo(binding.keys);