From 5dafad264955a292f93a447ecd4e0181a3ef0ad5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 15:39:39 +0100 Subject: [PATCH] fix: stop Enter answering yes to a destructive plugin confirm while the focus is on Cancel --- .../__tests__/plugin-dialog-focus.test.tsx | 32 +++++++++++++++++++ components/plugins/plugin-dialog-host.tsx | 3 ++ 2 files changed, 35 insertions(+) diff --git a/components/plugins/__tests__/plugin-dialog-focus.test.tsx b/components/plugins/__tests__/plugin-dialog-focus.test.tsx index 53385b48d..2d83d761d 100644 --- a/components/plugins/__tests__/plugin-dialog-focus.test.tsx +++ b/components/plugins/__tests__/plugin-dialog-focus.test.tsx @@ -39,6 +39,38 @@ describe('plugin dialog focus', () => { await waitFor(() => expect(opener).toHaveFocus()); }); + it('leaves Enter to the focused button, so Cancel on a destructive confirm never answers yes', async () => { + render(); + let answer: Promise; + act(() => { answer = awaitDialog({ pluginId: 'p', kind: 'confirm', title: 'Delete it?', message: 'Sure?', danger: true }); }); + const cancel = screen.getByRole('button', { name: 'cancel' }); + expect(cancel).toHaveFocus(); + + // Not prevented: the browser turns Enter on a button into a click on it. + let notPrevented = false; + await act(async () => { notPrevented = fireEvent.keyDown(cancel, { key: 'Enter' }); }); + expect(notPrevented).toBe(true); + expect(head()).not.toBeNull(); + + await act(async () => { fireEvent.click(cancel); }); + await expect(answer!).resolves.toBe(false); + }); + + it('does not take Enter from the message of a destructive confirm as a yes', async () => { + render(); + act(() => { void awaitDialog({ pluginId: 'p', kind: 'confirm', title: 'Delete it?', message: 'Sure?', danger: true }); }); + await act(async () => { fireEvent.keyDown(screen.getByText('Sure?'), { key: 'Enter' }); }); + expect(head()).not.toBeNull(); + }); + + it('still accepts a harmless confirm on Enter from its message', async () => { + render(); + let answer: Promise; + act(() => { answer = awaitDialog({ pluginId: 'p', kind: 'confirm', title: 'Send?', message: 'Sure?' }); }); + await act(async () => { fireEvent.keyDown(screen.getByText('Sure?'), { key: 'Enter' }); }); + await expect(answer!).resolves.toBe(true); + }); + it('starts in the first field of a prompt and marks required fields', async () => { render(); let answer: Promise | null>; diff --git a/components/plugins/plugin-dialog-host.tsx b/components/plugins/plugin-dialog-host.tsx index a4fa12b38..24ed5d314 100644 --- a/components/plugins/plugin-dialog-host.tsx +++ b/components/plugins/plugin-dialog-host.tsx @@ -169,6 +169,9 @@ function PluginDialogView({ current, layer }: { current: DialogRequest; layer: ' // validation and works from within an input); confirm/alert accept. // Custom dialogs resolve only via their own onResult callback or the // host's X/Escape - an Enter press has no defined meaning for them. + // On a button Enter is that button's: Cancel must not answer yes. And + // a destructive confirm only says yes from its own button. + if ((e.target as Element | null)?.closest?.('button') || current.danger) return; e.preventDefault(); resolveThis(true); }