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

* fix(rich-markdown-editor): stop drag/paste of an existing image from re-uploading a duplicate

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 <img> 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 <img> 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.

* fix(rich-markdown-editor): fix link color lost under bold/italic/strikethrough/code

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 <a> — 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 <mark>` / `.rich-markdown-prose <mark> 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.

* fix(rich-markdown-editor): fix real-world gaps in the image dupe-upload fix

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.

* fix(rich-markdown-editor): fix the entire class of mark-color-vs-ambient-color bugs, not just links

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).

* fix(rich-markdown-editor): stop paste-clone from persisting the display-layer image URL

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 <img
src>, 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.
This commit is contained in:
Waleed
2026-07-10 15:27:51 -07:00
committed by GitHub
parent fac8ec0c51
commit 5de66db678
5 changed files with 670 additions and 6 deletions
@@ -1,8 +1,36 @@
/**
* @vitest-environment jsdom
*/
import { describe, expect, it } from 'vitest'
import { extractImageFiles } from './image-paste'
import { Editor } from '@tiptap/core'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { extractEmbeddedFileRef } from '@/lib/uploads/utils/embedded-image-ref'
import {
createPublicFileContentSource,
createWorkspaceFileContentSource,
} from '@/hooks/use-file-content-source'
import { createMarkdownEditorExtensions } from './editor-extensions'
import {
extractImageFiles,
extractImgSrcs,
findHostedImageAttrs,
hasHostedImageHtml,
isInlineRouteSrc,
shouldSkipFileUpload,
} from './image-paste'
// jsdom lacks `elementFromPoint`; the Placeholder extension's viewport tracking calls it on mount.
beforeEach(() => {
vi.stubGlobal(
'ResizeObserver',
class {
observe() {}
unobserve() {}
disconnect() {}
}
)
Element.prototype.scrollIntoView = vi.fn()
document.elementFromPoint = vi.fn(() => null)
})
function imageFile(name = 'shot.png'): File {
return new File([''], name, { type: 'image/png' })
@@ -54,3 +82,209 @@ describe('extractImageFiles', () => {
expect(result).toEqual([])
})
})
describe('hasHostedImageHtml', () => {
const isHosted = (src: string) => src.startsWith('/api/files/view/')
it('detects an <img> whose src is recognized as one of our own hosted files', () => {
expect(hasHostedImageHtml('<img src="/api/files/view/wf_abc" alt="x">', isHosted)).toBe(true)
})
it('is false when the html has no img, or the img src is not one of ours', () => {
expect(hasHostedImageHtml('<p>hello</p>', isHosted)).toBe(false)
expect(hasHostedImageHtml('<img src="https://other-site.com/photo.jpg">', isHosted)).toBe(false)
expect(hasHostedImageHtml('', isHosted)).toBe(false)
})
it('matches a hosted img among multiple candidates', () => {
expect(
hasHostedImageHtml(
'<img src="https://other-site.com/a.png"><img src="/api/files/view/wf_abc">',
isHosted
)
).toBe(true)
})
// Regression: the browser doesn't put the node's persisted `attrs.src` (`/api/files/view/...`)
// onto the clipboard when a rendered <img> is copied — it puts the actual DOM `src`, which is
// `resolveImageSrc`'s REWRITTEN display URL (`/…/files/inline?key=…`/`?fileId=…`). A predicate
// that only recognizes the persisted shape (as `extractEmbeddedFileRef` alone does) never matches
// a real same-page copy, silently falling through to the re-upload path it exists to avoid.
it('recognizes the real rendered <img src> end-to-end, not just the persisted reference shape', () => {
const ws = createWorkspaceFileContentSource('ws-1')
const renderedFromKey = ws.resolveImageSrc(
'/api/files/serve/workspace/ws-1/1700000000000-deadbeefdeadbeef-photo.png'
)
const renderedFromFileId = ws.resolveImageSrc('/api/files/view/wf_abc')
expect(renderedFromKey).toMatch(/^\/api\/workspaces\/ws-1\/files\/inline\?key=/)
expect(renderedFromFileId).toBe('/api/workspaces/ws-1/files/inline?fileId=wf_abc')
// extractEmbeddedFileRef alone (the persisted-content recognizer) does NOT match either
// rendered form — that's the exact gap isInlineRouteSrc closes.
expect(extractEmbeddedFileRef(renderedFromKey as string)).toBeNull()
expect(extractEmbeddedFileRef(renderedFromFileId as string)).toBeNull()
const isHostedReal = (src: string) => extractEmbeddedFileRef(src) !== null
expect(hasHostedImageHtml(`<img src="${renderedFromKey}">`, isHostedReal)).toBe(true)
expect(hasHostedImageHtml(`<img src="${renderedFromFileId}">`, isHostedReal)).toBe(true)
})
it('recognizes the public-share inline route too', () => {
const pub = createPublicFileContentSource('tok_1', '/api/files/public/tok_1/content')
const rendered = pub.resolveImageSrc('/api/files/view/wf_abc')
expect(rendered).toBe('/api/files/public/tok_1/inline?fileId=wf_abc')
expect(hasHostedImageHtml(`<img src="${rendered}">`, () => false)).toBe(true)
})
it('matches a valid unquoted src attribute (unquoted attribute values are valid HTML)', () => {
expect(hasHostedImageHtml('<img src=/api/files/view/wf_abc>', isHosted)).toBe(true)
expect(hasHostedImageHtml("<img alt='x' src=/api/files/view/wf_abc alt=y>", isHosted)).toBe(
true
)
expect(hasHostedImageHtml('<img src=https://other-site.com/a.png>', isHosted)).toBe(false)
})
it('matches single-quoted src attributes too', () => {
expect(hasHostedImageHtml("<img src='/api/files/view/wf_abc'>", isHosted)).toBe(true)
})
})
describe('extractImgSrcs', () => {
it('extracts every img src in document order, including duplicates', () => {
expect(
extractImgSrcs('<img src="/a.png"><p>text</p><img src="/b.png"><img src="/a.png">')
).toEqual(['/a.png', '/b.png', '/a.png'])
})
it('returns an empty array for html with no img', () => {
expect(extractImgSrcs('<p>hello</p>')).toEqual([])
expect(extractImgSrcs('')).toEqual([])
})
})
describe('shouldSkipFileUpload (shared by paste and drop)', () => {
const isHosted = (src: string) => src.startsWith('/api/files/view/')
const hostedHtml = '<img src="/api/files/view/wf_abc">'
it('skips upload for a single already-hosted image', () => {
expect(shouldSkipFileUpload([imageFile()], hostedHtml, isHosted)).toBe(true)
})
it('does not skip when there is no html, or the html is not one of ours', () => {
expect(shouldSkipFileUpload([imageFile()], '', isHosted)).toBe(false)
expect(shouldSkipFileUpload([imageFile()], '<img src="https://x.com/a.png">', isHosted)).toBe(
false
)
})
it('does not skip when there are no files to upload in the first place', () => {
expect(shouldSkipFileUpload([], hostedHtml, isHosted)).toBe(false)
})
// Regression: a genuinely mixed paste/drop (the hosted image plus a separate new one) must
// still upload the new file — bailing out entirely here would silently drop it.
it('does not skip a mixed paste/drop carrying more than one image file', () => {
expect(
shouldSkipFileUpload([imageFile('a.png'), imageFile('b.png')], hostedHtml, isHosted)
).toBe(false)
})
// Regression: this must be content-based (the accompanying html), not keyed off any mutable
// per-drag flag like ProseMirror's `view.dragging` — that flag can go briefly stale (cleared up
// to ~50ms late via `dragend` when a prior internal drag was dropped outside the view), and a
// flag-based check would incorrectly suppress upload of an unrelated new file dropped in that
// window. This function only reacts to what THIS specific event's `html`/`images` contain.
it('is a pure function of the images/html actually offered, independent of any drag-session flag', () => {
expect(shouldSkipFileUpload([imageFile()], '', isHosted)).toBe(false)
expect(shouldSkipFileUpload([imageFile()], hostedHtml, isHosted)).toBe(true)
})
})
describe('isInlineRouteSrc', () => {
it('recognizes the workspace- and public-scoped inline route with key or fileId', () => {
expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline?key=workspace%2Fws-1%2Fa.png')).toBe(
true
)
expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline?fileId=wf_abc')).toBe(true)
expect(isInlineRouteSrc('/api/files/public/tok_1/inline?fileId=wf_abc')).toBe(true)
})
it('rejects non-inline paths, unrecognized query params, and external/absolute origins', () => {
expect(isInlineRouteSrc('/api/files/serve/workspace/ws-1/a.png')).toBe(false)
expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline')).toBe(false)
expect(isInlineRouteSrc('/api/workspaces/ws-1/files/inline?other=1')).toBe(false)
expect(isInlineRouteSrc('https://other-site.com/files/inline?key=x')).toBe(false)
expect(isInlineRouteSrc('data:image/png;base64,aaaa')).toBe(false)
})
})
describe('findHostedImageAttrs', () => {
const ws = createWorkspaceFileContentSource('ws-1')
function docWithImages(...attrs: Array<Record<string, unknown>>): Editor {
return new Editor({
extensions: createMarkdownEditorExtensions({ placeholder: '' }),
content: {
type: 'doc',
content: attrs.map((a) => ({ type: 'image', attrs: a })),
},
})
}
// Regression: this is the exact mechanism that avoids persisting the display-layer inline URL
// (Cursor's "Paste persists display image URLs" finding) — cloning the REAL node's attrs rather
// than re-deriving a node from the clipboard html's rewritten src.
it('finds the existing node whose RESOLVED (display) src matches, and returns its REAL persisted attrs', () => {
const persistedSrc = '/api/files/view/wf_abc'
const editor = docWithImages({ src: persistedSrc, alt: 'photo', width: '300' })
const renderedSrc = ws.resolveImageSrc(persistedSrc) as string
expect(renderedSrc).not.toBe(persistedSrc) // sanity: the rendered form really differs
const match = findHostedImageAttrs(editor.state.doc, [renderedSrc], ws.resolveImageSrc)
expect(match).not.toBeNull()
expect(match?.src).toBe(persistedSrc) // the REAL persisted src, not the rendered one
expect(match?.alt).toBe('photo')
expect(match?.width).toBe('300')
})
it('returns null when no node in the doc resolves to any target src', () => {
const editor = docWithImages({ src: '/api/files/view/wf_other' })
const match = findHostedImageAttrs(
editor.state.doc,
['/api/workspaces/ws-1/files/inline?fileId=wf_abc'],
ws.resolveImageSrc
)
expect(match).toBeNull()
})
it('returns null for an empty doc or an empty target list', () => {
const editor = docWithImages()
expect(findHostedImageAttrs(editor.state.doc, ['/anything'], ws.resolveImageSrc)).toBeNull()
const editorWithImage = docWithImages({ src: '/api/files/view/wf_abc' })
expect(findHostedImageAttrs(editorWithImage.state.doc, [], ws.resolveImageSrc)).toBeNull()
})
it('matches the first of several images, not just the last', () => {
const editor = docWithImages(
{ src: '/api/files/view/wf_one', alt: 'one' },
{ src: '/api/files/view/wf_two', alt: 'two' }
)
const renderedTwo = ws.resolveImageSrc('/api/files/view/wf_two') as string
const match = findHostedImageAttrs(editor.state.doc, [renderedTwo], ws.resolveImageSrc)
expect(match?.alt).toBe('two')
})
it('returns a defensive copy, not a live reference to the node attrs object', () => {
const persistedSrc = '/api/files/view/wf_abc'
const editor = docWithImages({ src: persistedSrc, alt: 'photo' })
const renderedSrc = ws.resolveImageSrc(persistedSrc) as string
const match = findHostedImageAttrs(editor.state.doc, [renderedSrc], ws.resolveImageSrc)
expect(match).not.toBeNull()
if (match) match.alt = 'mutated'
let originalAlt: unknown
editor.state.doc.descendants((node) => {
if (node.type.name === 'image') originalAlt = node.attrs.alt
})
expect(originalAlt).toBe('photo')
})
})
@@ -12,3 +12,121 @@ export function extractImageFiles(transfer: DataTransfer | null): File[] {
.map((item) => item.getAsFile())
.filter((file): file is File => file !== null)
}
// `src` may be double-quoted, single-quoted, or (validly) unquoted per the HTML spec — the browser's
// own clipboard serialization always quotes it, but other producers of `text/html` are not obligated
// to.
const IMG_SRC_RE = /<img\b[^>]*\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'>]+))/gi
const INLINE_ROUTE_QUERY_KEYS = new Set(['key', 'fileId'])
/**
* True for the *display-layer* inline route `resolveImageSrc` (see `use-file-content-source.tsx`)
* rewrites an embed to — workspace-scoped `/api/workspaces/{workspaceId}/files/inline?key=…`/`?fileId=…`
* or public-share-scoped `/api/files/public/{token}/inline?key=…`/`?fileId=…`. This is the shape
* actually rendered into `<img src>`, and so what a same-page copy's `text/html` clipboard payload
* actually contains — NOT the raw stored reference `extractEmbeddedFileRef` (in
* `@/lib/uploads/utils/embedded-image-ref`) recognizes, which only matches the persisted `src` before
* that rewrite. Checked separately from (rather than folded into) `extractEmbeddedFileRef` since that
* helper is shared with server-side authorization/export code operating on persisted content, where
* this display-only shape should never legitimately appear.
*/
export function isInlineRouteSrc(src: string): boolean {
try {
const parsed = new URL(src, 'http://placeholder')
if (parsed.origin !== 'http://placeholder') return false
if (!parsed.pathname.endsWith('/inline')) return false
for (const key of parsed.searchParams.keys()) {
if (INLINE_ROUTE_QUERY_KEYS.has(key)) return true
}
return false
} catch {
return false
}
}
/**
* Extracts every `<img>` `src` value found in `html`, in document order (may contain duplicates).
*/
export function extractImgSrcs(html: string): string[] {
const srcs: string[] = []
for (const match of html.matchAll(IMG_SRC_RE)) {
const src = match[1] ?? match[2] ?? match[3]
if (src) srcs.push(src)
}
return srcs
}
/**
* True when `html` contains an `<img>` whose `src` is already one of our own hosted workspace file
* references. Copying a rendered `<img>` that's already on the page (e.g. Cmd+C after clicking it to
* select it) makes the browser put BOTH `text/html` (the real serialized node, with its real hosted
* `src`) AND a synthesized image `File` onto the clipboard — the same "drag a web image out" behavior
* that {@link extractImageFiles} alone can't tell apart from a genuinely new external image paste.
*/
export function hasHostedImageHtml(html: string, isHostedRef: (src: string) => boolean): boolean {
return extractImgSrcs(html).some((src) => isHostedRef(src) || isInlineRouteSrc(src))
}
/**
* True when a paste or drop should be diverted away from the upload-from-file path — it carries
* exactly one image file, and the accompanying `text/html` shows it's a same-page copy of an
* already-hosted image (see {@link hasHostedImageHtml}) rather than a genuinely new external image.
* Content-based (not `view.dragging`-based, for the drop case): `view.dragging` can go briefly stale
* (cleared up to ~50ms late by ProseMirror's own `dragend` handler when a prior internal drag was
* dropped outside this view) and must never suppress upload of an unrelated, genuinely new file that
* happens to land in that window — this check only reacts to what THIS specific event's `html`
* actually contains. Gated on exactly one file — a genuinely mixed paste/drop (the hosted image plus a
* separate new one) must still upload the new file, not have the whole paste/drop diverted.
*/
export function shouldSkipFileUpload(
images: File[],
html: string,
isHostedRef: (src: string) => boolean
): boolean {
return images.length === 1 && Boolean(html) && hasHostedImageHtml(html, isHostedRef)
}
/** Minimal shape of a ProseMirror image node — just enough to read its type name and attrs. */
interface ImageLikeNode {
type: { name: string }
attrs: Record<string, unknown>
}
/** Minimal shape of a ProseMirror doc — just enough to walk its nodes. */
interface DescendantsDoc {
descendants: (callback: (node: ImageLikeNode) => boolean | undefined) => void
}
/**
* Finds the first `image` node already in `doc` whose *rendered* src (`resolveImageSrc(node.attrs.src)`)
* matches one of `targetSrcs`, and returns its attrs — a defensive copy, safe to hand straight to
* `insertContentAt`. Returns `null` if no match is found (e.g. the source node was deleted, or this is
* genuinely a different document than the one the html was copied from).
*
* Used to clone a same-page copy/drag of an already-hosted image faithfully — the exact persisted
* `src` (and every other attribute: width, height, href, title…) — rather than re-deriving a node from
* the clipboard/dataTransfer `html`, whose `src` is `resolveImageSrc`'s rewritten *display* URL, not the
* real persisted one. Inserting a node built from that display URL would bake it into the document,
* which public share/export/referenced-by-doc tracking don't recognize (they only match the persisted
* shape) — this lookup avoids ever constructing such a node in the first place.
*/
export function findHostedImageAttrs(
doc: DescendantsDoc,
targetSrcs: string[],
resolveImageSrc: (src: string | undefined) => string | undefined
): Record<string, unknown> | null {
const targets = new Set(targetSrcs)
let found: Record<string, unknown> | null = null
doc.descendants((node) => {
if (found) return false
if (node.type.name === 'image') {
const resolved = resolveImageSrc(node.attrs.src as string | undefined)
if (resolved && targets.has(resolved)) {
found = { ...node.attrs }
return false
}
}
return true
})
return found
}
@@ -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('<a href="#">link</a>')
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: '<strong>bold</strong>', color: 'var(--text-primary)' },
{ tag: 'em', html: '<em>italic</em>', color: 'var(--text-primary)' },
{ tag: 'del', html: '<del>struck</del>', color: 'var(--text-tertiary)' },
{ tag: 's', html: '<s>struck</s>', color: 'var(--text-tertiary)' },
{ tag: 'code', html: '<code>code</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 (`<a><mark>text</mark></a>`) — link color wins', () => {
it.each([
{ tag: 'strong', html: '<a href="#"><strong>bold link</strong></a>' },
{ tag: 'em', html: '<a href="#"><em>italic link</em></a>' },
{ tag: 'del', html: '<a href="#"><del>struck link</del></a>' },
{ tag: 's', html: '<a href="#"><s>struck link</s></a>' },
{ tag: 'code', html: '<a href="#"><code>code link</code></a>' },
])('$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 (`<mark><a>text</a></mark>`) — link color still wins', () => {
it.each([
{ tag: 'strong', html: '<strong><a href="#">bold link</a></strong>' },
{ tag: 'em', html: '<em><a href="#">italic link</a></em>' },
{ tag: 'del', html: '<del><a href="#">struck link</a></del>' },
{ tag: 's', html: '<s><a href="#">struck link</a></s>' },
{ tag: 'code', html: '<code><a href="#">code link</a></code>' },
])('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('<a href="#"><strong>bold link</strong></a>')
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('<a href="#"><em>italic link</em></a>')
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(`<a href="#"><${tag}>struck link</${tag}></a>`)
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('<a href="#"><code>code link</code></a>')
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('<a href="#"><strong><em>bold italic link</em></strong></a>')
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(
'<a href="#"><strong><em><del>bold italic struck link</del></em></strong></a>'
)
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('<strong><em>bold italic, no link</em></strong>')
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('<p><a href="#">a link</a> and <strong>bold text</strong></p>')
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}><strong>bold</strong> <em>italic</em></${tag}>`)
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 <a>.
it('h6 keeps its dimmer --text-secondary, and bold/italic inside it inherit that tone (not --text-primary)', () => {
const root = mount('<h6><strong>bold</strong> <em>italic</em> plain</h6>')
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('<h6><code>code</code></h6>')
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('<h6><del>struck</del></h6>')
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('<h6><a href="#">linked heading</a></h6>')
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('<h6><a href="#"><strong><em>bold italic link</em></strong></a></h6>')
expect(colorOf(root.querySelector('em'))).toBe(LINK_COLOR)
expect(colorOf(root.querySelector('strong'))).toBe(LINK_COLOR)
})
})
@@ -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);
@@ -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<HTMLDivElement>(null)
const uploadFile = useUploadWorkspaceFile()
const editorInstanceRef = useRef<Editor | null>(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 `<img>` 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 `<img>` (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 `<img>`
* (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