From ec4fd9c529af27afa05854c5570e58dc4eec837c Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:03:05 +0100 Subject: [PATCH] fix: keep mail shortcuts working after a click into an HTML message --- .../conversation-frame-keys.test.tsx | 64 +++++++++++ components/email/email-viewer.tsx | 3 + components/email/thread-conversation-view.tsx | 3 + lib/__tests__/email-frame-keys.test.ts | 108 ++++++++++++++++++ lib/email-frame-keys.ts | 90 +++++++++++++++ 5 files changed, 268 insertions(+) create mode 100644 components/email/__tests__/conversation-frame-keys.test.tsx create mode 100644 lib/__tests__/email-frame-keys.test.ts create mode 100644 lib/email-frame-keys.ts diff --git a/components/email/__tests__/conversation-frame-keys.test.tsx b/components/email/__tests__/conversation-frame-keys.test.tsx new file mode 100644 index 000000000..a39dd5012 --- /dev/null +++ b/components/email/__tests__/conversation-frame-keys.test.tsx @@ -0,0 +1,64 @@ +// mail-3: a key pressed after a click into an HTML message of a conversation +// stayed in the message's frame, so the mail shortcuts did nothing. +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { fireEvent, render } from '@testing-library/react'; + +const t = Object.assign((k: string) => k, { raw: (k: string) => k, rich: (k: string) => k }); +vi.mock('next-intl', () => ({ + useTranslations: () => t, + useLocale: () => 'en', +})); +vi.mock('@/hooks/use-wopi-status', () => ({ useWopiStatus: () => ({ enabled: false }), canWopiOpen: () => false })); +vi.mock('@/hooks/use-own-domain-address', () => ({ useOwnDomainAddress: () => () => false })); + +class NoopResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} +vi.stubGlobal('ResizeObserver', NoopResizeObserver); + +const { ThreadConversationView } = await import('@/components/email/thread-conversation-view'); + +function renderHtmlMessage() { + const email = { + id: 'e1', threadId: 't1', mailboxIds: { a: true }, keywords: { $seen: true }, + from: [{ name: 'Sender', email: 'sender@t.example' }], to: [{ email: 'me@example.org' }], + subject: 's', receivedAt: '2026-09-26T10:00:00Z', size: 1, preview: 'hi', hasAttachment: false, + htmlBody: [{ partId: '1', type: 'text/html' }], textBody: [], attachments: [], + bodyValues: { '1': { value: '

Newsletter

' } }, + }; + const { container } = render( + {}} + />, + ); + const iframe = container.querySelector('iframe')!; + const srcdoc = iframe.getAttribute('srcdoc') ?? ''; + const doc = iframe.contentDocument!; + doc.body.innerHTML = /]*>([\s\S]*)<\/body>/.exec(srcdoc)![1]; + fireEvent.load(iframe); + return doc; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('keys in a conversation message', () => { + it('reach the page, where the shortcut handlers listen', () => { + const doc = renderHtmlMessage(); + const heard: string[] = []; + const listen = (e: KeyboardEvent) => heard.push(e.key); + window.addEventListener('keydown', listen); + try { + const FrameKeyboardEvent = (doc.defaultView as unknown as { KeyboardEvent: typeof KeyboardEvent }).KeyboardEvent; + doc.getElementById('p')!.dispatchEvent(new FrameKeyboardEvent('keydown', { key: 'j', code: 'KeyJ', bubbles: true, cancelable: true })); + } finally { + window.removeEventListener('keydown', listen); + } + expect(heard).toEqual(['j']); + }); +}); diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 0b1b5fac7..dc1ed92b0 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -23,6 +23,7 @@ import { collectReferencedCids, isEmbeddedInBody, isReportPart, normalizeCid, re import { getAttachmentDisplayName } from "@/lib/attachment-name"; import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse"; import { measureEmailFrameHeight } from "@/lib/email-frame-height"; +import { forwardFrameKeys } from "@/lib/email-frame-keys"; import { DARK_PENDING_CLASS, applyEmailDarkMode, emailColorScheme, emailDarkModeCss, emailHasNativeDarkMode as htmlHasNativeDarkMode, resolveColorSchemeMedia } from "@/lib/email-dark-mode"; import { fitEmailBodyWidth, type FitOptions } from "@/lib/email-fit-width"; import { hasFullBleedCanvas } from "@/lib/email-full-bleed"; @@ -3733,6 +3734,8 @@ export function EmailViewer({ onSelection: (text) => { if (iframe.contentDocument === doc) onBodySelection(text); }, onContextMenu: (point, text) => selectionMenu.openRef.current(point, { emailId: emailIdRef.current, text }), }); + // j, k, e and the other shortcuts keep working after a click into the body. + forwardFrameKeys(doc, iframe); } } catch { diff --git a/components/email/thread-conversation-view.tsx b/components/email/thread-conversation-view.tsx index 0977c9136..328f64a97 100644 --- a/components/email/thread-conversation-view.tsx +++ b/components/email/thread-conversation-view.tsx @@ -20,6 +20,7 @@ import { ReceivedLabelChip } from "./received-label-chip"; import { collectReferencedCids, isEmbeddedInBody, isReportPart, normalizeCid, rewriteCidReferences } from "@/lib/attachment-visibility"; import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse"; import { measureEmailFrameHeight } from "@/lib/email-frame-height"; +import { forwardFrameKeys } from "@/lib/email-frame-keys"; import { fitEmailBodyWidth } from "@/lib/email-fit-width"; import { mailLanguage } from "@/lib/mail-language"; import { htmlToReadableText } from "@/lib/html-to-readable-text"; @@ -926,6 +927,8 @@ export function ConversationEmailBody({ emailId: email.id, onContextMenu: (point, text) => selectionMenu.openRef.current(point, { emailId: email.id, text }), }); + // j, k, e and the other shortcuts keep working after a click into the body. + forwardFrameKeys(doc, iframe); setHtmlBodyEl(doc.body); } catch { // contentDocument may be inaccessible under stricter sandboxes; ignore. diff --git a/lib/__tests__/email-frame-keys.test.ts b/lib/__tests__/email-frame-keys.test.ts new file mode 100644 index 000000000..e8cf4da93 --- /dev/null +++ b/lib/__tests__/email-frame-keys.test.ts @@ -0,0 +1,108 @@ +import { renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { useKeyboardShortcuts } from '@/hooks/use-keyboard-shortcuts'; +import { forwardFrameKeys } from '../email-frame-keys'; + +// mail-3: after a click into an HTML message, j, k, e, # and the rest did +// nothing, since the key presses went to the body's frame and never reached +// the page's shortcut handlers. + +const email = { id: 'm1' } as never; + +function frameWith(html: string) { + const frame = document.createElement('iframe'); + document.body.appendChild(frame); + const doc = frame.contentDocument!; + doc.body.innerHTML = html; + forwardFrameKeys(doc, frame); + return { frame, doc }; +} + +/** A key press as the frame's own window makes it. */ +function pressIn(doc: Document, target: Element, key: string, code: string, init: KeyboardEventInit = {}) { + const FrameKeyboardEvent = (doc.defaultView as unknown as { KeyboardEvent: typeof KeyboardEvent }).KeyboardEvent; + const event = new FrameKeyboardEvent('keydown', { key, code, bubbles: true, cancelable: true, ...init }); + target.dispatchEvent(event); + return event; +} + +function mountShortcuts() { + const handlers = { onNextEmail: vi.fn(), onArchive: vi.fn(), onDelete: vi.fn(), onCloseEmail: vi.fn(), onSelectAll: vi.fn() }; + renderHook(() => useKeyboardShortcuts({ emails: [email], selectedEmailId: 'm1', handlers })); + return handlers; +} + +afterEach(() => { + document.body.innerHTML = ''; +}); + +describe('keys pressed in a message body', () => { + it('reach the app shortcuts: j, e and #', () => { + const handlers = mountShortcuts(); + const { doc } = frameWith('

Newsletter

'); + const p = doc.querySelector('p')!; + const j = pressIn(doc, p, 'j', 'KeyJ'); + pressIn(doc, p, 'e', 'KeyE'); + pressIn(doc, p, '#', 'Digit3', { shiftKey: true }); + expect(handlers.onNextEmail).toHaveBeenCalledTimes(1); + expect(handlers.onArchive).toHaveBeenCalledTimes(1); + expect(handlers.onDelete).toHaveBeenCalledTimes(1); + // The frame doesn't act on a key a shortcut took. + expect(j.defaultPrevented).toBe(true); + }); + + it('stay in a form inside the mail', () => { + const handlers = mountShortcuts(); + const { doc } = frameWith('
x
'); + pressIn(doc, doc.querySelector('input')!, 'j', 'KeyJ'); + pressIn(doc, doc.querySelector('textarea')!, 'e', 'KeyE'); + pressIn(doc, doc.querySelector('b')!, 'Backspace', 'Backspace'); + expect(handlers.onNextEmail).not.toHaveBeenCalled(); + expect(handlers.onArchive).not.toHaveBeenCalled(); + expect(handlers.onDelete).not.toHaveBeenCalled(); + }); + + it('leave Ctrl+A, Ctrl+C and the scrolling keys to the body', () => { + const handlers = mountShortcuts(); + const { doc } = frameWith('

Text

'); + const p = doc.querySelector('p')!; + const selectAll = pressIn(doc, p, 'a', 'KeyA', { ctrlKey: true }); + const copy = pressIn(doc, p, 'c', 'KeyC', { ctrlKey: true }); + const space = pressIn(doc, p, ' ', 'Space'); + const down = pressIn(doc, p, 'ArrowDown', 'ArrowDown'); + // Ctrl+A in the list selects every message; in the body it selects the text. + expect(handlers.onSelectAll).not.toHaveBeenCalled(); + expect(handlers.onNextEmail).not.toHaveBeenCalled(); + for (const event of [selectAll, copy, space, down]) expect(event.defaultPrevented).toBe(false); + }); + + it('Escape puts the focus back on the open message in the list', () => { + const handlers = mountShortcuts(); + const list = document.createElement('div'); + list.id = 'message-list'; + list.innerHTML = '
A
B
'; + document.body.appendChild(list); + const { doc } = frameWith('

Text

'); + pressIn(doc, doc.querySelector('p')!, 'Escape', 'Escape'); + expect(document.activeElement?.textContent).toBe('B'); + // Only the focus moves: the message stays open. + expect(handlers.onCloseEmail).not.toHaveBeenCalled(); + }); + + it('Escape without a list (a message window) is the app\'s Escape', () => { + const handlers = mountShortcuts(); + const { doc } = frameWith('

Text

'); + pressIn(doc, doc.querySelector('p')!, 'Escape', 'Escape'); + expect(handlers.onCloseEmail).toHaveBeenCalledTimes(1); + }); + + it('stop while a menu is open on the page', () => { + const handlers = mountShortcuts(); + const menu = document.createElement('div'); + menu.setAttribute('role', 'menu'); + document.body.appendChild(menu); + const { doc } = frameWith('

Text

'); + pressIn(doc, doc.querySelector('p')!, 'e', 'KeyE'); + expect(handlers.onArchive).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/email-frame-keys.ts b/lib/email-frame-keys.ts new file mode 100644 index 000000000..7ae575e3a --- /dev/null +++ b/lib/email-frame-keys.ts @@ -0,0 +1,90 @@ +/** + * An HTML message renders in a frame, and a key pressed after a click into it + * goes to the frame's own window: j, k, e, # and the rest stopped working + * until the user clicked back into the list. This hands those keys on to the + * page as a copy of the key press, sent from the frame element, so the app's + * shortcut handlers (hooks/use-keyboard-shortcuts, hooks/use-undo-shortcut) + * take it with their usual checks: an open menu or dialog, single-key + * shortcuts turned off. Escape puts the focus back on the message's row in + * the list, as Escape in a dialog gives the focus back. + * + * Left to the frame: anything typed into a form in the mail, keys with Ctrl, + * Alt or ⌘ (copy, select all), and the keys that scroll the body or move + * through its links (arrows, Space, Tab, Enter, Page Up and Down). + */ + +import { focusTemporarily } from "@/lib/keyboard"; + +/** Keys that aren't one character but are mail shortcuts. */ +const NAMED_KEYS = new Set(["Delete", "Backspace", "Escape"]); + +const EDITABLE = + 'input, textarea, select, [contenteditable=""], [contenteditable="true"], [contenteditable="plaintext-only"]'; + +/** + * Whether a key press in the frame came from a place that takes text. By tag + * and attribute: the frame's elements belong to another window, so + * `instanceof HTMLElement` (lib/keyboard) is false for all of them. + */ +function typedInFrame(event: KeyboardEvent, doc: Document): boolean { + if (doc.designMode === "on") return true; + const target = event.target as Node | null; + const el = target && target.nodeType === 1 ? (target as Element) : target?.parentElement ?? null; + if (!el) return false; + if (el.closest(EDITABLE)) return true; + return (el as HTMLElement).isContentEditable === true; +} + +/** Whether this key press in the frame belongs to the app. */ +export function isFrameKeyForApp(event: KeyboardEvent, doc: Document): boolean { + if (event.defaultPrevented || event.isComposing) return false; + if (event.ctrlKey || event.metaKey || event.altKey) return false; + if (event.key === " " || (event.key.length !== 1 && !NAMED_KEYS.has(event.key))) return false; + return !typedInFrame(event, doc); +} + +/** + * The open message's row in the message list, or the list itself when no row + * is open. Null where there is no list (a message window). + */ +function listFocusTarget(doc: Document): HTMLElement | null { + const list = doc.getElementById("message-list"); + if (!list) return null; + return ( + list.querySelector('[role="row"][aria-current="true"]') ?? + list.querySelector('[role="row"][tabindex="0"]') ?? + list + ); +} + +/** + * Hands the app's keys from a message frame's document to the page. Returns a + * function that stops it; a frame whose document is replaced stops by itself. + */ +export function forwardFrameKeys(doc: Document, frame: HTMLIFrameElement): () => void { + const onKeyDown = (event: KeyboardEvent) => { + if (!isFrameKeyForApp(event, doc)) return; + if (event.key === "Escape") { + const target = listFocusTarget(frame.ownerDocument); + if (target) { + event.preventDefault(); + focusTemporarily(target, { preventScroll: true }); + return; + } + } + const copy = new KeyboardEvent("keydown", { + key: event.key, + code: event.code, + shiftKey: event.shiftKey, + repeat: event.repeat, + bubbles: true, + cancelable: true, + composed: true, + }); + frame.dispatchEvent(copy); + // A shortcut ran: the frame doesn't act on the key as well. + if (copy.defaultPrevented) event.preventDefault(); + }; + doc.addEventListener("keydown", onKeyDown); + return () => doc.removeEventListener("keydown", onKeyDown); +}