From eda8567b7394777c71720bed24596f17a700e07e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 10 Jul 2026 14:49:40 -0700 Subject: [PATCH 1/5] fix(rich-markdown-editor): stop drag/paste of an existing image from re-uploading a duplicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dragging an image block to reorder it, or copy-pasting an already-hosted image within the editor, both re-uploaded the image as a brand-new file instead of reusing/moving the existing one: - Dragging an to reorder it is a native HTML5 drag; browsers synthesize an image File into event.dataTransfer for it (the same mechanism that lets you drag a web image to your desktop), indistinguishable from a real external drop by dataTransfer contents alone. Our handleDrop treated that File as a genuinely new image, uploaded it, and inserted a duplicate node — while ProseMirror's own default move logic never got to run, so the original was left behind too. Fixed by checking `view.dragging` (ProseMirror's own signal that a drop follows a dragstart within this same view) and bailing out to let its default move logic run. - The same browser behavior applies to copy-paste: selecting a rendered already on the page and pressing Cmd+C puts BOTH `text/html` (the real node, with its real hosted src) AND a synthesized image File onto the clipboard. Our handlePaste preferred the File, re-uploading and inserting a new node rather than cloning the original (silently dropping width/href/title in the process). Fixed by preferring the HTML sibling — via the existing extractEmbeddedFileRef helper — whenever it already names one of our own hosted files. --- .../rich-markdown-editor/image-paste.test.ts | 25 ++++++++++++++++++- .../rich-markdown-editor/image-paste.ts | 19 ++++++++++++++ .../rich-markdown-editor.tsx | 24 +++++++++++++++++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts index 766e4c77ef6..7d9de1feb77 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts @@ -2,7 +2,7 @@ * @vitest-environment jsdom */ import { describe, expect, it } from 'vitest' -import { extractImageFiles } from './image-paste' +import { extractImageFiles, hasHostedImageHtml } from './image-paste' function imageFile(name = 'shot.png'): File { return new File([''], name, { type: 'image/png' }) @@ -54,3 +54,26 @@ describe('extractImageFiles', () => { expect(result).toEqual([]) }) }) + +describe('hasHostedImageHtml', () => { + const isHosted = (src: string) => src.startsWith('/api/files/view/') + + it('detects an whose src is recognized as one of our own hosted files', () => { + expect(hasHostedImageHtml('x', isHosted)).toBe(true) + }) + + it('is false when the html has no img, or the img src is not one of ours', () => { + expect(hasHostedImageHtml('

hello

', isHosted)).toBe(false) + expect(hasHostedImageHtml('', isHosted)).toBe(false) + expect(hasHostedImageHtml('', isHosted)).toBe(false) + }) + + it('matches a hosted img among multiple candidates', () => { + expect( + hasHostedImageHtml( + '', + isHosted + ) + ).toBe(true) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts index ff72fededf9..fd1a00fd50a 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts @@ -12,3 +12,22 @@ export function extractImageFiles(transfer: DataTransfer | null): File[] { .map((item) => item.getAsFile()) .filter((file): file is File => file !== null) } + +const IMG_SRC_RE = /]*\bsrc\s*=\s*["']([^"']+)["']/gi + +/** + * True when `html` contains an `` whose `src` is already one of our own hosted workspace file + * references. Copying a rendered `` 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. When + * this is true, the paste handler should let the editor's default HTML-based paste clone the existing + * node (reusing its real `src`, and every other attribute) instead of re-uploading the pasted bytes as + * a brand-new file. + */ +export function hasHostedImageHtml(html: string, isHostedRef: (src: string) => boolean): boolean { + for (const match of html.matchAll(IMG_SRC_RE)) { + if (isHostedRef(match[1])) return true + } + return false +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index 7114210f8ca..31114c33884 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -7,13 +7,14 @@ import type { Editor } from '@tiptap/react' import { EditorContent, useEditor } from '@tiptap/react' import { useRouter } from 'next/navigation' import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' +import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref' import { useUploadWorkspaceFile } from '@/hooks/queries/workspace-files' import type { SaveStatus } from '@/hooks/use-autosave' import { PreviewLoadingFrame } from '../preview-shared' import { useEditableFileContent } from '../use-editable-file-content' import { createMarkdownEditorExtensions } from './editor-extensions' import { findHeadingPos } from './heading-anchors' -import { extractImageFiles } from './image-paste' +import { extractImageFiles, hasHostedImageHtml } from './image-paste' import { applyFrontmatter, normalizeLinkHref, @@ -298,8 +299,21 @@ export function LoadedRichMarkdownEditor({ window.open(normalized, '_blank', 'noopener,noreferrer') return true }, + /** + * Inserts pasted image files at the caret. A same-page copy of an already-hosted `` (e.g. + * Cmd+C after clicking it to select it) makes the browser add BOTH `text/html` (the real node, + * with its real hosted `src`) AND a synthesized image `File` to the clipboard — indistinguishable + * from a genuine external image paste by `clipboardData` files/items alone. When the HTML sibling + * already names one of our own hosted files, bail out and let the editor's default HTML paste + * clone that node (reusing its real `src` and every other attribute) instead of re-uploading the + * pasted bytes as a brand-new, distinct file. + */ handlePaste: (view, event) => { if (!view.editable) return false + const html = event.clipboardData?.getData('text/html') ?? '' + if (html && hasHostedImageHtml(html, (src) => extractEmbeddedFileRef(src) !== null)) { + return false + } const images = extractImageFiles(event.clipboardData) if (images.length === 0) return false event.preventDefault() @@ -310,9 +324,17 @@ export function LoadedRichMarkdownEditor({ * Inserts dropped image files at the drop point. Any other file drop (e.g. a PDF) is swallowed so * the browser doesn't navigate away from the editor; internal text drags carry no files and fall * through to the default behavior. + * + * Dragging an existing image node to reorder it is also an internal drag, but the browser's + * native drag-and-drop synthesizes an image `File` into `event.dataTransfer` for a dragged `` + * (the same mechanism that lets a user drag a web image out to their desktop) — indistinguishable + * from a real external drop by `dataTransfer` contents alone. `view.dragging` is ProseMirror's own + * signal that this drop follows a `dragstart` within this same view, so bail out and let its + * default move logic run instead of re-uploading the dragged image as a duplicate. */ handleDrop: (view, event) => { if (!view.editable) return false + if (view.dragging) return false const images = extractImageFiles(event.dataTransfer) if (images.length > 0) { event.preventDefault() From a80afb6c258209d79995efbc60c8a6c31731c749 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 10 Jul 2026 14:49:51 -0700 Subject: [PATCH 2/5] fix(rich-markdown-editor): fix link color lost under bold/italic/strikethrough/code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit strong/em/del/s/code each set their own explicit `color` for the plain (no-link) case. Nested inside a link, that explicit rule on the mark itself always wins over the color inherited from the ancestor — an inherited value never beats an element's own explicit rule, regardless of how specific the ancestor's selector is. So an italic (or bold/struck-through/inline-code) link rendered in the mark's plain-text color instead of the link's blue. Adds an explicit `.rich-markdown-prose a ` / `.rich-markdown-prose a` override, covering both DOM nesting orders since ProseMirror's mark order (and so which nests outside the other) depends on which was toggled first, not a fixed schema order. Only `color` is touched — each mark's own font-weight/font-style/text-decoration/background composes normally underneath. 24 new tests load the real, shipped CSS into jsdom and assert against getComputedStyle for every mark x both nesting directions x multi-mark stacks, plus regression guards that a mark with no link keeps its own color and a link elsewhere in the doc doesn't bleed color into unrelated text. Verified all 11 color-assertion tests fail against the pre-fix CSS. --- .../link-mark-precedence.test.ts | 165 ++++++++++++++++++ .../rich-markdown-editor.css | 21 +++ 2 files changed, 186 insertions(+) create mode 100644 apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts new file mode 100644 index 00000000000..0eab233305c --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts @@ -0,0 +1,165 @@ +/** + * @vitest-environment jsdom + * + * A mark (bold/italic/strikethrough/inline-code) stacked on top of a link must not steal the link's + * blue: `strong`/`em`/`del`/`s`/`code` each set their own explicit `color` for the no-link case, and an + * element's own explicit rule always wins over an inherited value regardless of how specific the + * ancestor's (`a`'s) selector is — so without an override targeting the mark itself, a bold/italic/ + * struck-through/code link renders in the mark's plain-text color instead of the link color. + * + * These load the real, shipped `rich-markdown-editor.css` (not a copy) and assert against + * `getComputedStyle` in jsdom. jsdom's CSS engine does not resolve `var(...)` references to actual + * color values, but it does correctly resolve the cascade/specificity winner and returns that + * declaration's authored value verbatim — so asserting the winning declaration is `var(--brand-secondary)` + * (vs. e.g. `var(--text-primary)`) is a precise, real assertion about which CSS rule wins, not a proxy. + */ +import { readFileSync } from 'node:fs' +import path from 'node:path' +import { afterEach, beforeAll, describe, expect, it } from 'vitest' + +const CSS_PATH = path.join(__dirname, 'rich-markdown-editor.css') +const LINK_COLOR = 'var(--brand-secondary)' + +beforeAll(() => { + const style = document.createElement('style') + style.textContent = readFileSync(CSS_PATH, 'utf-8') + document.head.appendChild(style) +}) + +let container: HTMLDivElement | null = null + +afterEach(() => { + container?.remove() + container = null +}) + +/** Mounts `html` inside a `.rich-markdown-prose` container (the real editor's root class) and returns it. */ +function mount(html: string): HTMLDivElement { + container = document.createElement('div') + container.className = 'rich-markdown-prose' + container.innerHTML = html + document.body.appendChild(container) + return container +} + +function colorOf(el: Element | null): string { + if (!el) throw new Error('element not found') + return getComputedStyle(el).color +} + +describe('link color baseline (no stacked mark)', () => { + it('a plain link is brand-secondary colored', () => { + const root = mount('link') + expect(colorOf(root.querySelector('a'))).toBe(LINK_COLOR) + }) +}) + +describe('a mark with no link keeps its own color (regression guard: the fix must not force every mark blue)', () => { + it.each([ + { tag: 'strong', html: 'bold', color: 'var(--text-primary)' }, + { tag: 'em', html: 'italic', color: 'var(--text-primary)' }, + { tag: 'del', html: 'struck', color: 'var(--text-tertiary)' }, + { tag: 's', html: 'struck', color: 'var(--text-tertiary)' }, + { tag: 'code', html: 'code', color: 'var(--text-primary)' }, + ])('$tag alone renders $color, not link color', ({ tag, html, color }) => { + const root = mount(html) + expect(colorOf(root.querySelector(tag))).toBe(color) + expect(colorOf(root.querySelector(tag))).not.toBe(LINK_COLOR) + }) +}) + +describe('mark nested INSIDE a link (`text`) — link color wins', () => { + it.each([ + { tag: 'strong', html: 'bold link' }, + { tag: 'em', html: 'italic link' }, + { tag: 'del', html: 'struck link' }, + { tag: 's', html: 'struck link' }, + { tag: 'code', html: 'code link' }, + ])('$tag inside a link is link-colored, not its own default color', ({ tag, html }) => { + const root = mount(html) + expect(colorOf(root.querySelector(tag))).toBe(LINK_COLOR) + }) +}) + +describe('mark WRAPPING a link (`text`) — link color still wins', () => { + it.each([ + { tag: 'strong', html: 'bold link' }, + { tag: 'em', html: 'italic link' }, + { tag: 'del', html: 'struck link' }, + { tag: 's', html: 'struck link' }, + { tag: 'code', html: 'code link' }, + ])('a link inside $tag is link-colored regardless of nesting direction', ({ tag, html }) => { + const root = mount(html) + expect(colorOf(root.querySelector('a'))).toBe(LINK_COLOR) + }) +}) + +describe('each mark keeps its own non-color styling even when link-colored', () => { + it('bold link keeps font-weight 600', () => { + const root = mount('bold link') + const strong = root.querySelector('strong') as HTMLElement + expect(colorOf(strong)).toBe(LINK_COLOR) + expect(getComputedStyle(strong).fontWeight).toBe('600') + }) + + it('italic link keeps font-style italic', () => { + const root = mount('italic link') + const em = root.querySelector('em') as HTMLElement + expect(colorOf(em)).toBe(LINK_COLOR) + expect(getComputedStyle(em).fontStyle).toBe('italic') + }) + + it('strikethrough link keeps text-decoration line-through (del and s)', () => { + for (const tag of ['del', 's']) { + const root = mount(`<${tag}>struck link`) + const el = root.querySelector(tag) as HTMLElement + expect(colorOf(el)).toBe(LINK_COLOR) + expect(getComputedStyle(el).textDecoration).toContain('line-through') + container?.remove() + } + }) + + it('inline-code link keeps its monospace font and background', () => { + const root = mount('code link') + const code = root.querySelector('code') as HTMLElement + const computed = getComputedStyle(code) + expect(colorOf(code)).toBe(LINK_COLOR) + expect(computed.fontFamily).toContain('mono') + expect(computed.background).toContain('var(--surface-5)') + }) +}) + +describe('multiple marks stacked together with a link', () => { + it('bold + italic + link: link color wins, both font-weight and font-style are preserved', () => { + const root = mount('bold italic link') + const em = root.querySelector('em') as HTMLElement + const strong = root.querySelector('strong') as HTMLElement + expect(colorOf(em)).toBe(LINK_COLOR) + expect(colorOf(strong)).toBe(LINK_COLOR) + expect(getComputedStyle(em).fontStyle).toBe('italic') + expect(getComputedStyle(strong).fontWeight).toBe('600') + }) + + it('bold + italic + strikethrough + link: link color wins at every nesting level', () => { + const root = mount( + 'bold italic struck link' + ) + for (const tag of ['strong', 'em', 'del']) { + expect(colorOf(root.querySelector(tag))).toBe(LINK_COLOR) + } + }) + + it('a mark stack with NO link present is unaffected by the link-precedence rule', () => { + const root = mount('bold italic, no link') + expect(colorOf(root.querySelector('em'))).toBe('var(--text-primary)') + expect(colorOf(root.querySelector('strong'))).toBe('var(--text-primary)') + }) +}) + +describe('a link elsewhere in the document does not bleed color into unrelated marks', () => { + it('a sibling bold run outside any link keeps its own color', () => { + const root = mount('

a link and bold text

') + expect(colorOf(root.querySelector('a'))).toBe(LINK_COLOR) + expect(colorOf(root.querySelector('strong'))).toBe('var(--text-primary)') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css index c8b56385b56..0a908d03849 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css @@ -232,6 +232,27 @@ padding: 0.125rem 0.375rem; } +/* A mark (bold/italic/strikethrough/inline-code) stacked with a link must not steal the link's blue — + * each mark rule above sets its own explicit `color` for the no-link case, which otherwise wins over + * the inherited link color (an inherited value never beats an element's own explicit rule, regardless + * of the ancestor selector's specificity). Covers both DOM nesting orders since ProseMirror's mark + * array — and so the render order marks nest in — depends on which was toggled first, not a fixed + * schema order. Declared after every mark rule above so it wins on source order too, not just + * specificity. Only `color` is overridden here: each mark's own font-weight/font-style/ + * text-decoration/background is unrelated to color and keeps compositing normally underneath. */ +.rich-markdown-prose a strong, +.rich-markdown-prose a em, +.rich-markdown-prose a del, +.rich-markdown-prose a s, +.rich-markdown-prose a code, +.rich-markdown-prose strong a, +.rich-markdown-prose em a, +.rich-markdown-prose del a, +.rich-markdown-prose s a, +.rich-markdown-prose code a { + color: var(--brand-secondary); +} + .rich-markdown-prose pre, .rich-markdown-prose .mermaid-diagram-frame { background: var(--surface-5); From 8c67afcfd4d613327c5666632fa9ee8ffa3acd1f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 10 Jul 2026 15:01:46 -0700 Subject: [PATCH 3/5] fix(rich-markdown-editor): fix real-world gaps in the image dupe-upload fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while re-verifying the drag/paste dupe-upload fix against the actual rendered DOM before trusting it: - hasHostedImageHtml's predicate only recognized the *persisted* src shape (extractEmbeddedFileRef, e.g. /api/files/view/...). The DOM (and so a same-page copy's clipboard html) always contains resolveImageSrc's REWRITTEN inline-route URL instead (/api/workspaces/{id}/files/inline?key=.../?fileId=..., or the public-share equivalent) — a shape the fix never recognized, so it silently never engaged for a real browser copy. Added isInlineRouteSrc to also recognize it, verified end-to-end against the real resolveImageSrc output (not a hand-typed guess). - The img-src regex only matched quoted attribute values; an unquoted src (valid HTML) fell through to the old re-upload path instead of being recognized as hosted. - The paste bypass fired on ANY hosted image found in the html, even when the clipboard also offered additional image files — a genuinely mixed paste (the hosted image plus a separate new one) would have the new file silently dropped instead of uploaded. Narrowed to only bypass when exactly one image file is offered. - The drop bypass checked view.dragging unconditionally, including for the plain-file swallow branch below it — a stale view.dragging (ProseMirror clears it up to ~50ms late via dragend when a prior internal drag was dropped outside the view) could suppress swallowing an unrelated non-image file drop (e.g. a PDF) in that window, letting it fall through to the browser default. Gated on images.length > 0 so staleness can only ever affect the image-specific path it exists for. Extracted the paste/drop bypass decisions into shouldSkipPasteUpload/shouldSkipDropUpload so they're unit-testable without mounting the full editor component. --- .../rich-markdown-editor/image-paste.test.ts | 119 +++++++++++++++++- .../rich-markdown-editor/image-paste.ts | 61 ++++++++- .../rich-markdown-editor.tsx | 17 ++- 3 files changed, 188 insertions(+), 9 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts index 7d9de1feb77..eb651f51f69 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts @@ -2,7 +2,18 @@ * @vitest-environment jsdom */ import { describe, expect, it } from 'vitest' -import { extractImageFiles, hasHostedImageHtml } from './image-paste' +import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref' +import { + createPublicFileContentSource, + createWorkspaceFileContentSource, +} from '@/hooks/use-file-content-source' +import { + extractImageFiles, + hasHostedImageHtml, + isInlineRouteSrc, + shouldSkipDropUpload, + shouldSkipPasteUpload, +} from './image-paste' function imageFile(name = 'shot.png'): File { return new File([''], name, { type: 'image/png' }) @@ -76,4 +87,110 @@ describe('hasHostedImageHtml', () => { ) ).toBe(true) }) + + // Regression: the browser doesn't put the node's persisted `attrs.src` (`/api/files/view/...`) + // onto the clipboard when a rendered 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 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(``, isHostedReal)).toBe(true) + expect(hasHostedImageHtml(``, 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(``, () => false)).toBe(true) + }) + + it('matches a valid unquoted src attribute (unquoted attribute values are valid HTML)', () => { + expect(hasHostedImageHtml('', isHosted)).toBe(true) + expect(hasHostedImageHtml("x", isHosted)).toBe( + true + ) + expect(hasHostedImageHtml('', isHosted)).toBe(false) + }) + + it('matches single-quoted src attributes too', () => { + expect(hasHostedImageHtml("", isHosted)).toBe(true) + }) +}) + +describe('shouldSkipPasteUpload', () => { + const isHosted = (src: string) => src.startsWith('/api/files/view/') + const hostedHtml = '' + + it('skips upload for a single already-hosted image', () => { + expect(shouldSkipPasteUpload([imageFile()], hostedHtml, isHosted)).toBe(true) + }) + + it('does not skip when there is no html, or the html is not one of ours', () => { + expect(shouldSkipPasteUpload([imageFile()], '', isHosted)).toBe(false) + expect(shouldSkipPasteUpload([imageFile()], '', isHosted)).toBe( + false + ) + }) + + it('does not skip when there are no files to upload in the first place', () => { + expect(shouldSkipPasteUpload([], hostedHtml, isHosted)).toBe(false) + }) + + // Regression: a genuinely mixed paste (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 carrying more than one image file', () => { + expect( + shouldSkipPasteUpload([imageFile('a.png'), imageFile('b.png')], hostedHtml, isHosted) + ).toBe(false) + }) +}) + +describe('shouldSkipDropUpload', () => { + it('skips upload for an internal image-node drag (dragging + an image file present)', () => { + expect(shouldSkipDropUpload({ slice: {}, move: true }, [imageFile()])).toBe(true) + }) + + it('does not skip when dragging is null (a genuine external drop)', () => { + expect(shouldSkipDropUpload(null, [imageFile()])).toBe(false) + }) + + // Regression: `view.dragging` can go briefly stale after a prior internal drag was dropped + // outside the view (ProseMirror clears it up to ~50ms late via `dragend`). It must never suppress + // handling of an unrelated drop that carries no image files (e.g. a PDF) in that window. + it('does not skip a stale dragging state when the drop carries no image files', () => { + expect(shouldSkipDropUpload({ slice: {}, move: true }, [])).toBe(false) + }) +}) + +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) + }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts index fd1a00fd50a..5472ef3d8a7 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts @@ -13,7 +13,36 @@ export function extractImageFiles(transfer: DataTransfer | null): File[] { .filter((file): file is File => file !== null) } -const IMG_SRC_RE = /]*\bsrc\s*=\s*["']([^"']+)["']/gi +// `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 = /]*\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 ``, 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 + } +} /** * True when `html` contains an `` whose `src` is already one of our own hosted workspace file @@ -27,7 +56,35 @@ const IMG_SRC_RE = /]*\bsrc\s*=\s*["']([^"']+)["']/gi */ export function hasHostedImageHtml(html: string, isHostedRef: (src: string) => boolean): boolean { for (const match of html.matchAll(IMG_SRC_RE)) { - if (isHostedRef(match[1])) return true + const src = match[1] ?? match[2] ?? match[3] + if (src && (isHostedRef(src) || isInlineRouteSrc(src))) return true } return false } + +/** + * True when a paste should be left to the editor's default HTML-based handling instead of going + * through the upload-from-file path: exactly one image file is offered, and the clipboard's HTML + * sibling shows it's a same-page copy of an already-hosted image (see {@link hasHostedImageHtml}). + * Gated on exactly one file — a genuinely mixed paste (the hosted image plus a separate new one) + * must still upload the new file, not have the whole paste swallowed by this bypass. + */ +export function shouldSkipPasteUpload( + images: File[], + html: string, + isHostedRef: (src: string) => boolean +): boolean { + return images.length === 1 && Boolean(html) && hasHostedImageHtml(html, isHostedRef) +} + +/** + * True when a drop should be left to ProseMirror's default move handling instead of going through + * the upload-from-file path: it's a within-view node drag (`dragging`, ProseMirror's own signal — + * see `EditorView.dragging`) that dropped at least one image file. Gated on `images.length > 0`, not + * `dragging` alone — `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 + * handling of an unrelated, non-image file drop that happens to land in that window. + */ +export function shouldSkipDropUpload(dragging: unknown, images: File[]): boolean { + return Boolean(dragging) && images.length > 0 +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index 31114c33884..f7eb95c3a00 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -14,7 +14,7 @@ import { PreviewLoadingFrame } from '../preview-shared' import { useEditableFileContent } from '../use-editable-file-content' import { createMarkdownEditorExtensions } from './editor-extensions' import { findHeadingPos } from './heading-anchors' -import { extractImageFiles, hasHostedImageHtml } from './image-paste' +import { extractImageFiles, shouldSkipDropUpload, shouldSkipPasteUpload } from './image-paste' import { applyFrontmatter, normalizeLinkHref, @@ -306,15 +306,17 @@ export function LoadedRichMarkdownEditor({ * from a genuine external image paste by `clipboardData` files/items alone. When the HTML sibling * already names one of our own hosted files, bail out and let the editor's default HTML paste * clone that node (reusing its real `src` and every other attribute) instead of re-uploading the - * pasted bytes as a brand-new, distinct file. + * pasted bytes as a brand-new, distinct file. Only applied when exactly one image file is offered: + * a genuinely mixed paste (the hosted image plus a separate new one) must still upload the new + * file rather than have the whole paste swallowed by the single-image bypass below. */ handlePaste: (view, event) => { if (!view.editable) return false + const images = extractImageFiles(event.clipboardData) const html = event.clipboardData?.getData('text/html') ?? '' - if (html && hasHostedImageHtml(html, (src) => extractEmbeddedFileRef(src) !== null)) { + if (shouldSkipPasteUpload(images, html, (src) => extractEmbeddedFileRef(src) !== null)) { return false } - const images = extractImageFiles(event.clipboardData) if (images.length === 0) return false event.preventDefault() void insertImagesRef.current(images, view.state.selection.from) @@ -330,12 +332,15 @@ export function LoadedRichMarkdownEditor({ * (the same mechanism that lets a user drag a web image out to their desktop) — indistinguishable * from a real external drop by `dataTransfer` contents alone. `view.dragging` is ProseMirror's own * signal that this drop follows a `dragstart` within this same view, so bail out and let its - * default move logic run instead of re-uploading the dragged image as a duplicate. + * default move logic run instead of re-uploading the dragged image as a duplicate. Gated on + * `images.length > 0` (not checked unconditionally) so a stale `view.dragging` — it's cleared up + * to ~50ms late by ProseMirror's own `dragend` handler when a prior internal drag was dropped + * outside this view — can never suppress the plain-file swallow guard below for an unrelated drop. */ handleDrop: (view, event) => { if (!view.editable) return false - if (view.dragging) return false const images = extractImageFiles(event.dataTransfer) + if (shouldSkipDropUpload(view.dragging, images)) return false if (images.length > 0) { event.preventDefault() const dropPos = view.posAtCoords({ left: event.clientX, top: event.clientY })?.pos From 26b1c54bae1700a3c73694104a210750820b1767 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 10 Jul 2026 15:18:39 -0700 Subject: [PATCH 4/5] fix(rich-markdown-editor): fix the entire class of mark-color-vs-ambient-color bugs, not just links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auditing every explicit `color` in this file for the same failure mode (an element's own explicit color always wins over an inherited one, regardless of ancestor specificity) surfaced a second, previously-unfixed instance: bold/italic text inside an h6 heading showed the brighter --text-primary instead of h6's own intentionally dimmer --text-secondary, since strong/em hardcoded --text-primary as their default. strong/em's color was always redundant with the prose root's own default anyway — removing it entirely (matching the highlight/`mark` rule's existing `color: inherit` convention in this same file) lets normal CSS inheritance carry the correct color through from ANY ambient context, not just links: a link's blue, h6's dimmer tone, or any future colored container this file doesn't know about yet. `code` has the same redundant color, also removed. del/s genuinely need their own dimmer default (distinct from the prose default), so they keep an explicit color plus the link-color override — now the only mark that needs one, since strong/em/ code no longer set a competing color to override in the first place. 10 new tests cover every heading level x strong/em/code/del/s x link, including 2 that fail against the pre-fix CSS (bold/italic and inline-code inside h6 both incorrectly showed --text-primary). --- .../link-mark-precedence.test.ts | 66 +++++++++++++++++-- .../rich-markdown-editor.css | 38 ++++++----- 2 files changed, 79 insertions(+), 25 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts index 0eab233305c..a2edbbae3e5 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts @@ -1,16 +1,23 @@ /** * @vitest-environment jsdom * - * A mark (bold/italic/strikethrough/inline-code) stacked on top of a link must not steal the link's - * blue: `strong`/`em`/`del`/`s`/`code` each set their own explicit `color` for the no-link case, and an - * element's own explicit rule always wins over an inherited value regardless of how specific the - * ancestor's (`a`'s) selector is — so without an override targeting the mark itself, a bold/italic/ - * struck-through/code link renders in the mark's plain-text color instead of the link color. + * A mark stacked on top of another color-setting context must not steal that context's color: an + * element's own explicit `color` rule always wins over an inherited value, regardless of how specific + * the ancestor's selector is. Two contexts hit this in practice: + * + * - A link's blue, stacked with bold/italic/strikethrough/inline-code. `strong`/`em`/`code` no longer + * set their own `color` at all (it was redundant with the prose default anyway, and — like the + * `mark`/highlight rule's own `color: inherit` — the absence of a rule is what lets inheritance + * carry through correctly from ANY ambient context, not just links). `del`/`s` genuinely need their + * own dimmer default, so they keep an explicit `color` plus an override for the link case. + * - `h6`'s intentionally dimmer `--text-secondary` (vs. every other heading and the prose default, + * both `--text-primary`), stacked with bold/italic — before strong/em's color was removed, a bold + * run inside an `h6` incorrectly showed the brighter `--text-primary` instead of `h6`'s own tone. * * These load the real, shipped `rich-markdown-editor.css` (not a copy) and assert against * `getComputedStyle` in jsdom. jsdom's CSS engine does not resolve `var(...)` references to actual - * color values, but it does correctly resolve the cascade/specificity winner and returns that - * declaration's authored value verbatim — so asserting the winning declaration is `var(--brand-secondary)` + * color values, but it does correctly resolve the cascade/specificity/inheritance winner and returns + * that declaration's authored value verbatim — so asserting the winning value is `var(--brand-secondary)` * (vs. e.g. `var(--text-primary)`) is a precise, real assertion about which CSS rule wins, not a proxy. */ import { readFileSync } from 'node:fs' @@ -163,3 +170,48 @@ describe('a link elsewhere in the document does not bleed color into unrelated m expect(colorOf(root.querySelector('strong'))).toBe('var(--text-primary)') }) }) + +describe('headings stack correctly with marks (same bug class, a different ambient color)', () => { + it.each(['h1', 'h2', 'h3', 'h4', 'h5'])( + '%s carries --text-primary, same as bold/italic inside it — no visible regression either way', + (tag) => { + const root = mount(`<${tag}>bold italic`) + expect(colorOf(root.querySelector(tag))).toBe('var(--text-primary)') + expect(colorOf(root.querySelector('strong'))).toBe('var(--text-primary)') + expect(colorOf(root.querySelector('em'))).toBe('var(--text-primary)') + } + ) + + // h6 is the one heading with its own dimmer tone (--text-secondary, not --text-primary like every + // other heading and the prose default) — the exact shape of ambient color strong/em must inherit + // rather than reset, the same failure mode as the link case above just triggered by a heading + // instead of an . + it('h6 keeps its dimmer --text-secondary, and bold/italic inside it inherit that tone (not --text-primary)', () => { + const root = mount('
bold italic plain
') + const h6 = root.querySelector('h6') as HTMLElement + expect(colorOf(h6)).toBe('var(--text-secondary)') + expect(colorOf(root.querySelector('strong'))).toBe('var(--text-secondary)') + expect(colorOf(root.querySelector('em'))).toBe('var(--text-secondary)') + }) + + it('inline code inside h6 also inherits --text-secondary, not its own hardcoded default', () => { + const root = mount('
code
') + expect(colorOf(root.querySelector('code'))).toBe('var(--text-secondary)') + }) + + it('a struck-through run inside h6 keeps its own dimmer tertiary tone (mark-own-color still wins over a non-link ambient context)', () => { + const root = mount('
struck
') + expect(colorOf(root.querySelector('del'))).toBe('var(--text-tertiary)') + }) + + it('an anchor heading (link wrapping the whole heading) still shows link color, unaffected by h6', () => { + const root = mount('
linked heading
') + expect(colorOf(root.querySelector('a'))).toBe(LINK_COLOR) + }) + + it('bold + italic + link inside h6: link color wins over both the mark defaults and h6 itself', () => { + const root = mount('
bold italic link
') + expect(colorOf(root.querySelector('em'))).toBe(LINK_COLOR) + expect(colorOf(root.querySelector('strong'))).toBe(LINK_COLOR) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css index 0a908d03849..5c698d25835 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css @@ -94,16 +94,23 @@ color: var(--text-secondary); } +/* No explicit `color` on strong/em — an inherited value never beats an element's own explicit rule + * regardless of the ancestor selector's specificity, so a hardcoded color here would silently win + * over ANY ambient color a ancestor sets that differs from the prose default (a link's blue, `h6`'s + * dimmer `--text-secondary`, or any future colored container) — the same failure mode `color: + * inherit` on the `mark` (highlight) rule below already avoids. Bold/italic text is meant to carry + * the surrounding color, not reset it; omitting `color` here lets normal CSS inheritance do that + * correctly in every context, including ones this file doesn't know about yet. */ .rich-markdown-prose strong { font-weight: 600; - color: var(--text-primary); } .rich-markdown-prose em { font-style: italic; - color: var(--text-primary); } +/* del/s DO need their own explicit dimmer color (distinct from the surrounding text) as their + * default — but see the link-color override below for why that must still yield inside a link. */ .rich-markdown-prose del, .rich-markdown-prose s { color: var(--text-tertiary); @@ -223,33 +230,28 @@ font-style: italic; } +/* No explicit `color` — see the strong/em comment above; inline code composites its own font/ + * background/radius over whatever color the surrounding context (link, heading, prose default) + * already supplies. */ .rich-markdown-prose code { font-family: var(--font-martian-mono, ui-monospace, monospace); font-size: 0.875em; background: var(--surface-5); - color: var(--text-primary); border-radius: 4px; padding: 0.125rem 0.375rem; } -/* A mark (bold/italic/strikethrough/inline-code) stacked with a link must not steal the link's blue — - * each mark rule above sets its own explicit `color` for the no-link case, which otherwise wins over - * the inherited link color (an inherited value never beats an element's own explicit rule, regardless - * of the ancestor selector's specificity). Covers both DOM nesting orders since ProseMirror's mark - * array — and so the render order marks nest in — depends on which was toggled first, not a fixed - * schema order. Declared after every mark rule above so it wins on source order too, not just - * specificity. Only `color` is overridden here: each mark's own font-weight/font-style/ - * text-decoration/background is unrelated to color and keeps compositing normally underneath. */ -.rich-markdown-prose a strong, -.rich-markdown-prose a em, +/* del/s are the one mark that both needs its own explicit default color (dimmer than the prose + * default, unlike strong/em/code above) AND must still yield it to a link's blue — an inherited + * value never beats an element's own explicit rule regardless of the ancestor selector's + * specificity, so without this override a struck-through link would show the dimmer tertiary color + * instead of the link's blue. Covers both DOM nesting orders since ProseMirror's mark array — and so + * the render order marks nest in — depends on which was toggled first, not a fixed schema order. + * Declared after the del/s rule above so it wins on source order too, not just specificity. */ .rich-markdown-prose a del, .rich-markdown-prose a s, -.rich-markdown-prose a code, -.rich-markdown-prose strong a, -.rich-markdown-prose em a, .rich-markdown-prose del a, -.rich-markdown-prose s a, -.rich-markdown-prose code a { +.rich-markdown-prose s a { color: var(--brand-secondary); } From a8479f1a541d4b1c31df6501e88f4f582a5a5be6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 10 Jul 2026 15:18:52 -0700 Subject: [PATCH 5/5] fix(rich-markdown-editor): stop paste-clone from persisting the display-layer image URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor caught a real correctness bug in the paste/drop dupe-upload fix: bypassing to the editor's DEFAULT html-based paste for an already-hosted image made it re-parse the clipboard html's , which is resolveImageSrc's REWRITTEN *display* URL, not the real persisted one — baking that display-only URL into the document. Public share, export, and referenced-by-doc tracking only recognize the persisted shape, so the pasted image would silently vanish from all three. Fixed by no longer letting default paste construct the node at all: findHostedImageAttrs walks the CURRENT doc for an existing image node whose *resolved* src matches the clipboard html's, and returns that node's real, persisted attrs (src, width, href, title — everything) to clone ourselves. Falls through to a normal upload (always correct, just occasionally redundant) if no match is found, rather than ever trusting the html's src directly. Also merged the paste- and drop-specific skip checks into one shouldSkipFileUpload, and switched the drop side off `view.dragging` entirely (Greptile: it can go briefly stale, up to ~50ms, when a prior internal drag was dropped outside the view, which could suppress upload of an unrelated new file dropped in that window) — now purely a function of what the current event's html/images actually contain, which the drop's own default move logic (relocating the real node, never re-parsing html) was never at risk from in the first place. --- .../rich-markdown-editor/image-paste.test.ts | 148 ++++++++++++++---- .../rich-markdown-editor/image-paste.ts | 88 ++++++++--- .../rich-markdown-editor.tsx | 75 +++++++-- 3 files changed, 246 insertions(+), 65 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts index eb651f51f69..d782141e81c 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts @@ -1,20 +1,37 @@ /** * @vitest-environment jsdom */ -import { describe, expect, it } from 'vitest' +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, - shouldSkipDropUpload, - shouldSkipPasteUpload, + 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' }) } @@ -132,48 +149,54 @@ describe('hasHostedImageHtml', () => { }) }) -describe('shouldSkipPasteUpload', () => { +describe('extractImgSrcs', () => { + it('extracts every img src in document order, including duplicates', () => { + expect( + extractImgSrcs('

text

') + ).toEqual(['/a.png', '/b.png', '/a.png']) + }) + + it('returns an empty array for html with no img', () => { + expect(extractImgSrcs('

hello

')).toEqual([]) + expect(extractImgSrcs('')).toEqual([]) + }) +}) + +describe('shouldSkipFileUpload (shared by paste and drop)', () => { const isHosted = (src: string) => src.startsWith('/api/files/view/') const hostedHtml = '' it('skips upload for a single already-hosted image', () => { - expect(shouldSkipPasteUpload([imageFile()], hostedHtml, isHosted)).toBe(true) + 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(shouldSkipPasteUpload([imageFile()], '', isHosted)).toBe(false) - expect(shouldSkipPasteUpload([imageFile()], '', isHosted)).toBe( + expect(shouldSkipFileUpload([imageFile()], '', isHosted)).toBe(false) + expect(shouldSkipFileUpload([imageFile()], '', isHosted)).toBe( false ) }) it('does not skip when there are no files to upload in the first place', () => { - expect(shouldSkipPasteUpload([], hostedHtml, isHosted)).toBe(false) + expect(shouldSkipFileUpload([], hostedHtml, isHosted)).toBe(false) }) - // Regression: a genuinely mixed paste (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 carrying more than one image file', () => { + // 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( - shouldSkipPasteUpload([imageFile('a.png'), imageFile('b.png')], hostedHtml, isHosted) + shouldSkipFileUpload([imageFile('a.png'), imageFile('b.png')], hostedHtml, isHosted) ).toBe(false) }) -}) - -describe('shouldSkipDropUpload', () => { - it('skips upload for an internal image-node drag (dragging + an image file present)', () => { - expect(shouldSkipDropUpload({ slice: {}, move: true }, [imageFile()])).toBe(true) - }) - it('does not skip when dragging is null (a genuine external drop)', () => { - expect(shouldSkipDropUpload(null, [imageFile()])).toBe(false) - }) - - // Regression: `view.dragging` can go briefly stale after a prior internal drag was dropped - // outside the view (ProseMirror clears it up to ~50ms late via `dragend`). It must never suppress - // handling of an unrelated drop that carries no image files (e.g. a PDF) in that window. - it('does not skip a stale dragging state when the drop carries no image files', () => { - expect(shouldSkipDropUpload({ slice: {}, move: true }, [])).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) }) }) @@ -194,3 +217,74 @@ describe('isInlineRouteSrc', () => { expect(isInlineRouteSrc('data:image/png;base64,aaaa')).toBe(false) }) }) + +describe('findHostedImageAttrs', () => { + const ws = createWorkspaceFileContentSource('ws-1') + + function docWithImages(...attrs: Array>): 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') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts index 5472ef3d8a7..d3bc170f317 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts @@ -44,32 +44,41 @@ export function isInlineRouteSrc(src: string): boolean { } } +/** + * Extracts every `` `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 `` whose `src` is already one of our own hosted workspace file * references. Copying a rendered `` 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. When - * this is true, the paste handler should let the editor's default HTML-based paste clone the existing - * node (reusing its real `src`, and every other attribute) instead of re-uploading the pasted bytes as - * a brand-new file. + * 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 { - for (const match of html.matchAll(IMG_SRC_RE)) { - const src = match[1] ?? match[2] ?? match[3] - if (src && (isHostedRef(src) || isInlineRouteSrc(src))) return true - } - return false + return extractImgSrcs(html).some((src) => isHostedRef(src) || isInlineRouteSrc(src)) } /** - * True when a paste should be left to the editor's default HTML-based handling instead of going - * through the upload-from-file path: exactly one image file is offered, and the clipboard's HTML - * sibling shows it's a same-page copy of an already-hosted image (see {@link hasHostedImageHtml}). - * Gated on exactly one file — a genuinely mixed paste (the hosted image plus a separate new one) - * must still upload the new file, not have the whole paste swallowed by this bypass. + * 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 shouldSkipPasteUpload( +export function shouldSkipFileUpload( images: File[], html: string, isHostedRef: (src: string) => boolean @@ -77,14 +86,47 @@ export function shouldSkipPasteUpload( 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 +} + +/** Minimal shape of a ProseMirror doc — just enough to walk its nodes. */ +interface DescendantsDoc { + descendants: (callback: (node: ImageLikeNode) => boolean | undefined) => void +} + /** - * True when a drop should be left to ProseMirror's default move handling instead of going through - * the upload-from-file path: it's a within-view node drag (`dragging`, ProseMirror's own signal — - * see `EditorView.dragging`) that dropped at least one image file. Gated on `images.length > 0`, not - * `dragging` alone — `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 - * handling of an unrelated, non-image file drop that happens to land in that window. + * 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 shouldSkipDropUpload(dragging: unknown, images: File[]): boolean { - return Boolean(dragging) && images.length > 0 +export function findHostedImageAttrs( + doc: DescendantsDoc, + targetSrcs: string[], + resolveImageSrc: (src: string | undefined) => string | undefined +): Record | null { + const targets = new Set(targetSrcs) + let found: Record | 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 } diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index f7eb95c3a00..5057c835af9 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -10,11 +10,17 @@ import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace' import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref' import { useUploadWorkspaceFile } from '@/hooks/queries/workspace-files' import type { SaveStatus } from '@/hooks/use-autosave' +import { useFileContentSource } from '@/hooks/use-file-content-source' import { PreviewLoadingFrame } from '../preview-shared' import { useEditableFileContent } from '../use-editable-file-content' import { createMarkdownEditorExtensions } from './editor-extensions' import { findHeadingPos } from './heading-anchors' -import { extractImageFiles, shouldSkipDropUpload, shouldSkipPasteUpload } from './image-paste' +import { + extractImageFiles, + extractImgSrcs, + findHostedImageAttrs, + shouldSkipFileUpload, +} from './image-paste' import { applyFrontmatter, normalizeLinkHref, @@ -209,6 +215,7 @@ export function LoadedRichMarkdownEditor({ const containerRef = useRef(null) const uploadFile = useUploadWorkspaceFile() const editorInstanceRef = useRef(null) + const source = useFileContentSource() /** * The `/Image` slash command opens this hidden picker; `pendingImagePosRef` holds the caret position @@ -251,6 +258,31 @@ export function LoadedRichMarkdownEditor({ } } + /** + * A same-page copy/drag of an already-hosted `` carries the clipboard/dataTransfer `html`'s + * *display* src (`source.resolveImageSrc`'s rewrite), not the real persisted one — inserting a node + * built straight from that html would bake the display-only URL into the document, breaking public + * share/export/referenced-by-doc tracking for it (they only recognize the persisted shape). + * `findHostedImageAttrs` finds the real, already-present node with a matching resolved src instead, + * so the clone gets the exact real `src` (and every other attribute — width, href, title…) rather + * than a re-derived guess. Returns `false` (falls through to a normal upload) if no match is found, + * which is always correct, just occasionally a redundant upload — unlike blindly trusting the html. + */ + const cloneHostedImageRef = useRef<(imgSrcs: string[], at: number) => boolean>(() => false) + cloneHostedImageRef.current = (imgSrcs, at) => { + const editor = editorInstanceRef.current + if (!editor) return false + const matchedAttrs = findHostedImageAttrs(editor.state.doc, imgSrcs, source.resolveImageSrc) + if (!matchedAttrs) return false + const safePosition = Math.min(at, editor.state.doc.content.size) + try { + editor.chain().insertContentAt(safePosition, { type: 'image', attrs: matchedAttrs }).run() + return true + } catch { + return false + } + } + const editor = useEditor({ extensions: EXTENSIONS, editable: isEditable, @@ -304,18 +336,26 @@ export function LoadedRichMarkdownEditor({ * Cmd+C after clicking it to select it) makes the browser add BOTH `text/html` (the real node, * with its real hosted `src`) AND a synthesized image `File` to the clipboard — indistinguishable * from a genuine external image paste by `clipboardData` files/items alone. When the HTML sibling - * already names one of our own hosted files, bail out and let the editor's default HTML paste - * clone that node (reusing its real `src` and every other attribute) instead of re-uploading the - * pasted bytes as a brand-new, distinct file. Only applied when exactly one image file is offered: - * a genuinely mixed paste (the hosted image plus a separate new one) must still upload the new - * file rather than have the whole paste swallowed by the single-image bypass below. + * already names one of our own hosted files, look up the matching node already in this doc and + * clone ITS real attrs (see `cloneHostedImageRef`) instead of re-uploading the pasted bytes as a + * brand-new, distinct file — letting the editor's DEFAULT html-based paste do that clone instead + * would persist the html's display-layer src rather than the real one. Only applied when exactly + * one image file is offered: a genuinely mixed paste (the hosted image plus a separate new one) + * must still upload the new file rather than have the whole paste diverted by this bypass. */ handlePaste: (view, event) => { if (!view.editable) return false const images = extractImageFiles(event.clipboardData) const html = event.clipboardData?.getData('text/html') ?? '' - if (shouldSkipPasteUpload(images, html, (src) => extractEmbeddedFileRef(src) !== null)) { - return false + if (shouldSkipFileUpload(images, html, (src) => extractEmbeddedFileRef(src) !== null)) { + const cloned = cloneHostedImageRef.current( + extractImgSrcs(html), + view.state.selection.from + ) + if (cloned) { + event.preventDefault() + return true + } } if (images.length === 0) return false event.preventDefault() @@ -330,17 +370,22 @@ export function LoadedRichMarkdownEditor({ * Dragging an existing image node to reorder it is also an internal drag, but the browser's * native drag-and-drop synthesizes an image `File` into `event.dataTransfer` for a dragged `` * (the same mechanism that lets a user drag a web image out to their desktop) — indistinguishable - * from a real external drop by `dataTransfer` contents alone. `view.dragging` is ProseMirror's own - * signal that this drop follows a `dragstart` within this same view, so bail out and let its - * default move logic run instead of re-uploading the dragged image as a duplicate. Gated on - * `images.length > 0` (not checked unconditionally) so a stale `view.dragging` — it's cleared up - * to ~50ms late by ProseMirror's own `dragend` handler when a prior internal drag was dropped - * outside this view — can never suppress the plain-file swallow guard below for an unrelated drop. + * from a real external drop by `dataTransfer` files/items alone. When the accompanying `text/html` + * shows it's a same-page drag of an already-hosted image, bail out and let ProseMirror's own + * default move logic run — it relocates the actual existing node object (`dragging.node`), never + * re-parsing html, so (unlike the paste case above) there's no display-vs-persisted src risk here. + * Checked from `html`, not `view.dragging`, so a stale `dragging` flag (ProseMirror clears it up + * to ~50ms late via `dragend` when a prior internal drag was dropped outside this view) can never + * suppress the plain-file swallow guard below for an unrelated drop that happens to land in that + * window — this only reacts to what THIS specific drop's `html` actually contains. */ handleDrop: (view, event) => { if (!view.editable) return false const images = extractImageFiles(event.dataTransfer) - if (shouldSkipDropUpload(view.dragging, images)) return false + const html = event.dataTransfer?.getData('text/html') ?? '' + if (shouldSkipFileUpload(images, html, (src) => extractEmbeddedFileRef(src) !== null)) { + return false + } if (images.length > 0) { event.preventDefault() const dropPos = view.posAtCoords({ left: event.clientX, top: event.clientY })?.pos