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..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,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 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) + }) + + // 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('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(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()], '', 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>): 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 ff72fededf9..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 @@ -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 = /]*\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 + } +} + +/** + * 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. + */ +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 +} + +/** 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 | 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/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..a2edbbae3e5 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/link-mark-precedence.test.ts @@ -0,0 +1,217 @@ +/** + * @vitest-environment jsdom + * + * 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/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' +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)') + }) +}) + +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 c8b56385b56..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,15 +230,31 @@ 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; } +/* 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 del a, +.rich-markdown-prose s a { + color: var(--brand-secondary); +} + .rich-markdown-prose pre, .rich-markdown-prose .mermaid-diagram-frame { background: var(--surface-5); 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..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 @@ -7,13 +7,20 @@ 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 { 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 } from './image-paste' +import { + extractImageFiles, + extractImgSrcs, + findHostedImageAttrs, + shouldSkipFileUpload, +} from './image-paste' import { applyFrontmatter, normalizeLinkHref, @@ -208,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 @@ -250,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, @@ -298,9 +331,32 @@ 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, 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 (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() void insertImagesRef.current(images, view.state.selection.from) @@ -310,10 +366,26 @@ 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` 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) + 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