fix(rich-markdown-editor): reliable image selection + serialization/paste polish (#5590)

* fix(rich-markdown-editor): reliable image selection + resize and broken-image polish - Reactive editability. The editor runs with shouldRerenderOnTransaction:false, so a node view that read editor.isEditable once at render kept a stale value after setEditable() toggled (e.g. an agent stream settling into the doc), leaving a pasted image showing read-only affordances and code blocks stuck on their read-only label until a full refresh. A shared useEditorEditable hook subscribes to the editor's update/transaction events so both node views track editability reactively. - Deterministic click-to-select. A handleClickOn plugin sets the image's NodeSelection on a plain click so selecting never depends on ProseMirror's click-vs-drag arbitration; grab-anywhere drag-reorder is kept, and modified clicks (Cmd/Ctrl to follow a linked badge) fall through. - Resize commits once. The width previews in local state during the drag and commits to the node once on pointer-up (or pointer-cancel), so a resize is a single undo step and an interrupted drag isn't lost. - Broken-image placeholder. A src that fails to load renders as a visible box with its alt text and stays selectable, instead of collapsing to a bare broken-icon. * fix(rich-markdown-editor): keep bare URLs and autolinks bare on serialize The normalizing serializer rewrote a bare URL or <url>/<email> autolink to [url](url) / [a@b.com](mailto:a@b.com) on every save, churning every README's links. postProcessSerializedMarkdown now collapses a link back to its bare form when the visible text already equals the destination (a plain http(s) URL, or an email behind mailto:) — GFM re-autolinks it, so the round-trip is identical with a far quieter diff. Titled links, explicit links, and any link inside a fenced/inline code region are left untouched. Idempotent. * feat(rich-markdown-editor): linkify a selection when a URL is pasted over it Pasting a single URL (or a bare www. host / email) over a non-empty text selection within one block now wraps the selection in a link, keeping the visible text. www. gets https://, an email gets mailto:, and the href is scheme-sanitized (javascript:/data: rejected; mailto: requires a real user@host address). Collapsed carets, cross-block selections, multi-word pastes, node selections, and code contexts fall through to normal paste. * chore(rich-markdown-editor): drop useless String.raw in highlight.ts biome 2.0's noUselessStringRaw flags HIGHLIGHT_BODY — its pattern has no escape sequences, so String.raw is equivalent to a plain template literal (byte-identical value; interpolated into the other String.raw regexes unchanged). Pre-existing on staging; the repo-wide lint gate blocks CI on it.
2026-09-24 15:45:35 +08:00 · 2026-07-10 23:23:42 -07:00
parent 9ee499e9f7
commit 2ba0b5837d
8 changed files with 255 additions and 22 deletions
@@ -15,6 +15,7 @@ import { NodeViewContent, NodeViewWrapper, ReactNodeViewRenderer } from '@tiptap
 import { Check, ChevronDown, Code, Copy, Eye, WrapText } from 'lucide-react'
 import { looksLikeMermaid, MermaidDiagram } from '../mermaid-diagram'
 import { detectLanguage } from './detect-language'
+import { useEditorEditable } from './use-editor-editable'

 const PLAIN = 'plain'
 const MERMAID = 'mermaid'
@@ -59,6 +60,7 @@ function CodeBlockView({ node, updateAttributes, editor, getPos }: ReactNodeView
  const [editingInline, setEditingInline] = useState(false)
  const [peekSource, setPeekSource] = useState(false)
  const { copied, copy } = useCopyToClipboard({ resetMs: 1500 })
+  const editable = useEditorEditable(editor)

  const explicitLanguage = node.attrs.language as string | null
  const text = node.textContent
@@ -68,7 +70,7 @@ function CodeBlockView({ node, updateAttributes, editor, getPos }: ReactNodeView
  // diagram on blur (the Linear/GitHub model). The Show source / Show diagram control drives this by
  // focusing into / blurring the block; read-only uses {@link peekSource} since there is no caret.
  useEffect(() => {
-    if (!isMermaid || !editor.isEditable) {
+    if (!isMermaid || !editable) {
      setEditingInline(false)
      return
    }
@@ -91,9 +93,9 @@ function CodeBlockView({ node, updateAttributes, editor, getPos }: ReactNodeView
      editor.off('focus', sync)
      editor.off('blur', sync)
    }
-  }, [editor, getPos, isMermaid])
+  }, [editor, getPos, isMermaid, editable])

-  const showSource = editor.isEditable ? editingInline : peekSource
+  const showSource = editable ? editingInline : peekSource
  const showDiagram = isMermaid && text.trim().length > 0 && !showSource

  // Skip language detection on the mermaid path — the picker/label never render there.
@@ -104,7 +106,7 @@ function CodeBlockView({ node, updateAttributes, editor, getPos }: ReactNodeView
    'Plain text'

  const toggleSource = () => {
-    if (!editor.isEditable) {
+    if (!editable) {
      setPeekSource((value) => !value)
      return
    }
@@ -144,7 +146,7 @@ function CodeBlockView({ node, updateAttributes, editor, getPos }: ReactNodeView
          </button>
        )}
        {!isMermaid &&
-          (editor.isEditable ? (
+          (editable ? (
            // Editable: a language picker. Read-only: a static label — selecting a language calls
            // updateAttributes, which would mutate a doc that must not change.
            <DropdownMenu onOpenChange={setMenuOpen}>
@@ -179,7 +181,7 @@ function CodeBlockView({ node, updateAttributes, editor, getPos }: ReactNodeView
              {label}
            </span>
          ))}
-        {!isMermaid && editor.isEditable && (
+        {!isMermaid && editable && (
          <button
            type='button'
            aria-label='Toggle line wrap'
@@ -13,7 +13,7 @@ import { Plugin } from '@tiptap/pm/state'
 * (`=(?!=)`) but never `==`, so a highlight over text containing `=` (e.g. `==a=b==`) round-trips while
 * the closing `==` still terminates the run.
 */
-const HIGHLIGHT_BODY = String.raw`(?:[^=]|=(?!=))+?`
+const HIGHLIGHT_BODY = `(?:[^=]|=(?!=))+?`
 const HIGHLIGHT_TOKEN = new RegExp(String.raw`^==(?!\s)(${HIGHLIGHT_BODY})(?<!\s)==`)
 /** Input/paste rule form (anchored on a preceding boundary) so typing `==x==` toggles the mark. */
 const HIGHLIGHT_INPUT = new RegExp(String.raw`(?:^|\s)(==(?!\s)(${HIGHLIGHT_BODY})(?<!\s)==)$`)
@@ -2,10 +2,12 @@ import { useEffect, useRef, useState } from 'react'
 import { cn } from '@sim/emcn'
 import type { JSONContent } from '@tiptap/core'
 import { Image } from '@tiptap/extension-image'
+import { NodeSelection, Plugin } from '@tiptap/pm/state'
 import type { ReactNodeViewProps } from '@tiptap/react'
 import { NodeViewWrapper, ReactNodeViewRenderer } from '@tiptap/react'
 import { useFileContentSource } from '@/hooks/use-file-content-source'
 import { normalizeLinkHref } from './markdown-fidelity'
+import { useEditorEditable } from './use-editor-editable'

 const MIN_WIDTH = 64

@@ -168,7 +170,12 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
  const source = useFileContentSource()
  const imageRef = useRef<HTMLImageElement>(null)
  const dragAbortRef = useRef<AbortController | null>(null)
+  const dragWidthRef = useRef<number | null>(null)
  const [dragging, setDragging] = useState(false)
+  /** Live width during a resize drag; kept out of the doc so the whole resize is one undo step. */
+  const [dragWidth, setDragWidth] = useState<number | null>(null)
+  /** Whether the current src failed to load; reset on src change so a retried/edited src can load. */
+  const [failed, setFailed] = useState(false)
  const attrs = node.attrs as {
    src?: string
    alt?: string
@@ -178,6 +185,7 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
  }

  useEffect(() => () => dragAbortRef.current?.abort(), [])
+  useEffect(() => setFailed(false), [attrs.src])

  const startResize = (event: React.PointerEvent) => {
    event.preventDefault()
@@ -195,23 +203,34 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
      'pointermove',
      (move) => {
        const next = Math.max(MIN_WIDTH, Math.round(startWidth + (move.clientX - startX)))
-        updateAttributes({ width: String(next) })
-      },
-      { signal }
-    )
-    window.addEventListener(
-      'pointerup',
-      () => {
-        setDragging(false)
-        controller.abort()
+        dragWidthRef.current = next
+        setDragWidth(next)
      },
      { signal }
    )
+    const finish = () => {
+      const finalWidth = dragWidthRef.current
+      setDragging(false)
+      setDragWidth(null)
+      dragWidthRef.current = null
+      controller.abort()
+      if (finalWidth !== null) updateAttributes({ width: String(finalWidth) })
+    }
+    window.addEventListener('pointerup', finish, { signal })
+    window.addEventListener('pointercancel', finish, { signal })
  }

-  const widthStyle = attrs.width
-    ? { width: /^\d+$/.test(attrs.width) ? `${attrs.width}px` : attrs.width }
+  const committedWidth = attrs.width
+    ? /^\d+$/.test(attrs.width)
+      ? `${attrs.width}px`
+      : attrs.width
    : undefined
+  const widthStyle =
+    dragWidth !== null
+      ? { width: `${dragWidth}px` }
+      : committedWidth
+        ? { width: committedWidth }
+        : undefined

  // Sanitize the linked-image target before rendering the anchor — a parsed markdown href is
  // untrusted and could be `javascript:`/`data:`; an unsafe value drops the link (image only).
@@ -219,7 +238,7 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN

  // Read-only: no drag-to-reorder and no resize handle — both call updateAttributes / dispatch a move,
  // mutating a doc that must not change. The image still renders (and follows its link on click).
-  const editable = editor.isEditable
+  const editable = useEditorEditable(editor)

  const image = (
    <img
@@ -233,9 +252,13 @@ function ResizableImageView({ node, updateAttributes, selected, editor }: ReactN
      draggable={editable}
      data-drag-handle={editable ? '' : undefined}
      style={widthStyle}
+      onError={() => setFailed(true)}
+      onLoad={() => setFailed(false)}
      className={cn(
        'block max-w-full rounded-lg border border-[var(--border)]',
-        editable && 'cursor-grab'
+        editable && 'cursor-grab',
+        failed &&
+          'min-h-[72px] min-w-[140px] bg-[var(--surface-5)] p-3 text-[var(--text-muted)] text-caption'
      )}
    />
  )
@@ -275,4 +298,28 @@ export const ResizableImage = MarkdownImage.extend({
  addNodeView() {
    return ReactNodeViewRenderer(ResizableImageView)
  },
+  /**
+   * Guarantee a plain click on the image forms a node selection. The image body is also a native drag
+   * source (grab-anywhere reorder), and while prosemirror-view ≥1.32.4 no longer implicitly selects on
+   * drag, the reverse — a click reliably selecting — is not guaranteed for an atom whose body competes
+   * with the drag gesture (see the ProseMirror "Draggable and NodeViews" discussion and TipTap #4526).
+   * Selecting here makes it deterministic while leaving drag-to-reorder intact. Read-only clicks and
+   * modified clicks (Cmd/Ctrl to follow a linked badge, Shift/Alt to extend) fall through to the editor's
+   * `handleClick` / default behavior.
+   */
+  addProseMirrorPlugins() {
+    const nodeName = this.name
+    return [
+      new Plugin({
+        props: {
+          handleClickOn(view, _pos, node, nodePos, event) {
+            if (!view.editable || node.type.name !== nodeName) return false
+            if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return false
+            view.dispatch(view.state.tr.setSelection(NodeSelection.create(view.state.doc, nodePos)))
+            return true
+          },
+        },
+      }),
+    ]
+  },
 })
@@ -8,6 +8,33 @@ const BOM = '\uFEFF'
 const FRONTMATTER_REGEX = /^---\r?\n(?:[\s\S]*?\r?\n)?---[ \t]*(?:\r?\n)*/
 const ESCAPED_CALLOUT_REGEX = /^(\s*>(?:\s*>)*\s*)\\\[!([A-Za-z]+)\\\]/gm

+/**
+ * Alternates a code region (fenced block or inline span \u2014 never rewritten) with an inline link whose
+ * destination has no title and isn't angle-bracketed. The code branch is listed first so a link inside
+ * code is consumed as code and left untouched. The destination stops at `)` / whitespace, so a link
+ * carrying a title (`[x](url "t")`) never matches and is preserved verbatim.
+ */
+const CODE_OR_PLAIN_LINK_REGEX =
+  /(```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]+`)|\[([^\]]+)]\(([^)\s<>]+)\)/g
+const HTTP_URL_REGEX = /^https?:\/\/\S+$/i
+
+/**
+ * Collapses an autolinked destination back to its bare form: our normalizing serializer rewrites a bare
+ * URL or `<url>` autolink to `[url](url)` and a bare email to `[a@b.com](mailto:a@b.com)`, which churns
+ * every README's links into explicit-link syntax on the first save. When the visible text already equals
+ * the destination (a plain `http(s)` URL, or an email behind `mailto:`), GFM re-autolinks the bare form,
+ * so emitting it round-trips identically with a far quieter diff. Links inside code and titled links are
+ * left untouched (see {@link CODE_OR_PLAIN_LINK_REGEX}).
+ */
+function collapseAutolinkedUrls(markdown: string): string {
+  return markdown.replace(CODE_OR_PLAIN_LINK_REGEX, (match, code, text, href) => {
+    if (code) return code
+    if (text === href && HTTP_URL_REGEX.test(href)) return href
+    if (href === `mailto:${text}`) return text
+    return match
+  })
+}
+
 export interface SplitMarkdown {
  /** Out-of-band leading prefix (a BOM and/or the frontmatter block), byte-exact, or `''`. */
  frontmatter: string
@@ -87,5 +114,8 @@ export function normalizeLinkHref(href: string): string {
 * begins with whitespace.
 */
 export function postProcessSerializedMarkdown(markdown: string): string {
-  return markdown.replace(ESCAPED_CALLOUT_REGEX, '$1[!$2]').replace(/\n+$/, '\n')
+  return collapseAutolinkedUrls(markdown.replace(ESCAPED_CALLOUT_REGEX, '$1[!$2]')).replace(
+    /\n+$/,
+    '\n'
+  )
 }
@@ -255,3 +255,63 @@ describe('markdown paste', () => {
    expect(transformHtml(editor, '<script>x<script>y</script>')).toBe('')
  })
 })
+
+describe('linkify a selection on URL paste', () => {
+  function linkify(
+    pasted: string,
+    from = 1,
+    to = 10
+  ): { handled: boolean; href?: string; text: string } {
+    editor = mount()
+    editor.commands.setContent('select me here', { contentType: 'markdown' })
+    editor.commands.setTextSelection({ from, to })
+    const handled = paste(editor, pasted)
+    return {
+      handled,
+      href: JSON.stringify(editor.getJSON()).match(/"href":"([^"]+)"/)?.[1],
+      text: editor.getText(),
+    }
+  }
+
+  it('wraps a non-empty text selection in a link when a URL is pasted (keeping the text)', () => {
+    const r = linkify('https://sim.ai')
+    expect(r.handled).toBe(true)
+    expect(r.href).toBe('https://sim.ai')
+    expect(r.text).toBe('select me here')
+  })
+
+  it('prepends https:// to a bare www host and mailto: to a bare email', () => {
+    expect(linkify('www.sim.ai').href).toBe('https://www.sim.ai')
+    expect(linkify('a@b.com').href).toBe('mailto:a@b.com')
+  })
+
+  it('does not linkify a collapsed caret (empty selection)', () => {
+    const r = linkify('https://sim.ai', 5, 5)
+    expect(r.handled).toBe(false)
+  })
+
+  it('does not linkify a multi-word paste over a selection', () => {
+    expect(linkify('not a url just words').handled).toBe(false)
+  })
+
+  it('does not linkify an unsafe javascript: url', () => {
+    const r = linkify('javascript:alert(1)')
+    expect(r.handled).toBe(false)
+    expect(r.href).toBeUndefined()
+  })
+
+  it('links a real mailto: but not a crafted mailto: payload', () => {
+    expect(linkify('mailto:a@b.com').href).toBe('mailto:a@b.com')
+    const crafted = linkify('mailto:javascript:alert(1)')
+    expect(crafted.handled).toBe(false)
+    expect(crafted.href).toBeUndefined()
+  })
+
+  it('does not linkify a selection spanning multiple blocks', () => {
+    editor = mount()
+    editor.commands.setContent('alpha\n\nbeta', { contentType: 'markdown' })
+    editor.commands.setTextSelection({ from: 3, to: 9 })
+    expect(paste(editor, 'https://sim.ai')).toBe(false)
+    expect(JSON.stringify(editor.getJSON())).not.toContain('"type":"link"')
+  })
+})
@@ -1,7 +1,30 @@
 import { Extension } from '@tiptap/core'
 import { Plugin } from '@tiptap/pm/state'
+import { normalizeLinkHref } from './markdown-fidelity'
 import { parseMarkdownToDoc } from './markdown-parse'

+/**
+ * A single link the paste can wrap a selection in: an http(s) URL, a `mailto:` to a real address, a bare
+ * `www.` host, or a bare email. `mailto:` requires an actual `user@host.tld` payload so a crafted value
+ * like `mailto:javascript:…` (no `@`) never matches and falls through to a normal paste.
+ */
+const HTTP_URL = /^https?:\/\/\S+$/i
+const EMAIL = /^[^\s@]+@[^\s@]+\.[^\s@]+$/
+const MAILTO_URL = /^mailto:[^\s@]+@[^\s@]+\.[^\s@]+$/i
+const BARE_WWW = /^www\.\S+\.\S+$/i
+
+/**
+ * If pasted text is a single link, return the href to wrap a selection in — `www.` gets `https://`, a
+ * bare email gets `mailto:`. Returns null for anything else (a multi-word or non-URL paste falls through
+ * to normal insertion). The caller still runs the result through `normalizeLinkHref` for scheme safety.
+ */
+function pastedLinkHref(text: string): string | null {
+  if (HTTP_URL.test(text) || MAILTO_URL.test(text)) return text
+  if (BARE_WWW.test(text)) return `https://${text}`
+  if (EMAIL.test(text)) return `mailto:${text}`
+  return null
+}
+
 /**
 * Structural markdown — strong signals the plain text is genuinely markdown (a link, image, badge,
 * list, heading, blockquote, fenced block, or GFM table). Our parser round-trips these more faithfully
@@ -143,11 +166,21 @@ export const MarkdownPaste = Extension.create({
      new Plugin({
        props: {
          transformPastedHTML: (html) => stripNonContentHtml(html),
-          handlePaste: (_view, event) => {
+          handlePaste: (view, event) => {
            if (!editor.isEditable) return false
            if (editor.isActive('codeBlock') || editor.isActive('code')) return false
            const text = event.clipboardData?.getData('text/plain')
            if (!text) return false
+            const { selection } = view.state
+            if (
+              !selection.empty &&
+              selection.$from.sameParent(selection.$to) &&
+              selection.$from.parent.inlineContent
+            ) {
+              const href = pastedLinkHref(text.trim())
+              const safeHref = href ? normalizeLinkHref(href) : ''
+              if (safeHref) return editor.commands.setLink({ href: safeHref })
+            }
            const language = parseVscodeLanguage(event.clipboardData?.getData('vscode-editor-data'))
            if (language) {
              return editor.commands.insertContent({
@@ -501,3 +501,37 @@ describe('highlight ==mark==', () => {
    expect(markPresent('a == b == c')).toBe(false)
  })
 })
+
+describe('autolink / bare-URL preservation', () => {
+  it('keeps a bare URL bare instead of rewriting it to [url](url)', () => {
+    expect(roundTrip('visit https://sim.ai today').trim()).toBe('visit https://sim.ai today')
+    expect(roundTrip('both https://a.com and https://b.com').trim()).toBe(
+      'both https://a.com and https://b.com'
+    )
+  })
+
+  it('collapses an angle autolink and a bare email to their bare form', () => {
+    expect(roundTrip('see <https://sim.ai> here').trim()).toBe('see https://sim.ai here')
+    expect(roundTrip('mail <a@b.com> now').trim()).toBe('mail a@b.com now')
+  })
+
+  it('preserves explicit and titled links (only bare autolinks collapse)', () => {
+    expect(roundTrip('[Sim](https://sim.ai)').trim()).toBe('[Sim](https://sim.ai)')
+    expect(roundTrip('[https://a.com](https://b.com)').trim()).toBe(
+      '[https://a.com](https://b.com)'
+    )
+    expect(roundTrip('[text](https://x.com "t")').trim()).toBe('[text](https://x.com "t")')
+  })
+
+  it('never collapses a URL that only appears inside code', () => {
+    expect(roundTrip('```\nvisit https://sim.ai\n```').trim()).toBe(
+      '```\nvisit https://sim.ai\n```'
+    )
+    expect(roundTrip('inline `https://sim.ai` code').trim()).toBe('inline `https://sim.ai` code')
+  })
+
+  it('round-trips a bare URL idempotently', () => {
+    const once = roundTrip('go to https://sim.ai now')
+    expect(roundTrip(once)).toBe(once)
+  })
+})
@@ -0,0 +1,27 @@
+import { useEffect, useState } from 'react'
+import type { Editor } from '@tiptap/react'
+
+/**
+ * Reactively tracks `editor.isEditable` for React node views.
+ *
+ * The editor runs with `shouldRerenderOnTransaction: false`, and `editor.setEditable()` updates the
+ * option + re-applies view state but does NOT change any node-view prop — so a node view that reads
+ * `editor.isEditable` once at render keeps a stale value after the editability toggles (e.g. an agent
+ * stream settling into the doc). That leaves images showing no drag/resize/selection affordances and
+ * code blocks stuck on their read-only label until the node happens to re-render. Subscribing to the
+ * editor's `transaction`/`update` events re-reads the flag so the node view stays in sync.
+ */
+export function useEditorEditable(editor: Editor): boolean {
+  const [editable, setEditable] = useState(editor.isEditable)
+  useEffect(() => {
+    const sync = () => setEditable(editor.isEditable)
+    sync()
+    editor.on('transaction', sync)
+    editor.on('update', sync)
+    return () => {
+      editor.off('transaction', sync)
+      editor.off('update', sync)
+    }
+  }, [editor])
+  return editable
+}