fix: detect typing inside the QuotedHtml shadow island via composedPath #654
This commit is contained in:
@@ -14,6 +14,7 @@ import { ContextMenu, ContextMenuItem, ContextMenuSeparator } from "@/components
|
|||||||
import { sanitizeSignatureHtml, sanitizeSignatureHtmlForDisplay, sanitizeEmailHtml, escapeHtml } from "@/lib/email-sanitization";
|
import { sanitizeSignatureHtml, sanitizeSignatureHtmlForDisplay, sanitizeEmailHtml, escapeHtml } from "@/lib/email-sanitization";
|
||||||
import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix";
|
import { buildReplySubject, buildForwardSubject } from "@/lib/subject-prefix";
|
||||||
import { isFilePreviewable } from "@/lib/file-preview";
|
import { isFilePreviewable } from "@/lib/file-preview";
|
||||||
|
import { isEditableEventTarget } from "@/lib/keyboard";
|
||||||
import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html";
|
import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html";
|
||||||
import { buildSignatureBlock } from "@/components/email/signature-block";
|
import { buildSignatureBlock } from "@/components/email/signature-block";
|
||||||
import { emailHooks, contactHooks } from "@/lib/plugin-hooks";
|
import { emailHooks, contactHooks } from "@/lib/plugin-hooks";
|
||||||
@@ -1154,10 +1155,9 @@ export function EmailComposer({
|
|||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const handleTemplateKey = (e: KeyboardEvent) => {
|
const handleTemplateKey = (e: KeyboardEvent) => {
|
||||||
const target = e.target as HTMLElement;
|
// composedPath-based check so editing inside the QuotedHtml shadow
|
||||||
const tag = target?.tagName?.toLowerCase();
|
// island doesn't trigger the picker (#654).
|
||||||
if (tag === 'input' || tag === 'textarea' || tag === 'select') return;
|
if (isEditableEventTarget(e)) return;
|
||||||
if (target?.getAttribute('contenteditable') === 'true') return;
|
|
||||||
if (!templatesEnabled) return;
|
if (!templatesEnabled) return;
|
||||||
if (e.key === 't' && !e.ctrlKey && !e.metaKey && !e.altKey) {
|
if (e.key === 't' && !e.ctrlKey && !e.metaKey && !e.altKey) {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
|
|||||||
@@ -38,6 +38,7 @@ import {
|
|||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import { cn, buildMailboxTree, MailboxNode } from "@/lib/utils";
|
import { cn, buildMailboxTree, MailboxNode } from "@/lib/utils";
|
||||||
import { localizeMailboxName } from "@/lib/mailbox-label";
|
import { localizeMailboxName } from "@/lib/mailbox-label";
|
||||||
|
import { isEditableEventTarget } from "@/lib/keyboard";
|
||||||
import { Mailbox } from "@/lib/jmap/types";
|
import { Mailbox } from "@/lib/jmap/types";
|
||||||
import { useContextMenu } from "@/hooks/use-context-menu";
|
import { useContextMenu } from "@/hooks/use-context-menu";
|
||||||
import { MailboxContextMenu, type MailboxContextTarget } from "./mailbox-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
|
// window listener, so without this guard typing in a new email (the
|
||||||
// contentEditable composer, the subject field, search, etc.) toggled the
|
// contentEditable composer, the subject field, search, etc.) toggled the
|
||||||
// selected mailbox's subfolders open/closed on ArrowLeft/ArrowRight.
|
// selected mailbox's subfolders open/closed on ArrowLeft/ArrowRight.
|
||||||
const target = e.target as HTMLElement | null;
|
// composedPath-based so it also sees the QuotedHtml shadow island (#654).
|
||||||
if (
|
if (isEditableEventTarget(e)) return;
|
||||||
target &&
|
|
||||||
(target.tagName === 'INPUT' ||
|
|
||||||
target.tagName === 'TEXTAREA' ||
|
|
||||||
target.tagName === 'SELECT' ||
|
|
||||||
target.isContentEditable)
|
|
||||||
) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
if (!selectedMailbox || isCollapsed) return;
|
if (!selectedMailbox || isCollapsed) return;
|
||||||
|
|
||||||
const findNode = (nodes: MailboxNode[]): MailboxNode | null => {
|
const findNode = (nodes: MailboxNode[]): MailboxNode | null => {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
import { useEffect, useCallback, useRef } from "react";
|
import { useEffect, useCallback, useRef } from "react";
|
||||||
import { Email } from "@/lib/jmap/types";
|
import { Email } from "@/lib/jmap/types";
|
||||||
|
import { isEditableEventTarget } from "@/lib/keyboard";
|
||||||
|
|
||||||
export interface KeyboardShortcutHandlers {
|
export interface KeyboardShortcutHandlers {
|
||||||
// Navigation
|
// Navigation
|
||||||
@@ -43,18 +44,6 @@ export interface UseKeyboardShortcutsOptions {
|
|||||||
handlers: KeyboardShortcutHandlers;
|
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,
|
// 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
|
// 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
|
// layout-dependent event.key: letters from KeyA..KeyZ, and the symbol shortcuts
|
||||||
@@ -94,8 +83,10 @@ export function useKeyboardShortcuts({
|
|||||||
|
|
||||||
const handleKeyDown = useCallback(
|
const handleKeyDown = useCallback(
|
||||||
(event: KeyboardEvent) => {
|
(event: KeyboardEvent) => {
|
||||||
// Don't trigger shortcuts when typing in inputs
|
// Don't trigger shortcuts when typing in inputs. Must be event-based
|
||||||
if (isInputFocused()) return;
|
// (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 h = handlersRef.current;
|
||||||
const key = physicalShortcutKey(event);
|
const key = physicalShortcutKey(event);
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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 <div> 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";
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -8,6 +8,7 @@
|
|||||||
// The listener ignores events when an editable element has focus, matching
|
// The listener ignores events when an editable element has focus, matching
|
||||||
// the convention in `use-keyboard-shortcuts.ts`.
|
// the convention in `use-keyboard-shortcuts.ts`.
|
||||||
|
|
||||||
|
import { isEditableEventTarget } from '@/lib/keyboard';
|
||||||
import type { SandboxInstance } from './host-bridge';
|
import type { SandboxInstance } from './host-bridge';
|
||||||
|
|
||||||
interface Binding {
|
interface Binding {
|
||||||
@@ -62,16 +63,8 @@ function eventMatches(ev: KeyboardEvent, combo: NormalisedCombo): boolean {
|
|||||||
return ev.key.toLowerCase() === combo.key;
|
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 {
|
function onKeyDown(ev: KeyboardEvent): void {
|
||||||
if (isEditableTarget(ev.target)) return;
|
if (isEditableEventTarget(ev)) return;
|
||||||
if (bindings.size === 0) return;
|
if (bindings.size === 0) return;
|
||||||
for (const binding of bindings.values()) {
|
for (const binding of bindings.values()) {
|
||||||
const combo = normaliseCombo(binding.keys);
|
const combo = normaliseCombo(binding.keys);
|
||||||
|
|||||||
Reference in New Issue
Block a user