Skip to content

Commit 9909733

Browse files
committed
fix(ui): drop the composition guard — native submission already handles IMEs
The guard answered the wrong category of problem. `isComposing` checks are needed in *keydown* handlers, because an IME-consumed key still dispatches to JS — that is what the five removed Enter handlers were. Native form submission never sees that key: the IME consumes the confirming Enter (it reaches the page as keyCode 229, which the browser runs no default action for), so implicit submission cannot fire mid-composition. This is why no plain form on the web carries composition handling. The state the guard defended — composition open, unconsumed trusted Enter delivered — is one only CDP emulation can fabricate: `imeSetComposition` sets composition state with no IME in the loop to consume the key. No real IME produces the sequence. Worse, the guard carried real risk in the other direction: Gboard's action key commits the composition and submits in one press, so if any IME delivers `submit` before `compositionend`, the guard would swallow a legitimate submission — the original bug, reintroduced for exactly the users it claimed to protect. `Form.Root` goes back to plain `preventDefault` wiring, `useFormSubmit` is deleted, and the composition tests now pin the *native* contract against the real popover: accepting a candidate does not submit, Enter afterwards does.
1 parent 8c2f261 commit 9909733

7 files changed

Lines changed: 55 additions & 125 deletions

File tree

packages/ariakit/src/input/Form.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
11
import { FormProvider as AriakitFormProvider } from "@ariakit/react";
22

33
import { assertEmpty } from "@blocknote/core";
4-
import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react";
4+
import { ComponentProps, useDictionary } from "@blocknote/react";
55

66
export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => {
77
const { children, onSubmit, omitSubmitButton, ...rest } = props;
88
const dict = useDictionary();
9-
const formProps = useFormSubmit(onSubmit);
109

1110
assertEmpty(rest);
1211

1312
return (
1413
<AriakitFormProvider>
15-
<form {...formProps}>
14+
<form
15+
onSubmit={(event) => {
16+
// These forms have no action — a real submission would navigate.
17+
event.preventDefault();
18+
onSubmit?.();
19+
}}
20+
>
1621
{children}
1722
{/*
1823
Gives the form a submit button, which is what makes Enter submit it at

packages/mantine/src/form/Form.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
import { assertEmpty } from "@blocknote/core";
2-
import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react";
2+
import { ComponentProps, useDictionary } from "@blocknote/react";
33

44
export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => {
55
const { children, onSubmit, omitSubmitButton, ...rest } = props;
66
const dict = useDictionary();
7-
const formProps = useFormSubmit(onSubmit);
87

98
assertEmpty(rest);
109

1110
return (
12-
<form {...formProps}>
11+
<form
12+
onSubmit={(event) => {
13+
// These forms have no action — a real submission would navigate.
14+
event.preventDefault();
15+
onSubmit?.();
16+
}}
17+
>
1318
{children}
1419
{/*
1520
Gives the form a submit button, which is what makes Enter submit it at

packages/react/src/hooks/useFormSubmit.ts

Lines changed: 0 additions & 54 deletions
This file was deleted.

packages/react/src/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ export * from "./hooks/useCreateBlockNote.js";
136136
export * from "./hooks/useEditorChange.js";
137137
export * from "./hooks/useEditorFocus.js";
138138
export * from "./hooks/useEditorFocusChange.js";
139-
export * from "./hooks/useFormSubmit.js";
140139
export * from "./hooks/useEditorDomElement.js";
141140
export * from "./hooks/useEditorSelectionBoundingBox.js";
142141
export * from "./hooks/useEditorSelectionChange.js";

packages/shadcn/src/form/Form.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
import { assertEmpty } from "@blocknote/core";
2-
import { ComponentProps, useDictionary, useFormSubmit } from "@blocknote/react";
2+
import { ComponentProps, useDictionary } from "@blocknote/react";
33

44
export const Form = (props: ComponentProps["Generic"]["Form"]["Root"]) => {
55
const { children, onSubmit, omitSubmitButton, ...rest } = props;
66
const dict = useDictionary();
7-
const formProps = useFormSubmit(onSubmit);
87

98
assertEmpty(rest);
109

1110
return (
12-
<form {...formProps}>
11+
<form
12+
onSubmit={(event) => {
13+
// These forms have no action — a real submission would navigate.
14+
event.preventDefault();
15+
onSubmit?.();
16+
}}
17+
>
1318
{children}
1419
{/*
1520
Gives the form a submit button, which is what makes Enter submit it at

tests/src/end-to-end/form/compositionSubmit.test.tsx

Lines changed: 20 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,23 @@ import { browserName, commands, userEvent } from "../../utils/context.js";
33
import type { ImeCompositionCommand } from "../../utils/imeComposition.js";
44

55
/**
6-
* Every popover Enter handler used to guard on `isComposing`, so that Enter
7-
* pressed to accept an IME candidate committed the candidate instead of the
8-
* form. Those handlers are gone — submission now runs off the form's `submit`
9-
* event — which moves the question to the platform: can a composition-ending
10-
* Enter reach a form as an implicit submission?
6+
* Why the popover forms need no composition guard.
117
*
12-
* If it can, dropping the guards regressed CJK input everywhere, and the
13-
* guards have to come back at the form level. So it is asserted rather than
14-
* assumed.
8+
* The Enter handlers that `Form.Root`'s submit path replaced all guarded on
9+
* `isComposing` — necessary for a *keydown* handler, because the keydown for
10+
* an IME-consumed key still dispatches to JS. Native form submission is a
11+
* different category: the IME consumes the confirming Enter (it reaches the
12+
* page as keyCode 229, which the browser runs no default action for), so
13+
* implicit submission never fires mid-composition. This is why no plain
14+
* `<form onSubmit>` in the world carries composition handling.
15+
*
16+
* These tests pin the two halves of that contract on the real IME event
17+
* sequence. What they deliberately do *not* do is inject a bare Enter while
18+
* composition is held open: CDP can fabricate that state, and the browser
19+
* does submit on it, but no real IME delivers an unconsumed Enter
20+
* mid-composition — and guarding against the fabricated state would mean
21+
* betting that every IME fires `compositionend` before the submit it
22+
* triggers, or a Gboard-style single-press commit-and-submit gets swallowed.
1523
*/
1624

1725
const browserCommands = commands as typeof commands & {
@@ -34,15 +42,8 @@ function buildForm() {
3442
form = document.createElement("form");
3543
const submits: string[] = [];
3644
const compositions: string[] = [];
37-
// Mirrors what `useFormSubmit` wires onto a real `Form.Root`.
38-
let composing = false;
39-
form.addEventListener("compositionstart", () => (composing = true));
40-
form.addEventListener("compositionend", () => (composing = false));
4145
form.addEventListener("submit", (event) => {
4246
event.preventDefault();
43-
if (composing) {
44-
return;
45-
}
4647
submits.push("submit");
4748
});
4849

@@ -67,13 +68,14 @@ function buildForm() {
6768
return { input, submits, compositions };
6869
}
6970

70-
describeIme("Enter during an IME composition", () => {
71+
describeIme("IME composition and form submission", () => {
7172
test("accepting a candidate does not submit the form", async () => {
73+
// The real accept path: the IME replaces the composition with the final
74+
// text (`insertText`), and the confirming key never reaches the page as
75+
// an actionable Enter — so nothing submits, natively.
7276
const { input, submits, compositions } = buildForm();
7377
input.focus();
7478

75-
// Accepting a candidate the way an IME does: the final text replaces the
76-
// composing text, and the confirming key never reaches the page.
7779
await browserCommands.imeComposition([
7880
{ type: "setComposition", text: "にほん" },
7981
{ type: "commit", text: "日本" },
@@ -87,36 +89,6 @@ describeIme("Enter during an IME composition", () => {
8789
).toEqual([]);
8890
});
8991

90-
test("Enter arriving mid-composition does not submit the form", async () => {
91-
// The case that makes the guard necessary rather than defensive: the
92-
// browser delivers this Enter as `keydown` with `isComposing: true` and
93-
// performs implicit submission for it regardless, so without the guard a
94-
// CJK user accepting a candidate submits the popover mid-word.
95-
const { input, submits, compositions } = buildForm();
96-
const composingOnKeyDown: boolean[] = [];
97-
input.addEventListener("keydown", (event) =>
98-
composingOnKeyDown.push(event.isComposing),
99-
);
100-
input.focus();
101-
102-
await browserCommands.imeComposition([
103-
{ type: "setComposition", text: "にほん" },
104-
]);
105-
await userEvent.keyboard("{Enter}");
106-
107-
// Pin the precondition too: if a future engine stopped delivering this
108-
// Enter to the page, the guard would be untested rather than unnecessary.
109-
expect(
110-
composingOnKeyDown,
111-
"Enter must reach the page mid-composition",
112-
).toEqual([true]);
113-
expect(compositions).not.toContain("compositionend");
114-
expect(
115-
submits,
116-
"Enter must not submit while a composition is in progress",
117-
).toEqual([]);
118-
});
119-
12092
test("Enter after the composition ends does submit", async () => {
12193
// The other half of the contract: once composition is over, Enter has to
12294
// work normally, or CJK users could never submit at all.

tests/src/end-to-end/form/popoverSubmit.test.tsx

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ describe("Submitting a toolbar popover with Enter", () => {
154154
// `Input.imeSetComposition` is CDP-only, so the real composition state can
155155
// only be entered in chromium.
156156
test.skipIf(browserName !== "chromium")(
157-
"Enter mid-composition does not commit the popover",
157+
"accepting an IME candidate does not commit the popover",
158158
async () => {
159-
// The platform performs implicit submission for an Enter delivered with
160-
// `isComposing: true` (see ./compositionSubmit.test.tsx), so accepting
161-
// an IME candidate would otherwise commit the link mid-word. This drives
162-
// the real popover rather than a stand-in, so it covers the guard
163-
// `Form.Root` actually ships.
159+
// The real accept path: the IME consumes the confirming key and
160+
// replaces the composition with the final text, so no actionable Enter
161+
// reaches the page and nothing submits — natively, with no composition
162+
// guard in `Form.Root` (see ./compositionSubmit.test.tsx for why none
163+
// is needed).
164164
await focusOnEditor();
165165
await userEvent.keyboard("link me");
166166
await userEvent.keyboard("{Home}{Shift>}{End}{/Shift}");
@@ -171,19 +171,17 @@ describe("Submitting a toolbar popover with Enter", () => {
171171
await userEvent.click(input);
172172

173173
await browserCommands.imeComposition([
174-
{ type: "setComposition", text: "にほん" },
174+
{ type: "setComposition", text: "example.co" },
175+
{ type: "commit", text: "example.com" },
175176
]);
176-
await userEvent.keyboard("{Enter}");
177177

178+
expect(input.value).toBe("example.com");
178179
expect(
179180
document.querySelector(`${EDITOR_SELECTOR} a`),
180-
"accepting an IME candidate must not commit the link",
181+
"accepting a candidate must not commit the link",
181182
).toBeNull();
182183

183-
// And once composition is over, Enter still works.
184-
await browserCommands.imeComposition([
185-
{ type: "commit", text: "example.com" },
186-
]);
184+
// Enter after the composition commits it as usual.
187185
await userEvent.keyboard("{Enter}");
188186
await waitForSelector(`${EDITOR_SELECTOR} a`);
189187
},

0 commit comments

Comments
 (0)