jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-11-14 05:12:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(desktop): keep the draft space when typing right after a mention pick (#6875)

Browse Source 
## Problem

Desktop Smoke E2E flaked on `mentions.spec.ts` ("typing a mention before
existing text does not interleave spaces"): picking a suggestion
mid-draft and typing immediately could swallow the draft's own space,
producing `hello @bob abcworld` instead of `hello @bob a world`.

With `hello world` drafted and ` @bo` typed after `hello`, picking the
suggestion leaves `hello @bob world` — the mention's trailing space
followed by the draft's space. Chromium reconciles that whitespace run
by re-emitting it as a replacement (`replace(" " -> " a")`), anchored at
*either* edge of the run. The settling-time text-input redirect only
recognized a caret or a rewrite anchored at the chip edge, so the other
shapes fell through to ProseMirror's destructive default and ate the
separator.

The race pre-dates this branch and reproduces on `main`.

## Fix

`mentionHighlightExtension.ts` now redirects on the invariant instead of
on a specific rewrite shape. While autocomplete is settling the user
cannot have a selection (a selection cancels settlement), so a
whitespace-only replacement adjacent to a mention's trailing space is
always the browser normalizing whitespace — never an intentional delete.
The handler keeps every space already in the document and inserts only
the typed characters after the token, whichever edge the rewrite is
anchored at. Non-whitespace replacements still fall through as real
edits.

`insertPosForMentionTextInput` / `mentionTextInputInsertPos` become
`insertionForMentionTextInput` / `mentionTextInputInsertion`, returning
`{insertAt, text}` so the handler can rewrite the payload, not just the
position.

## Tests

- Helper-level cases for all four anchors (caret at chip edge, caret
past the trailing space, run rewritten from the chip edge, draft space
rewritten alone), plus space/NBSP variants and a non-whitespace
replacement that must be left alone.
- A plugin-level test drives the real `handleTextInput` prop and falls
back to ProseMirror's default when it declines — exactly as the browser
does with the return value — so it fails against the previous production
code rather than depending on winning a timing race in a headless
browser.

Desktop unit suite passes on pre-push (5558/5558).

---------

Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Matt Toohey
2026-08-27 17:37:42 +10:00
committed by GitHub
parent b622003f74
commit c856be0fb9
2 changed files with 213 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

159
desktop/src/features/messages/lib/mentionHighlightExtension.test.mjs
View File

@@ -10,9 +10,10 @@ import {
buildHighlightPatterns,
createMentionCaretSettlement,
findHighlightMatches,
insertPosForMentionTextInput,
insertionForMentionTextInput,
MentionHighlightExtension,
mentionTextInputInsertPos,
mentionHighlightKey,
mentionTextInputInsertion,
positionAfterArrowLeftThroughMentionSpace,
selectionAfterMentionTrailingSpace,
shouldAdvanceMentionCaret,
@@ -252,35 +253,79 @@ test("createMentionCaretSettlement keeps two editors independent", () => {
assert.equal(composerB.peek(), 12);
});
test("insertPosForMentionTextInput redirects a caret at the chip edge", () => {
test("insertionForMentionTextInput redirects a caret at the chip edge", () => {
const doc = document(paragraph(text("@quinn ")));
const spacePos = 1 + "@quinn".length;
assert.deepEqual(insertionForMentionTextInput(doc, spacePos, spacePos, "x"), {
insertAt: spacePos + 1,
text: "x",
});
assert.equal(
insertPosForMentionTextInput(doc, spacePos, spacePos),
spacePos + 1,
);
assert.equal(
insertPosForMentionTextInput(doc, spacePos + 1, spacePos + 1),
insertionForMentionTextInput(doc, spacePos + 1, spacePos + 1, "x"),
null,
);
});
test("insertPosForMentionTextInput keeps a selected trailing space", () => {
test("insertionForMentionTextInput keeps a selected trailing space", () => {
const doc = document(paragraph(text("@quinn ")));
const spacePos = 1 + "@quinn".length;
assert.equal(
insertPosForMentionTextInput(doc, spacePos, spacePos + 1),
spacePos + 1,
assert.deepEqual(
insertionForMentionTextInput(doc, spacePos, spacePos + 1, "x"),
{ insertAt: spacePos + 1, text: "x" },
);
});
test("mentionTextInputInsertPos honors a deliberate caret after settlement", () => {
test("insertionForMentionTextInput keeps the draft space in a whitespace-run rewrite", () => {
// Chromium can rewrite the whole space run when typing between the
// mention's trailing space and a pre-existing draft space, emitting
// replace(" " -> " a") — usually with a non-breaking space, and anchored
// at either edge of the run. The draft's space must survive every shape.
const doc = document(paragraph(text("hello @bob world")));
const spacePos = 1 + "hello @bob".length;
const kept = { insertAt: spacePos + 1, text: "a" };
// Anchored at the chip edge, replacing the whole run.
assert.deepEqual(
insertionForMentionTextInput(doc, spacePos, spacePos + 2, " a"),
kept,
);
assert.deepEqual(
insertionForMentionTextInput(doc, spacePos, spacePos + 2, "\u00A0a"),
kept,
);
// Anchored past the trailing space, rewriting only the draft's own space.
// This shape used to fall through to the destructive default and produce
// "hello @bob abcworld" — the failure CI caught.
assert.deepEqual(
insertionForMentionTextInput(doc, spacePos + 1, spacePos + 2, "\u00A0a"),
kept,
);
// Trailing whitespace is the run being re-emitted on the other side.
assert.deepEqual(
insertionForMentionTextInput(doc, spacePos, spacePos + 2, "a\u00A0"),
kept,
);
// Whatever the shape, replaced whitespace is never dropped while settling.
assert.deepEqual(
insertionForMentionTextInput(doc, spacePos, spacePos + 2, "x"),
{ insertAt: spacePos + 1, text: "x" },
);
// Replacing something other than whitespace is a real edit — leave it.
assert.equal(
insertionForMentionTextInput(doc, spacePos, spacePos + 3, " a"),
null,
);
});
test("mentionTextInputInsertion honors a deliberate caret after settlement", () => {
const doc = document(paragraph(text("@bob ")));
const spacePos = 1 + "@bob".length;
assert.equal(mentionTextInputInsertPos(doc, spacePos, spacePos, false), null);
assert.equal(
mentionTextInputInsertPos(doc, spacePos, spacePos, true),
spacePos + 1,
mentionTextInputInsertion(doc, spacePos, spacePos, "x", false),
null,
);
assert.deepEqual(
mentionTextInputInsertion(doc, spacePos, spacePos, "x", true),
{ insertAt: spacePos + 1, text: "x" },
);
});
@@ -378,3 +423,85 @@ test("typing after a completed mention keeps the separator intact", () => {
const typed = typeAt(state, 1 + "@quinn world".length, "!");
assert.equal(typed.doc.textContent, "@quinn world!");
});
// ── the browser branch: Chromium's whitespace-run rewrite ─────────────
//
// The helper tests above model the payload. These drive the plugin's real
// `handleTextInput` prop and fall back to ProseMirror's default insertion
// when it declines, exactly as the browser does with the return value — so
// the assertion is sensitive to the production branch itself rather than to
// winning a timing race in a headless browser.
/** Apply an autocomplete pick: replace the typed token and settle the caret. */
function pickMentionAt(state, tokenFrom, tokenTo, inserted) {
const tr = state.tr.insertText(inserted, tokenFrom, tokenTo);
tr.setSelection(TextSelection.create(tr.doc, tokenFrom + inserted.length));
tr.setMeta(mentionHighlightKey, true);
return state.apply(tr);
}
/** Route a text-input event through the plugin, then the default handling. */
function textInput(state, from, to, text) {
let current = state;
const view = {
get state() {
return current;
},
dispatch(tr) {
current = current.apply(tr);
},
domAtPos: () => ({ node: {}, offset: 0 }),
root: undefined,
};
const handled = current.plugins.some(
(plugin) => plugin.props?.handleTextInput?.(view, from, to, text) === true,
);
return handled
? current
: current.apply(current.tr.insertText(text, from, to));
}
test("a whitespace-run rewrite after a mention pick keeps the draft space", () => {
// The CI failure: with "hello world" drafted, the caret placed after
// "hello", " @bo" typed and the "bob" suggestion picked, the document is
// "hello @bob world" — the mention's trailing space followed by the
// draft's own space. The next keystroke arrives as one of these shapes
// depending on how Chromium reconciles that whitespace run, and all of
// them have to keep both spaces.
const tokenFrom = 1 + "hello ".length;
const spacePos = 1 + "hello @bob".length;
const shapes = [
{ name: "caret at the chip edge", from: spacePos, to: spacePos, text: "a" },
{
name: "caret past the trailing space",
from: spacePos + 1,
to: spacePos + 1,
text: "a",
},
{
name: "run rewritten from the chip edge",
from: spacePos,
to: spacePos + 2,
text: "\u00A0a",
},
{
name: "draft space rewritten on its own",
from: spacePos + 1,
to: spacePos + 2,
text: "\u00A0a",
},
];
for (const shape of shapes) {
// A fresh editor per shape: settlement is per-plugin closure state.
const picked = pickMentionAt(
editorStateWithMentionHighlight("hello @bo world", ["bob"]),
tokenFrom,
tokenFrom + "@bo".length,
"@bob ",
);
assert.equal(picked.doc.textContent, "hello @bob world", shape.name);
const typed = textInput(picked, shape.from, shape.to, shape.text);
assert.equal(typed.doc.textContent, "hello @bob a world", shape.name);
}
});

89
desktop/src/features/messages/lib/mentionHighlightExtension.ts
View File

@@ -58,25 +58,71 @@ export function shouldAdvanceMentionCaret({
return next !== from && settling;
}
export type MentionTextInsertion = {
insertAt: number;
text: string;
};
const SPACE_RUN = /^[ \u00A0]+$/;
const OUTER_SPACES = /^[ \u00A0]+|[ \u00A0]+$/g;
/**
* Where to insert typed text when the caret (or a one-character selection)
* sits on the trailing space after an `@name` / `#channel` token.
* A selected trailing space would otherwise be replaced, producing
* `@bobhello`.
* Position just after the trailing space of the mention token that `pos` is
* adjacent to, or `null` when `pos` is nowhere near one.
*
* `pos` may sit at the token end (before the space) or already past the
* space: when Chromium rewrites the whitespace run around the caret it
* anchors the replacement at either edge, and both mean the same boundary.
*/
export function insertPosForMentionTextInput(
function mentionTrailingSpaceBoundary(
doc: ProseMirrorNode,
pos: number,
): number | null {
const afterSpace = selectionAfterMentionTrailingSpace(doc, pos);
if (afterSpace !== pos) return afterSpace;
if (pos > 0 && selectionAfterMentionTrailingSpace(doc, pos - 1) === pos) {
return pos;
}
return null;
}
/**
* Where (and what) to insert when typed text arrives at the trailing space
* after an `@name` / `#channel` token.
*
* - Caret on the space: insert after it, so the next keystroke lands after
* the token (`@bobhello` fix).
* - Whitespace replaced next to that space: keep every space the document
* already has and insert only the typed characters after the token's
* trailing space.
*
* The second rule matters because typing between the mention's trailing
* space and a pre-existing draft space makes Chromium re-emit the whole
* whitespace run — `replace(" " -> " a")`, usually with a non-breaking
* space, and anchored at either edge of the run. Applying any of those
* verbatim deletes the draft's space (`hello @bob abcworld`).
*
* Only whitespace is ever redirected, and only while autocomplete is
* settling — a window in which the user cannot have selected anything,
* because a selection cancels settlement. So a replacement arriving here
* is the browser normalizing whitespace, never an intentional delete, and
* preserving the document's spaces is the whole invariant. Recognizing one
* specific rewrite shape instead is what left the draft space exposed.
*/
export function insertionForMentionTextInput(
doc: ProseMirrorNode,
from: number,
to: number,
): number | null {
const next = selectionAfterMentionTrailingSpace(doc, from);
text: string,
): MentionTextInsertion | null {
if (from === to) {
return next === from ? null : next;
const next = selectionAfterMentionTrailingSpace(doc, from);
return next === from ? null : { insertAt: next, text };
}
if (to === next && next === from + 1) {
return next;
}
return null;
const boundary = mentionTrailingSpaceBoundary(doc, from);
if (boundary === null) return null;
if (!SPACE_RUN.test(doc.textBetween(from, to, "\n", "\0"))) return null;
return { insertAt: boundary, text: text.replace(OUTER_SPACES, "") };
}
/**
@@ -84,14 +130,15 @@ export function insertPosForMentionTextInput(
* deliberate ArrowLeft or chip click, honor the caret so `x` lands in the
* token (`@bobx`) instead of after the space (`@bob x`).
*/
export function mentionTextInputInsertPos(
export function mentionTextInputInsertion(
doc: ProseMirrorNode,
from: number,
to: number,
text: string,
settling: boolean,
): number | null {
): MentionTextInsertion | null {
if (!settling) return null;
return insertPosForMentionTextInput(doc, from, to);
return insertionForMentionTextInput(doc, from, to, text);
}
/** Caret just after a mention trailing space: ArrowLeft lands on the token end. */
@@ -396,18 +443,22 @@ export const MentionHighlightExtension = Extension.create({
return this.getState(state) ?? DecorationSet.empty;
},
handleTextInput(view, from, to, text) {
const insertAt = mentionTextInputInsertPos(
const insertion = mentionTextInputInsertion(
view.state.doc,
from,
to,
text,
settlement.peek() !== null,
);
if (insertAt == null) {
if (insertion == null) {
settlement.cancel();
return false;
}
const tr = view.state.tr.insertText(text, insertAt);
const caret = tr.mapping.map(insertAt, 1);
const tr = view.state.tr.insertText(
insertion.text,
insertion.insertAt,
);
const caret = tr.mapping.map(insertion.insertAt, 1);
tr.setSelection(TextSelection.create(tr.doc, caret));
view.dispatch(tr);
settlement.cancel();