fix(rich-markdown-editor): stop image dupe-uploads on drag/paste; fix link color under bold/italic/strikethrough/code

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts

@@ -1,8 +1,36 @@
 /**
  * @vitest-environment jsdom
  */
-import { describe, expect, it } from 'vitest'
-import { extractImageFiles } from './image-paste'
+import { Editor } from '@tiptap/core'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref'
+import {
+  createPublicFileContentSource,
+  createWorkspaceFileContentSource,
+} from '@/hooks/use-file-content-source'
+import { createMarkdownEditorExtensions } from './editor-extensions'
+import {
+  extractImageFiles,
+  extractImgSrcs,
+  findHostedImageAttrs,
+  hasHostedImageHtml,
+  isInlineRouteSrc,
+  shouldSkipFileUpload,
+} from './image-paste'
+
+// jsdom lacks `elementFromPoint`; the Placeholder extension's viewport tracking calls it on mount.
+beforeEach(() => {
+  vi.stubGlobal(
+    'ResizeObserver',
+    class {
+      observe() {}
+      unobserve() {}
+      disconnect() {}
+    }
+  )
+  Element.prototype.scrollIntoView = vi.fn()
+  document.elementFromPoint = vi.fn(() => null)
+})
 
 function imageFile(name = 'shot.png'): File {
   return new File([''], name, { type: 'image/png' })
@@ -54,3 +82,209 @@ describe('extractImageFiles', () => {
     expect(result).toEqual([])
   })
 })
+
+describe('hasHostedImageHtml', () => {
+  const isHosted = (src: string) => src.startsWith('/api/files/view/')
+
+  it('detects an <img> whose src is recognized as one of our own hosted files', () => {
+    expect(hasHostedImageHtml('<img src="/api/files/view/wf_abc" alt="x">', isHosted)).toBe(true)
+  })
+
+  it('is false when the html has no img, or the img src is not one of ours', () => {
+    expect(hasHostedImageHtml('<p>hello</p>', isHosted)).toBe(false)
+    expect(hasHostedImageHtml('<img src="https://other-site.com/photo.jpg">', isHosted)).toBe(false)
+    expect(hasHostedImageHtml('', isHosted)).toBe(false)
+  })
+
+  it('matches a hosted img among multiple candidates', () => {
+    expect(
+      hasHostedImageHtml(
+        '<img src="https://other-site.com/a.png"><img src="/api/files/view/wf_abc">',
+        isHosted
+      )
+    ).toBe(true)
+  })
+
+  // Regression: the browser doesn't put the node's persisted `attrs.src` (`/api/files/view/...`)
+  // onto the clipboard when a rendered <img> is copied — it puts the actual DOM `src`, which is
+  // `resolveImageSrc`'s REWRITTEN display URL (`/…/files/inline?key=…`/`?fileId=…`). A predicate
+  // that only recognizes the persisted shape (as `extractEmbeddedFileRef` alone does) never matches
+  // a real same-page copy, silently falling through to the re-upload path it exists to avoid.
+  it('recognizes the real rendered <img src> end-to-end, not just the persisted reference shape', () => {
+    const ws = createWorkspaceFileContentSource('ws-1')
+    const renderedFromKey = ws.resolveImageSrc(
+      '/api/files/serve/workspace/ws-1/1700000000000-deadbeefdeadbeef-photo.png'
+    )
+    const renderedFromFileId = ws.resolveImageSrc('/api/files/view/wf_abc')
+    expect(renderedFromKey).toMatch(/^\/api\/workspaces\/ws-1\/files\/inline\?key=/)
+    expect(renderedFromFileId).toBe('/api/workspaces/ws-1/files/inline?fileId=wf_abc')
+
+    // extractEmbeddedFileRef alone (the persisted-content recognizer) does NOT match either
+    // rendered form — that's the exact gap isInlineRouteSrc closes.
+    expect(extractEmbeddedFileRef(renderedFromKey as string)).toBeNull()
+    expect(extractEmbeddedFileRef(renderedFromFileId as string)).toBeNull()
+
+    const isHostedReal = (src: string) => extractEmbeddedFileRef(src) !== null
+    expect(hasHostedImageHtml(`<img src="${renderedFromKey}">`, isHostedReal)).toBe(true)
+    expect(hasHostedImageHtml(`<img src="${renderedFromFileId}">`, isHostedReal)).toBe(true)
+  })
+
+  it('recognizes the public-share inline route too', () => {
+    const pub = createPublicFileContentSource('tok_1', '/api/files/public/tok_1/content')
+    const rendered = pub.resolveImageSrc('/api/files/view/wf_abc')
+    expect(rendered).toBe('/api/files/public/tok_1/inline?fileId=wf_abc')
+    expect(hasHostedImageHtml(`<img src="${rendered}">`, () => false)).toBe(true)
+  })
+
+  it('matches a valid unquoted src attribute (unquoted attribute values are valid HTML)', () => {
+    expect(hasHostedImageHtml('<img src=/api/files/view/wf_abc>', isHosted)).toBe(true)
+    expect(hasHostedImageHtml("<img alt='x' src=/api/files/view/wf_abc alt=y>", isHosted)).toBe(
+      true
+    )
+    expect(hasHostedImageHtml('<img src=https://other-site.com/a.png>', isHosted)).toBe(false)
+  })
+
+  it('matches single-quoted src attributes too', () => {
+    expect(hasHostedImageHtml("<img src='/api/files/view/wf_abc'>", isHosted)).toBe(true)
+  })
+})
+
+describe('extractImgSrcs', () => {
+  it('extracts every img src in document order, including duplicates', () => {
+    expect(
+      extractImgSrcs('<img src="/a.png"><p>text</p><img src="/b.png"><img src="/a.png">')
+    ).toEqual(['/a.png', '/b.png', '/a.png'])
+  })
+
+  it('returns an empty array for html with no img', () => {
+    expect(extractImgSrcs('<p>hello</p>')).toEqual([])
+    expect(extractImgSrcs('')).toEqual([])
+  })
+})
+
+describe('shouldSkipFileUpload (shared by paste and drop)', () => {
+  const isHosted = (src: string) => src.startsWith('/api/files/view/')
+  const hostedHtml = '<img src="/api/files/view/wf_abc">'
+
+  it('skips upload for a single already-hosted image', () => {
+    expect(shouldSkipFileUpload([imageFile()], hostedHtml, isHosted)).toBe(true)
+  })
+
+  it('does not skip when there is no html, or the html is not one of ours', () => {
+    expect(shouldSkipFileUpload([imageFile()], '', isHosted)).toBe(false)
+    expect(shouldSkipFileUpload([imageFile()], '<img src="https://x.com/a.png">', isHosted)).toBe(
+      false
+    )
+  })
+
+  it('does not skip when there are no files to upload in the first place', () => {
+    expect(shouldSkipFileUpload([], hostedHtml, isHosted)).toBe(false)
+  })
+
+  // Regression: a genuinely mixed paste/drop (the hosted image plus a separate new one) must
+  // still upload the new file — bailing out entirely here would silently drop it.
+  it('does not skip a mixed paste/drop carrying more than one image file', () => {
+    expect(
+      shouldSkipFileUpload([imageFile('a.png'), imageFile('b.png')], hostedHtml, isHosted)
+    ).toBe(false)
+  })
+
+  // Regression: this must be content-based (the accompanying html), not keyed off any mutable
+  // per-drag flag like ProseMirror's `view.dragging` — that flag can go briefly stale (cleared up
+  // to ~50ms late via `dragend` when a prior internal drag was dropped outside the view), and a
+  // flag-based check would incorrectly suppress upload of an unrelated new file dropped in that
+  // window. This function only reacts to what THIS specific event's `html`/`images` contain.
+  it('is a pure function of the images/html actually offered, independent of any drag-session flag', () => {
+    expect(shouldSkipFileUpload([imageFile()], '', isHosted)).toBe(false)
+    expect(shouldSkipFileUpload([imageFile()], hostedHtml, isHosted)).toBe(true)
+  })
+})
+
+describe('isInlineRouteSrc', () => {
+  it('recognizes the workspace- and public-scoped inline route with key or fileId', () => {
+    expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline?key=workspace%2Fws-1%2Fa.png')).toBe(
+      true
+    )
+    expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline?fileId=wf_abc')).toBe(true)
+    expect(isInlineRouteSrc('/api/files/public/tok_1/inline?fileId=wf_abc')).toBe(true)
+  })
+
+  it('rejects non-inline paths, unrecognized query params, and external/absolute origins', () => {
+    expect(isInlineRouteSrc('/api/files/serve/workspace/ws-1/a.png')).toBe(false)
+    expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline')).toBe(false)
+    expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline?other=1')).toBe(false)
+    expect(isInlineRouteSrc('https://other-site.com/files/inline?key=x')).toBe(false)
+    expect(isInlineRouteSrc('data:image/png;base64,aaaa')).toBe(false)
+  })
+})
+
+describe('findHostedImageAttrs', () => {
+  const ws = createWorkspaceFileContentSource('ws-1')
+
+  function docWithImages(...attrs: Array<Record<string, unknown>>): Editor {
+    return new Editor({
+      extensions: createMarkdownEditorExtensions({ placeholder: '' }),
+      content: {
+        type: 'doc',
+        content: attrs.map((a) => ({ type: 'image', attrs: a })),
+      },
+    })
+  }
+
+  // Regression: this is the exact mechanism that avoids persisting the display-layer inline URL
+  // (Cursor's "Paste persists display image URLs" finding) — cloning the REAL node's attrs rather
+  // than re-deriving a node from the clipboard html's rewritten src.
+  it('finds the existing node whose RESOLVED (display) src matches, and returns its REAL persisted attrs', () => {
+    const persistedSrc = '/api/files/view/wf_abc'
+    const editor = docWithImages({ src: persistedSrc, alt: 'photo', width: '300' })
+    const renderedSrc = ws.resolveImageSrc(persistedSrc) as string
+    expect(renderedSrc).not.toBe(persistedSrc) // sanity: the rendered form really differs
+
+    const match = findHostedImageAttrs(editor.state.doc, [renderedSrc], ws.resolveImageSrc)
+    expect(match).not.toBeNull()
+    expect(match?.src).toBe(persistedSrc) // the REAL persisted src, not the rendered one
+    expect(match?.alt).toBe('photo')
+    expect(match?.width).toBe('300')
+  })
+
+  it('returns null when no node in the doc resolves to any target src', () => {
+    const editor = docWithImages({ src: '/api/files/view/wf_other' })
+    const match = findHostedImageAttrs(
+      editor.state.doc,
+      ['/api/workspaces/ws-1/files/inline?fileId=wf_abc'],
+      ws.resolveImageSrc
+    )
+    expect(match).toBeNull()
+  })
+
+  it('returns null for an empty doc or an empty target list', () => {
+    const editor = docWithImages()
+    expect(findHostedImageAttrs(editor.state.doc, ['/anything'], ws.resolveImageSrc)).toBeNull()
+    const editorWithImage = docWithImages({ src: '/api/files/view/wf_abc' })
+    expect(findHostedImageAttrs(editorWithImage.state.doc, [], ws.resolveImageSrc)).toBeNull()
+  })
+
+  it('matches the first of several images, not just the last', () => {
+    const editor = docWithImages(
+      { src: '/api/files/view/wf_one', alt: 'one' },
+      { src: '/api/files/view/wf_two', alt: 'two' }
+    )
+    const renderedTwo = ws.resolveImageSrc('/api/files/view/wf_two') as string
+    const match = findHostedImageAttrs(editor.state.doc, [renderedTwo], ws.resolveImageSrc)
+    expect(match?.alt).toBe('two')
+  })
+
+  it('returns a defensive copy, not a live reference to the node attrs object', () => {
+    const persistedSrc = '/api/files/view/wf_abc'
+    const editor = docWithImages({ src: persistedSrc, alt: 'photo' })
+    const renderedSrc = ws.resolveImageSrc(persistedSrc) as string
+    const match = findHostedImageAttrs(editor.state.doc, [renderedSrc], ws.resolveImageSrc)
+    expect(match).not.toBeNull()
+    if (match) match.alt = 'mutated'
+    let originalAlt: unknown
+    editor.state.doc.descendants((node) => {
+      if (node.type.name === 'image') originalAlt = node.attrs.alt
+    })
+    expect(originalAlt).toBe('photo')
+  })
+})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts

@@ -12,3 +12,121 @@ export function extractImageFiles(transfer: DataTransfer | null): File[] {
     .map((item) => item.getAsFile())
     .filter((file): file is File => file !== null)
 }
+
+// `src` may be double-quoted, single-quoted, or (validly) unquoted per the HTML spec — the browser's
+// own clipboard serialization always quotes it, but other producers of `text/html` are not obligated
+// to.
+const IMG_SRC_RE = /<img\b[^>]*\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'>]+))/gi
+const INLINE_ROUTE_QUERY_KEYS = new Set(['key', 'fileId'])
+
+/**
+ * True for the *display-layer* inline route `resolveImageSrc` (see `use-file-content-source.tsx`)
+ * rewrites an embed to — workspace-scoped `/api/workspaces/{workspaceId}/files/inline?key=…`/`?fileId=…`
+ * or public-share-scoped `/api/files/public/{token}/inline?key=…`/`?fileId=…`. This is the shape
+ * actually rendered into `<img src>`, and so what a same-page copy's `text/html` clipboard payload
+ * actually contains — NOT the raw stored reference `extractEmbeddedFileRef` (in
+ * `@/lib/uploads/utils/embedded-image-ref`) recognizes, which only matches the persisted `src` before
+ * that rewrite. Checked separately from (rather than folded into) `extractEmbeddedFileRef` since that
+ * helper is shared with server-side authorization/export code operating on persisted content, where
+ * this display-only shape should never legitimately appear.
+ */
+export function isInlineRouteSrc(src: string): boolean {
+  try {
+    const parsed = new URL(src, 'http://placeholder')
+    if (parsed.origin !== 'http://placeholder') return false
+    if (!parsed.pathname.endsWith('/inline')) return false
+    for (const key of parsed.searchParams.keys()) {
+      if (INLINE_ROUTE_QUERY_KEYS.has(key)) return true
+    }
+    return false
+  } catch {
+    return false
+  }
+}
+
+/**
+ * Extracts every `<img>` `src` value found in `html`, in document order (may contain duplicates).
+ */
+export function extractImgSrcs(html: string): string[] {
+  const srcs: string[] = []
+  for (const match of html.matchAll(IMG_SRC_RE)) {
+    const src = match[1] ?? match[2] ?? match[3]
+    if (src) srcs.push(src)
+  }
+  return srcs
+}
+
+/**
+ * True when `html` contains an `<img>` whose `src` is already one of our own hosted workspace file
+ * references. Copying a rendered `<img>` that's already on the page (e.g. Cmd+C after clicking it to
+ * select it) makes the browser put BOTH `text/html` (the real serialized node, with its real hosted
+ * `src`) AND a synthesized image `File` onto the clipboard — the same "drag a web image out" behavior
+ * that {@link extractImageFiles} alone can't tell apart from a genuinely new external image paste.
+ */
+export function hasHostedImageHtml(html: string, isHostedRef: (src: string) => boolean): boolean {
+  return extractImgSrcs(html).some((src) => isHostedRef(src) || isInlineRouteSrc(src))
+}
+
+/**
+ * True when a paste or drop should be diverted away from the upload-from-file path — it carries
+ * exactly one image file, and the accompanying `text/html` shows it's a same-page copy of an
+ * already-hosted image (see {@link hasHostedImageHtml}) rather than a genuinely new external image.
+ * Content-based (not `view.dragging`-based, for the drop case): `view.dragging` can go briefly stale
+ * (cleared up to ~50ms late by ProseMirror's own `dragend` handler when a prior internal drag was
+ * dropped outside this view) and must never suppress upload of an unrelated, genuinely new file that
+ * happens to land in that window — this check only reacts to what THIS specific event's `html`
+ * actually contains. Gated on exactly one file — a genuinely mixed paste/drop (the hosted image plus a
+ * separate new one) must still upload the new file, not have the whole paste/drop diverted.
+ */
+export function shouldSkipFileUpload(
+  images: File[],
+  html: string,
+  isHostedRef: (src: string) => boolean
+): boolean {
+  return images.length === 1 && Boolean(html) && hasHostedImageHtml(html, isHostedRef)
+}
+
+/** Minimal shape of a ProseMirror image node — just enough to read its type name and attrs. */
+interface ImageLikeNode {
+  type: { name: string }
+  attrs: Record<string, unknown>
+}
+
+/** Minimal shape of a ProseMirror doc — just enough to walk its nodes. */
+interface DescendantsDoc {
+  descendants: (callback: (node: ImageLikeNode) => boolean | undefined) => void
+}
+
+/**
+ * Finds the first `image` node already in `doc` whose *rendered* src (`resolveImageSrc(node.attrs.src)`)
+ * matches one of `targetSrcs`, and returns its attrs — a defensive copy, safe to hand straight to
+ * `insertContentAt`. Returns `null` if no match is found (e.g. the source node was deleted, or this is
+ * genuinely a different document than the one the html was copied from).
+ *
+ * Used to clone a same-page copy/drag of an already-hosted image faithfully — the exact persisted
+ * `src` (and every other attribute: width, height, href, title…) — rather than re-deriving a node from
+ * the clipboard/dataTransfer `html`, whose `src` is `resolveImageSrc`'s rewritten *display* URL, not the
+ * real persisted one. Inserting a node built from that display URL would bake it into the document,
+ * which public share/export/referenced-by-doc tracking don't recognize (they only match the persisted
+ * shape) — this lookup avoids ever constructing such a node in the first place.
+ */
+export function findHostedImageAttrs(
+  doc: DescendantsDoc,
+  targetSrcs: string[],
+  resolveImageSrc: (src: string | undefined) => string | undefined
+): Record<string, unknown> | null {
+  const targets = new Set(targetSrcs)
+  let found: Record<string, unknown> | null = null
+  doc.descendants((node) => {
+    if (found) return false
+    if (node.type.name === 'image') {
+      const resolved = resolveImageSrc(node.attrs.src as string | undefined)
+      if (resolved && targets.has(resolved)) {
+        found = { ...node.attrs }
+        return false
+      }
+    }
+    return true
+  })
+  return found
+}