diff --git a/packages/ui/markdown/Markdown.tsx b/packages/ui/markdown/Markdown.tsx index 70cd40899..cff5c923f 100644 --- a/packages/ui/markdown/Markdown.tsx +++ b/packages/ui/markdown/Markdown.tsx @@ -12,7 +12,6 @@ import { CODE_LIGATURE_CLASS } from '@multica/ui/lib/code-style' import { CodeBlock, InlineCode } from './CodeBlock' import { isAllowedFileCardHref, preprocessFileCards } from './file-cards' import { preprocessLinks } from './linkify' -import { remarkCjkAutolink } from './remark-cjk-autolink' import { preprocessMentionShortcodes } from './mentions' import 'katex/dist/katex.min.css' import './markdown.css' @@ -450,15 +449,11 @@ export function Markdown({ [mode, onUrlClick, onFileClick, renderMention, renderImage, renderFileCard] ) - // Preprocess: convert mention shortcodes, file paths, and file cards to - // renderable content. URLs are intentionally left bare (urls: false) so - // remark-gfm autolinks them in the parse tree — a bare URL can then no longer - // swallow an adjacent markdown delimiter like a closing `**` (MUL-4242). - // remarkCjkAutolink re-applies the CJK boundary on gfm's autolink nodes. + // Preprocess: convert mention shortcodes, raw URLs, and file cards to renderable content const processedContent = React.useMemo( () => { let result = preprocessMentionShortcodes(children) - result = preprocessLinks(result, { urls: false }) + result = preprocessLinks(result) result = preprocessFileCards(result, cdnDomain ?? '') return result }, @@ -472,7 +467,6 @@ export function Markdown({ [remarkMath, { singleDollarTextMath: false }], remarkBreaks, [remarkGfm, { singleTilde: false }], - remarkCjkAutolink, ]} rehypePlugins={[rehypeRaw, [rehypeSanitize, sanitizeSchema], rehypeKatex]} urlTransform={urlTransform} diff --git a/packages/ui/markdown/index.ts b/packages/ui/markdown/index.ts index 98ed2afba..d11121fb2 100644 --- a/packages/ui/markdown/index.ts +++ b/packages/ui/markdown/index.ts @@ -1,8 +1,7 @@ export { Markdown, MemoizedMarkdown, type MarkdownProps, type RenderMode } from './Markdown' export { CodeBlock, InlineCode, type CodeBlockProps } from './CodeBlock' export { StreamingMarkdown, type StreamingMarkdownProps } from './StreamingMarkdown' -export { preprocessLinks, detectLinks, hasLinks, CJK_URL_TERMINATOR_REGEX } from './linkify' -export { remarkCjkAutolink } from './remark-cjk-autolink' +export { preprocessLinks, detectLinks, hasLinks } from './linkify' export { preprocessMentionShortcodes } from './mentions' export { preprocessFileCards, diff --git a/packages/ui/markdown/linkify.ts b/packages/ui/markdown/linkify.ts index 7fee366cb..c62bae627 100644 --- a/packages/ui/markdown/linkify.ts +++ b/packages/ui/markdown/linkify.ts @@ -35,12 +35,18 @@ const BARE_FILENAME_REGEX = new RegExp(`^[\\w.-]+\\.(?:${FILE_EXTENSIONS})$`, 'i // character up to the next whitespace swallowed into the href. We truncate the // detected URL at the first occurrence of any of these characters. Character // set mirrors the fix applied in mattermost/marked#22. -// -// Exported so the read-only render pipeline can apply the same boundary to URLs -// that remark-gfm autolinks in the parse tree (see remark-cjk-autolink.ts). -export const CJK_URL_TERMINATOR_REGEX = +const CJK_URL_TERMINATOR_REGEX = /[！-／：-＠［-｀｛-～、。「-】]/ +// Markdown inline-formatting delimiters that linkify-it counts as URL characters +// but which, at the very end of a bare URL, are almost always the author closing +// an emphasis / strikethrough span (`**url**`, `*url*`, `~~url~~`) rather than +// part of the URL. We drop a trailing run of them so the surrounding markdown +// still parses — mirroring GFM's own autolink trailing-punctuation trim. A URL +// that genuinely ends in `*` / `~` loses that character from the link, exactly +// as it does on GitHub (MUL-4242). +const TRAILING_MD_DELIMITER = /[*~]+$/ + interface DetectedLink { type: 'url' | 'email' | 'file' text: string @@ -226,9 +232,11 @@ function rangesOverlap( /** * Run linkify-it on `text` and push normalized link records into `out`, - * shifted by `offset`. When linkify-it merges multiple URLs into one match - * because they are separated only by CJK punctuation (which it doesn't treat - * as a URL boundary), we truncate at that punctuation and re-scan the tail. + * shifted by `offset`. Two boundary corrections linkify-it doesn't make itself: + * - CJK punctuation ends a URL (linkify-it only breaks on ASCII); when it merged + * several URLs across CJK punctuation we truncate and re-scan the tail; and + * - a trailing run of markdown delimiters (`*`, `~`) is dropped from the URL so + * `**url**` keeps its closing emphasis marker outside the link. */ function collectLinkifyMatches(text: string, offset: number, out: DetectedLink[]): void { const matches = linkify.match(text) @@ -239,33 +247,40 @@ function collectLinkifyMatches(text: string, offset: number, out: DetectedLink[] if (cjkIdx === 0) continue // match starts with CJK punct — skip const truncate = cjkIdx > 0 - const matchText = truncate ? match.text.slice(0, cjkIdx) : match.text - const matchEnd = truncate ? match.index + cjkIdx : match.lastIndex + // URL text up to the first CJK terminator, then minus a trailing markdown + // delimiter run (e.g. the closing `**` of `**url**`). matchText stays a + // prefix of match.text, so the link end is just its length past match.index. + const matchText = (truncate ? match.text.slice(0, cjkIdx) : match.text).replace( + TRAILING_MD_DELIMITER, + '' + ) // Bare filenames such as "plan.md" or "README.md" are fuzzy-matched as // domains because their extension is also a valid TLD. They are file // references, not URLs — leave them as plain text rather than link to a // dead external site. Only schemeless (fuzzy) matches are suppressed; an // explicit "https://plan.md" the author typed is still honored. - if (!(match.schema === '' && BARE_FILENAME_REGEX.test(matchText))) { - // linkify-it may prepend a scheme (e.g. "http://" or "mailto:") to url - // while leaving text as the raw substring. Preserve that prefix. + if (matchText.length > 0 && !(match.schema === '' && BARE_FILENAME_REGEX.test(matchText))) { + // When we trimmed the raw match, rebuild the href from the scheme prefix + // linkify-it added (e.g. "http://" / "mailto:") plus the trimmed text; + // otherwise keep linkify-it's normalized url as-is. + const trimmed = matchText.length !== match.text.length const schemePrefix = match.url.slice(0, match.url.length - match.text.length) - const matchUrl = truncate ? schemePrefix + matchText : match.url + const matchUrl = trimmed ? schemePrefix + matchText : match.url out.push({ type: match.schema === 'mailto:' ? 'email' : 'url', text: matchText, url: matchUrl, start: match.index + offset, - end: matchEnd + offset + end: match.index + matchText.length + offset }) } if (truncate) { // Rescan the tail after the CJK punct — linkify-it had greedily swallowed // it, so any additional URLs after the punct were never emitted. - const tailStart = matchEnd + 1 + const tailStart = match.index + cjkIdx + 1 collectLinkifyMatches(text.slice(tailStart), offset + tailStart, out) return } @@ -274,17 +289,12 @@ function collectLinkifyMatches(text: string, offset: number, out: DetectedLink[] /** * Detect all links (URLs, emails, file paths) in text. - * - * `includeUrls` gates the URL/email pass. Read-only markdown renderers pass - * `false` and let remark-gfm autolink URLs in the parse tree instead, which - * cannot corrupt adjacent markdown (e.g. a trailing `**`). File paths, which - * remark-gfm does not linkify, are always detected. See preprocessLinks. */ -export function detectLinks(text: string, includeUrls = true): DetectedLink[] { +export function detectLinks(text: string): DetectedLink[] { const links: DetectedLink[] = [] - // 1. Detect URLs and emails with linkify-it, applying CJK boundary handling. - if (includeUrls) collectLinkifyMatches(text, 0, links) + // 1. Detect URLs and emails with linkify-it, applying boundary corrections. + collectLinkifyMatches(text, 0, links) // 2. Detect file paths with custom regex // Reset regex state @@ -321,17 +331,12 @@ export function detectLinks(text: string, includeUrls = true): DetectedLink[] { * Preprocess text to convert raw URLs and file paths into markdown links. * Skips code blocks and already-linked content. * - * `opts.urls` (default `true`) controls the URL/email pass. The Tiptap editor - * keeps it on because @tiptap/markdown does not autolink bare URLs. Read-only - * react-markdown renderers pass `false`: they let remark-gfm autolink URLs in - * the parse tree, where a bare URL can no longer swallow an adjacent markdown - * delimiter — the string pass here can't tell `https://x**` (URL + bold close) - * from a URL that legitimately ends in `*`, so it corrupted both (MUL-4242). - * File paths (which remark-gfm never linkifies) are converted in both modes. + * Shared by the Tiptap editor and the read-only react-markdown renderers, so + * both surfaces linkify identically. Trailing markdown delimiters are excluded + * from the URL (see collectLinkifyMatches), which keeps `**url**` bold and its + * link clean (MUL-4242). */ -export function preprocessLinks(text: string, opts?: { urls?: boolean }): string { - const includeUrls = opts?.urls ?? true - +export function preprocessLinks(text: string): string { // Quick check - if no potential links, return early if (!linkify.pretest(text) && !/[~/.]\//.test(text)) { return text @@ -339,7 +344,7 @@ export function preprocessLinks(text: string, opts?: { urls?: boolean }): string const codeRanges = findCodeRanges(text) const markdownLinkRanges = findMarkdownLinkRanges(text) - const links = detectLinks(text, includeUrls) + const links = detectLinks(text) if (links.length === 0) return text diff --git a/packages/ui/markdown/remark-cjk-autolink.ts b/packages/ui/markdown/remark-cjk-autolink.ts deleted file mode 100644 index 5e54a3cee..000000000 --- a/packages/ui/markdown/remark-cjk-autolink.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { CJK_URL_TERMINATOR_REGEX, detectLinks } from './linkify' - -/** - * remark-cjk-autolink — trim CJK punctuation that remark-gfm's autolink literal - * swallowed into a URL. - * - * Read-only renderers let remark-gfm autolink bare URLs in the parse tree, so an - * adjacent markdown delimiter (e.g. a closing `**`) is never absorbed into the - * href — that was MUL-4242. gfm's autolink literal, however, shares linkify-it's - * CJK weakness: `https://x/a。后面` extends the link across the ideographic full - * stop and the run after it, and `url1、url2` gets glued into one link. - * preprocessLinks used to trim this before parsing; since URLs are no longer - * preprocessed in read-only mode, we re-derive the real segments on the parsed - * tree with the same CJK-aware detector, which rescans the tail so every URL in - * a CJK-separated run stays linked. - * - * Only autolink *literals* are touched — links whose href was derived from the - * visible text (`https://…`, `www.…` → `http://…`, `a@b` → `mailto:a@b`). - * Explicit `[label](url)` links keep whatever destination the author wrote, even - * when it contains CJK punctuation. - */ - -interface MdNode { - type: string - url?: string - value?: string - children?: MdNode[] -} - -// The scheme prefix remark-gfm prepends to an autolink literal's href. Returns -// null when `url` was not derived from `text`, i.e. an explicit link — leave it. -function autolinkSchemePrefix(url: string, text: string): string | null { - if (url === text) return '' - for (const prefix of ['http://', 'https://', 'mailto:']) { - if (url === prefix + text) return prefix - } - return null -} - -// If `node` is an autolink literal whose text ran past a CJK terminator, rebuild -// it: gfm glued everything up to the next whitespace into one link, so re-derive -// the real segments and return the [link, text, link, …] sequence to splice in. -// Returns null (leave the node alone) when it is not an autolink literal or its -// boundary was already correct. -function splitCjkAutolink(node: MdNode): MdNode[] | null { - if (node.type !== 'link' || !node.url || node.children?.length !== 1) return null - const child = node.children[0] - if (!child || child.type !== 'text' || typeof child.value !== 'string') return null - - const text = child.value - if (autolinkSchemePrefix(node.url, text) === null) return null // not an autolink literal - if (!CJK_URL_TERMINATOR_REGEX.test(text)) return null // gfm's boundary was already correct - - // detectLinks reuses collectLinkifyMatches, which truncates each URL at the - // first CJK terminator AND rescans the tail — so multiple URLs separated by - // CJK punctuation (`url1、url2`) each come back as their own segment instead - // of only the first staying linked. - const links = detectLinks(text, true).filter((link) => link.type !== 'file') - if (links.length === 0) return null - - const out: MdNode[] = [] - let pos = 0 - for (const link of links) { - if (link.start > pos) out.push({ type: 'text', value: text.slice(pos, link.start) }) - out.push({ ...node, url: link.url, children: [{ type: 'text', value: link.text }] }) - pos = link.end - } - if (pos < text.length) out.push({ type: 'text', value: text.slice(pos) }) - return out -} - -function transform(node: MdNode): void { - const children = node.children - if (!children) return - for (let i = 0; i < children.length; i++) { - const split = splitCjkAutolink(children[i]!) - if (split) { - children.splice(i, 1, ...split) - i += split.length - 1 // skip the appended trailing-text node - } else { - transform(children[i]!) - } - } -} - -/** unified/remark plugin. Attach after remark-gfm. */ -export function remarkCjkAutolink() { - return (tree: unknown): void => { - transform(tree as MdNode) - } -} diff --git a/packages/views/editor/readonly-content.test.tsx b/packages/views/editor/readonly-content.test.tsx index f9fd0c7de..da3993961 100644 --- a/packages/views/editor/readonly-content.test.tsx +++ b/packages/views/editor/readonly-content.test.tsx @@ -618,11 +618,11 @@ describe("ReadonlyContent slash command rendering", () => { }); describe("ReadonlyContent bare URL autolinking (MUL-4242)", () => { - // A bare URL wrapped in bold used to be pre-linkified into [url**](url**), - // which swallowed the closing `**`: the bold never closed (leading `**` - // showed as literal asterisks) and the href was corrupted with a trailing - // `**`. URLs are now autolinked by remark-gfm in the parse tree — after - // emphasis is resolved — so an adjacent delimiter can no longer be absorbed. + // A bare URL wrapped in bold used to be linkified into [url**](url**), which + // swallowed the closing `**`: the bold never closed (leading `**` showed as + // literal asterisks) and the href was corrupted with a trailing `**`. The + // shared linkify now drops a trailing markdown-delimiter run from the URL, so + // the closing `**` stays as emphasis outside a clean [url](url). it("renders a bold-wrapped bare URL as bold plus a clean link", () => { const url = "https://github.com/multica-ai/multica/pull/5081"; const { container } = render(); @@ -636,6 +636,21 @@ describe("ReadonlyContent bare URL autolinking (MUL-4242)", () => { expect(anchor?.getAttribute("href")).not.toContain("*"); }); + it("bolds a bare URL even when a CJK punctuation immediately follows (variant B)", () => { + // `**url**（MUL）` — the closing `**` is glued to a fullwidth paren. gfm + // autolink swallowed the `**` here; the shared string linkify does not. + const url = "https://github.com/multica-ai/multica/pull/5133"; + const { container } = render( + , + ); + + const strong = container.querySelector("strong"); + expect(strong).not.toBeNull(); + expect(strong!.querySelector("a")?.getAttribute("href")).toBe(url); + expect(container.textContent).not.toContain("**"); + expect(container.textContent).toContain("（MUL-4277）"); + }); + it("still autolinks a plain bare URL", () => { const { container } = render( , @@ -655,8 +670,8 @@ describe("ReadonlyContent bare URL autolinking (MUL-4242)", () => { }); it("keeps every URL in a CJK-separated run linked, not just the first", () => { - // Regression: gfm glues `url1、url2` into one autolink; trimming only at the - // first 、 left url2 as plain text. The tail must be rescanned so both link. + // `url1、url2` — linkify-it merges both across the CJK comma; collectLinkify + // truncates at 、 and rescans the tail so both URLs become their own link. const { container } = render( , ); diff --git a/packages/views/editor/readonly-content.tsx b/packages/views/editor/readonly-content.tsx index d3b5e8558..8952e9959 100644 --- a/packages/views/editor/readonly-content.tsx +++ b/packages/views/editor/readonly-content.tsx @@ -41,7 +41,7 @@ import { IssueMentionCard } from "../issues/components/issue-mention-card"; import { ProjectChip } from "../projects/components/project-chip"; import { useLinkHover, LinkHoverCard } from "./link-hover-card"; import { openLink, isMentionHref } from "./utils/link-handler"; -import { isAllowedFileCardHref, remarkCjkAutolink } from "@multica/ui/markdown"; +import { isAllowedFileCardHref } from "@multica/ui/markdown"; import { preprocessMarkdown } from "./utils/preprocess"; import { highlightToHtml } from "./utils/highlight-markdown"; import { MermaidDiagram } from "./mermaid-diagram"; @@ -432,11 +432,8 @@ export const ReadonlyContent = memo(function ReadonlyContent({ className, attachments, }: ReadonlyContentProps) { - // urls: false — let remark-gfm autolink bare URLs in the parse tree so an - // adjacent markdown delimiter (e.g. a closing `**`) is never swallowed into - // the href (MUL-4242). remarkCjkAutolink below re-applies the CJK boundary. const processed = useMemo( - () => highlightToHtml(preprocessMarkdown(content, { urls: false })), + () => highlightToHtml(preprocessMarkdown(content)), [content], ); const wrapperRef = useRef(null); @@ -461,7 +458,6 @@ export const ReadonlyContent = memo(function ReadonlyContent({ [remarkMath, { singleDollarTextMath: false }], remarkBreaks, [remarkGfm, { singleTilde: false }], - remarkCjkAutolink, ]} rehypePlugins={[rehypeRaw, [rehypeSanitize, sanitizeSchema], rehypeKatex]} urlTransform={urlTransform} diff --git a/packages/views/editor/utils/preprocess-links.test.ts b/packages/views/editor/utils/preprocess-links.test.ts index 376d58678..db9a62623 100644 --- a/packages/views/editor/utils/preprocess-links.test.ts +++ b/packages/views/editor/utils/preprocess-links.test.ts @@ -137,32 +137,44 @@ describe("preprocessLinks — bare filenames are not auto-linked as URLs", () => }); }); -// Read-only react-markdown renderers pass { urls: false } and let remark-gfm -// autolink URLs in the parse tree instead, so a bare URL can no longer swallow -// an adjacent markdown delimiter like a closing ** (MUL-4242). File paths, which -// remark-gfm never linkifies, are still converted. -describe("preprocessLinks — urls:false (read-only mode)", () => { - it("leaves bare URLs untouched so remark-gfm can autolink them", () => { - expect(preprocessLinks("see https://example.com/x here", { urls: false })).toBe( - "see https://example.com/x here", +// Trailing markdown emphasis / strikethrough delimiters that linkify-it counts +// as URL characters must be dropped from the URL, so the closing `**` of +// `**url**` stays as emphasis instead of being swallowed into the href — that +// swallow was the MUL-4242 render bug. Mirrors GFM's own autolink trailing trim. +describe("preprocessLinks — trailing markdown delimiter is not part of the URL", () => { + it("keeps the closing ** outside a bold-wrapped bare URL", () => { + expect(preprocessLinks("**https://example.com/x**")).toBe( + "**[https://example.com/x](https://example.com/x)**", ); }); - it("does not rewrite a bold-wrapped URL into [url**](url**) (the root cause)", () => { - expect(preprocessLinks("**PR：https://example.com/x**", { urls: false })).toBe( - "**PR：https://example.com/x**", + it("handles the original bug shape (bold + fullwidth colon)", () => { + expect(preprocessLinks("**PR：https://example.com/x**")).toBe( + "**PR：[https://example.com/x](https://example.com/x)**", ); }); - it("still linkifies explicit ./ file paths", () => { - expect(preprocessLinks("see ./src/main.go here", { urls: false })).toBe( - "see [./src/main.go](./src/main.go) here", + it("keeps ** outside when a CJK punctuation immediately follows (variant B)", () => { + expect(preprocessLinks("**https://example.com/x**（MUL-4277）")).toBe( + "**[https://example.com/x](https://example.com/x)**（MUL-4277）", ); }); - it("default mode still linkifies URLs (editor path unchanged)", () => { - expect(preprocessLinks("see https://example.com/x here")).toBe( - "see [https://example.com/x](https://example.com/x) here", + it("keeps the closing * outside an italic-wrapped bare URL", () => { + expect(preprocessLinks("*https://example.com/x*")).toBe( + "*[https://example.com/x](https://example.com/x)*", + ); + }); + + it("strips a trailing * that really ends a URL — documented tradeoff, matches GFM", () => { + expect(preprocessLinks("see https://example.com/glob/* here")).toBe( + "see [https://example.com/glob/](https://example.com/glob/)* here", + ); + }); + + it("keeps a * in the middle of a URL", () => { + expect(preprocessLinks("https://example.com/a*b/c")).toBe( + "[https://example.com/a*b/c](https://example.com/a*b/c)", ); }); }); diff --git a/packages/views/editor/utils/preprocess.ts b/packages/views/editor/utils/preprocess.ts index 01fdcb282..b41391a9c 100644 --- a/packages/views/editor/utils/preprocess.ts +++ b/packages/views/editor/utils/preprocess.ts @@ -15,15 +15,14 @@ import { configStore } from "@multica/core/config"; * 3. File card syntax (new !file[name](url) + legacy [name](cdnUrl)) → HTML div for * fileCard node parsing * - * `opts.urls` (default `true`) forwards to preprocessLinks. The Tiptap editor - * needs it on; read-only react-markdown surfaces pass `false` and let remark-gfm - * autolink URLs in the parse tree instead (MUL-4242). See preprocessLinks. + * Shared by the Tiptap editor and the read-only react-markdown renderer so both + * linkify identically. */ -export function preprocessMarkdown(markdown: string, opts?: { urls?: boolean }): string { +export function preprocessMarkdown(markdown: string): string { if (!markdown) return ""; const cdnDomain = configStore.getState().cdnDomain; const step1 = preprocessMentionShortcodes(markdown); - const step2 = preprocessLinks(step1, opts); + const step2 = preprocessLinks(step1); const step3 = preprocessFileCards(step2, cdnDomain); return step3; }