Commit 5de66db
authored
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.1 parent fac8ec0 commit 5de66db
5 files changed
Lines changed: 670 additions & 6 deletions
File tree
- apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor
Lines changed: 236 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
5 | | - | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
6 | 34 | | |
7 | 35 | | |
8 | 36 | | |
| |||
54 | 82 | | |
55 | 83 | | |
56 | 84 | | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
Lines changed: 118 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
0 commit comments