diff --git a/desktop/src/shared/lib/createRemarkPrefixPlugin.test.mjs b/desktop/src/shared/lib/createRemarkPrefixPlugin.test.mjs new file mode 100644 index 00000000000..6712fe70cdf --- /dev/null +++ b/desktop/src/shared/lib/createRemarkPrefixPlugin.test.mjs @@ -0,0 +1,103 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import remarkChannelLinks from "./remarkChannelLinks.ts"; +import remarkMentions from "./remarkMentions.ts"; + +/** + * These run the real plugins over a real mdast tree. The pattern-level tests in + * `mentionPattern.test.mjs` strip the leading-boundary capture group by hand, + * which cannot catch a factory that swallows the boundary character instead of + * emitting it back as text — that only shows up in the tree. + */ + +function paragraph(...children) { + return { type: "root", children: [{ type: "paragraph", children }] }; +} +function text(value) { + return { type: "text", value }; +} +function kids(tree) { + return tree.children[0].children; +} +/** `[type, value]` for each child, so a swallowed space fails the assertion. */ +function shape(tree) { + return kids(tree).map((node) => [node.type, node.value]); +} + +function runMentions(value, names = ["alice", "bob"]) { + const tree = paragraph(text(value)); + remarkMentions({ mentionNames: names })(tree); + return tree; +} + +function runChannels(value, names = ["general"]) { + const tree = paragraph(text(value)); + remarkChannelLinks({ channelNames: names })(tree); + return tree; +} + +test("the boundary character survives as text before the mention", () => { + assert.deepEqual(shape(runMentions("hi @alice")), [ + ["text", "hi "], + ["mention", "@alice"], + ]); +}); + +test("adjacent mentions keep the space between them", () => { + assert.deepEqual(shape(runMentions("@alice @bob")), [ + ["mention", "@alice"], + ["text", " "], + ["mention", "@bob"], + ]); +}); + +test("a mention at the start of the text produces no empty leading node", () => { + assert.deepEqual(shape(runMentions("@alice hi")), [ + ["mention", "@alice"], + ["text", " hi"], + ]); +}); + +test("an opening paren before a mention stays text", () => { + // Team expansions render as `Team (@ana @bo)`, so `(` opens a mention. + assert.deepEqual(shape(runMentions("Team (@alice)")), [ + ["text", "Team ("], + ["mention", "@alice"], + ["text", ")"], + ]); +}); + +test("an email address is left entirely alone", () => { + assert.deepEqual(shape(runMentions("mail bob@alice.dev now")), [ + ["text", "mail bob@alice.dev now"], + ]); +}); + +test("a channel link keeps its preceding text", () => { + assert.deepEqual(shape(runChannels("see #general")), [ + ["text", "see "], + ["channel-link", "#general"], + ]); +}); + +test("an opening paren does not open a channel link", () => { + // The composer's channel highlighter accepts only start-of-text or + // whitespace; rendered messages must not be more permissive, or `(#general)` + // shows no chip while typing and turns into a link once sent. + assert.deepEqual(shape(runChannels("ask in (#general)")), [ + ["text", "ask in (#general)"], + ]); +}); + +test("the generic channel fallback also refuses a mid-word prefix", () => { + assert.deepEqual(shape(runChannels("issue-42#general", [])), [ + ["text", "issue-42#general"], + ]); +}); + +test("inline code is left untouched", () => { + const tree = paragraph({ type: "inlineCode", value: "@alice" }); + remarkMentions({ mentionNames: ["alice"] })(tree); + assert.deepEqual(shape(tree), [["inlineCode", "@alice"]]); +}); diff --git a/desktop/src/shared/lib/createRemarkPrefixPlugin.ts b/desktop/src/shared/lib/createRemarkPrefixPlugin.ts index 8fe1c91d24b..fcc246d186b 100644 --- a/desktop/src/shared/lib/createRemarkPrefixPlugin.ts +++ b/desktop/src/shared/lib/createRemarkPrefixPlugin.ts @@ -15,6 +15,14 @@ type NodeBuilderResult = Node | { node: Node; trailing?: string }; type NodeBuilder = (matchText: string) => NodeBuilderResult; +/** + * `leadGroup` names a capture group holding a leading boundary character that + * the pattern had to consume to assert one (WebKit before Safari 16.4 fails to + * *parse* lookbehind, blanking the whole app, so patterns capture instead). + * Its text is emitted back as plain text and is not part of the built node. + */ +type PrefixPluginOptions = { leadGroup?: number }; + /** * Create a remark plugin that walks the tree, finds regex matches in text * nodes, and replaces each match with a node produced by `buildNode`. @@ -22,17 +30,23 @@ type NodeBuilder = (matchText: string) => NodeBuilderResult; export function createRemarkPrefixPlugin( pattern: RegExp, buildNode: NodeBuilder, + options?: PrefixPluginOptions, ) { + const leadGroup = options?.leadGroup; return ( // biome-ignore lint/suspicious/noExplicitAny: remark tree types are not available tree: any, ) => { - walkChildren(tree, pattern, buildNode); + walkChildren(tree, pattern, buildNode, leadGroup); }; } -// biome-ignore lint/suspicious/noExplicitAny: remark tree types are not available -function walkChildren(node: any, pattern: RegExp, buildNode: NodeBuilder) { +function walkChildren( + node: Node, + pattern: RegExp, + buildNode: NodeBuilder, + leadGroup?: number, +) { if ( !node?.children || !Array.isArray(node.children) || @@ -45,7 +59,7 @@ function walkChildren(node: any, pattern: RegExp, buildNode: NodeBuilder) { const child = node.children[i]; if (child.type === "text") { - const parts = splitByPattern(child.value, pattern, buildNode); + const parts = splitByPattern(child.value, pattern, buildNode, leadGroup); if ( parts.length > 1 || (parts.length === 1 && parts[0].type !== "text") @@ -53,19 +67,23 @@ function walkChildren(node: any, pattern: RegExp, buildNode: NodeBuilder) { node.children.splice(i, 1, ...parts); } } else { - walkChildren(child, pattern, buildNode); + walkChildren(child, pattern, buildNode, leadGroup); } } } -// biome-ignore lint/suspicious/noExplicitAny: remark tree types are not available -function shouldSkipNode(node: any): boolean { +function shouldSkipNode(node: Node): boolean { return ( node.type === "link" || node.type === "code" || node.type === "inlineCode" ); } -function splitByPattern(text: string, pattern: RegExp, buildNode: NodeBuilder) { +function splitByPattern( + text: string, + pattern: RegExp, + buildNode: NodeBuilder, + leadGroup?: number, +) { // Reset lastIndex — the pattern is reused across text nodes with the `g` flag pattern.lastIndex = 0; // biome-ignore lint/suspicious/noExplicitAny: building mdast-compatible nodes @@ -79,11 +97,15 @@ function splitByPattern(text: string, pattern: RegExp, buildNode: NodeBuilder) { break; } - if (match.index > lastIndex) { - parts.push({ type: "text", value: text.slice(lastIndex, match.index) }); + const lead = leadGroup === undefined ? "" : (match[leadGroup] ?? ""); + const matchStart = match.index + lead.length; + if (matchStart > lastIndex) { + parts.push({ type: "text", value: text.slice(lastIndex, matchStart) }); } - const result = normalizeBuildNodeResult(buildNode(match[0])); + const result = normalizeBuildNodeResult( + buildNode(match[0].slice(lead.length)), + ); parts.push(result.node); if (result.trailing) { parts.push({ type: "text", value: result.trailing }); diff --git a/desktop/src/shared/lib/mentionPattern.test.mjs b/desktop/src/shared/lib/mentionPattern.test.mjs new file mode 100644 index 00000000000..dcc608bf34e --- /dev/null +++ b/desktop/src/shared/lib/mentionPattern.test.mjs @@ -0,0 +1,97 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + buildMentionPattern, + buildPrefixPattern, + PREFIX_LEAD_GROUP, +} from "./mentionPattern.ts"; + +/** All matches, with the leading boundary stripped the way the plugin does. */ +function matches(pattern, text) { + pattern.lastIndex = 0; + const found = []; + let match = pattern.exec(text); + while (match) { + const lead = match[PREFIX_LEAD_GROUP] ?? ""; + found.push({ + text: match[0].slice(lead.length), + index: match.index + lead.length, + }); + match = pattern.exec(text); + } + return found; +} + +test("a mention is matched at the start of the text", () => { + const pattern = buildMentionPattern(["alice"]); + assert.deepEqual(matches(pattern, "@alice hi"), [ + { text: "@alice", index: 0 }, + ]); +}); + +test("a mention is matched after whitespace and after an opening paren", () => { + const pattern = buildMentionPattern(["alice"]); + assert.deepEqual(matches(pattern, "hi @alice"), [ + { text: "@alice", index: 3 }, + ]); + assert.deepEqual(matches(pattern, "hi (@alice)"), [ + { text: "@alice", index: 4 }, + ]); +}); + +test("an email address is not a mention of its domain", () => { + // "@alice" here is part of bob@alice.dev — the composer's own highlighter + // draws the boundary in the same place, so the rendered message should too. + const pattern = buildMentionPattern(["alice"]); + assert.deepEqual(matches(pattern, "mail me at bob@alice.dev"), []); +}); + +test("the boundary does not eat the space between two adjacent mentions", () => { + const pattern = buildMentionPattern(["alice", "bob"]); + assert.deepEqual(matches(pattern, "@alice @bob"), [ + { text: "@alice", index: 0 }, + { text: "@bob", index: 7 }, + ]); +}); + +test("longest known name still wins over a shorter prefix of it", () => { + const pattern = buildMentionPattern(["ali", "alice"]); + assert.deepEqual(matches(pattern, "hi @alice"), [ + { text: "@alice", index: 3 }, + ]); +}); + +test("unknown names are not matched", () => { + const pattern = buildMentionPattern(["alice"]); + assert.deepEqual(matches(pattern, "hi @carol"), []); +}); + +test("with no known names the mention pattern never matches", () => { + assert.deepEqual(matches(buildMentionPattern([]), "hi @alice"), []); +}); + +test("the generic channel fallback also requires a leading boundary", () => { + const pattern = buildPrefixPattern("#", [], { fallbackToGeneric: true }); + assert.deepEqual(matches(pattern, "see #general"), [ + { text: "#general", index: 4 }, + ]); + assert.deepEqual(matches(pattern, "issue-42#general"), []); +}); + +test("an opening paren opens a mention but not a channel", () => { + // Exactly the split the composer's highlighter draws: `(?<=[\s(])@` versus + // `(?<=\s)#`. Team expansions render as `Team (@ana @bo)`; channels have no + // such form, so `(#general)` is not a channel link. + assert.deepEqual(matches(buildMentionPattern(["alice"]), "(@alice)"), [ + { text: "@alice", index: 1 }, + ]); + assert.deepEqual( + matches(buildPrefixPattern("#", ["general"]), "(#general)"), + [], + ); + assert.deepEqual( + matches(buildPrefixPattern("#", [], { fallbackToGeneric: true }), "(#gen)"), + [], + ); +}); diff --git a/desktop/src/shared/lib/mentionPattern.ts b/desktop/src/shared/lib/mentionPattern.ts index 73d12a7a44b..40aae79244d 100644 --- a/desktop/src/shared/lib/mentionPattern.ts +++ b/desktop/src/shared/lib/mentionPattern.ts @@ -7,6 +7,33 @@ export function escapeRegExp(str: string): string { const NEVER_MATCH = /(?!)/gi; +/** + * Capture index of the leading boundary character in a pattern built by + * {@link buildPrefixPattern} — pass it to `createRemarkPrefixPlugin` as + * `leadGroup` so the character is re-emitted as text rather than swallowed + * into the mention. + */ +export const PREFIX_LEAD_GROUP = 1; + +/** + * A prefix only opens a mention or channel link at the start of the text or + * after whitespace — `bob@alice.dev` is an address, not a mention of `@alice`. + * + * The boundary is a capture group rather than a lookbehind on purpose: WebKit + * before Safari 16.4 fails to parse lookbehind and blanks the whole app + * (#5547). + */ +const LEADING_BOUNDARY = "(^|\\s)"; + +/** + * Mentions additionally open after `(`, because team expansions render as + * `Team (@ana @bo)`. Channels deliberately do not — these two boundaries are + * exactly the ones the composer's highlighter applies to `@` and `#` + * respectively, and the rendered message has to agree with the composer or a + * `(#general)` that shows no chip while typing turns into a link once sent. + */ +const LEADING_BOUNDARY_WITH_PAREN = "(^|[\\s(])"; + /** * Build a regex that matches a given prefix followed by known multi-word names * (longest-first to avoid partial matches). When known names are provided, @@ -19,28 +46,38 @@ const NEVER_MATCH = /(?!)/gi; * - Otherwise returns a never-matching regex, preventing arbitrary `@word` * patterns from being highlighted as valid mentions when no p-tags are * present (used by remarkMentions / buildMentionPattern). + * + * `options.allowOpeningParen` widens the leading boundary to include `(`; see + * {@link LEADING_BOUNDARY_WITH_PAREN}. It is off by default so a new caller + * gets the stricter rule. */ export function buildPrefixPattern( prefix: string, knownNames: string[], - options?: { fallbackToGeneric?: boolean }, + options?: { fallbackToGeneric?: boolean; allowOpeningParen?: boolean }, ): RegExp { const sorted = [...new Set(knownNames)] .filter((name) => name.trim().length > 0) .sort((a, b) => b.length - a.length); const escapedPrefix = escapeRegExp(prefix); + const lead = options?.allowOpeningParen + ? LEADING_BOUNDARY_WITH_PAREN + : LEADING_BOUNDARY; if (sorted.length === 0) { if (options?.fallbackToGeneric) { - return new RegExp(`${escapedPrefix}\\S+`, "gi"); + return new RegExp(`${lead}${escapedPrefix}\\S+`, "gi"); } return NEVER_MATCH; } const nameAlternatives = sorted.map((name) => escapeRegExp(name)).join("|"); const boundary = "(?=[\\s,;.!?:)\\]}]|$)"; - return new RegExp(`${escapedPrefix}(?:${nameAlternatives})${boundary}`, "gi"); + return new RegExp( + `${lead}${escapedPrefix}(?:${nameAlternatives})${boundary}`, + "gi", + ); } /** @@ -50,5 +87,5 @@ export function buildPrefixPattern( * they correspond to an actual p-tagged member. */ export function buildMentionPattern(mentionNames: string[]): RegExp { - return buildPrefixPattern("@", mentionNames); + return buildPrefixPattern("@", mentionNames, { allowOpeningParen: true }); } diff --git a/desktop/src/shared/lib/remarkChannelLinks.ts b/desktop/src/shared/lib/remarkChannelLinks.ts index 008a6854071..52af95ac344 100644 --- a/desktop/src/shared/lib/remarkChannelLinks.ts +++ b/desktop/src/shared/lib/remarkChannelLinks.ts @@ -8,7 +8,7 @@ */ import { createRemarkPrefixPlugin } from "./createRemarkPrefixPlugin"; -import { buildPrefixPattern } from "./mentionPattern"; +import { buildPrefixPattern, PREFIX_LEAD_GROUP } from "./mentionPattern"; type RemarkChannelLinksOptions = { channelNames?: string[]; @@ -21,16 +21,20 @@ export default function remarkChannelLinks( fallbackToGeneric: true, }); - return createRemarkPrefixPlugin(channelPattern, (matchText) => { - const channelName = matchText.slice(1); - return { - type: "channel-link", - value: matchText, - data: { - hName: "channel-link", - hChildren: [{ type: "text", value: matchText }], - channelName, - }, - }; - }); + return createRemarkPrefixPlugin( + channelPattern, + (matchText) => { + const channelName = matchText.slice(1); + return { + type: "channel-link", + value: matchText, + data: { + hName: "channel-link", + hChildren: [{ type: "text", value: matchText }], + channelName, + }, + }; + }, + { leadGroup: PREFIX_LEAD_GROUP }, + ); } diff --git a/desktop/src/shared/lib/remarkMentions.ts b/desktop/src/shared/lib/remarkMentions.ts index 7413df6f35c..ec8773af567 100644 --- a/desktop/src/shared/lib/remarkMentions.ts +++ b/desktop/src/shared/lib/remarkMentions.ts @@ -8,7 +8,7 @@ */ import { createRemarkPrefixPlugin } from "./createRemarkPrefixPlugin"; -import { buildMentionPattern } from "./mentionPattern"; +import { buildMentionPattern, PREFIX_LEAD_GROUP } from "./mentionPattern"; type RemarkMentionsOptions = { mentionNames?: string[]; @@ -17,12 +17,16 @@ type RemarkMentionsOptions = { export default function remarkMentions(options?: RemarkMentionsOptions) { const mentionPattern = buildMentionPattern(options?.mentionNames ?? []); - return createRemarkPrefixPlugin(mentionPattern, (matchText) => ({ - type: "mention", - value: matchText, - data: { - hName: "mention", - hChildren: [{ type: "text", value: matchText }], - }, - })); + return createRemarkPrefixPlugin( + mentionPattern, + (matchText) => ({ + type: "mention", + value: matchText, + data: { + hName: "mention", + hChildren: [{ type: "text", value: matchText }], + }, + }), + { leadGroup: PREFIX_LEAD_GROUP }, + ); }