Skip to content

fix(rich-markdown-editor): make drag-reorder of an image actually move it, on real browser payloads#5617

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix-image-drag-move
Jul 12, 2026
Merged

fix(rich-markdown-editor): make drag-reorder of an image actually move it, on real browser payloads#5617
waleedlatif1 merged 2 commits into
stagingfrom
fix-image-drag-move

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to fix(files): image drag-reorder move + unwedge post-stream editor + bullet/image Backspace fixes #5608 (this commit was pushed after that PR merged, so it needs its own PR). 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 ring 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 pinned in source: TipTap's node-view dragstart bypasses ProseMirror's drag serialization entirely (verified in @tiptap/core — no PM text/html, no view.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: 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: origin-aware src normalization (toSameOriginPath — deliberately window.location.origin, not getBaseUrl(): the browser serializes against the origin the page is actually viewed on, which legitimately diverges from NEXT_PUBLIC_APP_URL on localhost/previews/apex-vs-www; cross-origin srcs are never treated as ours), applied to isInlineRouteSrc/hasHostedImageHtml/findHostedImageAttrs (the paste-clone path had the same absolute-URL gap for browser-native "Copy Image"). handleDrop now 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

  • Bug fix

Testing

  • Real-Chromium harness driving the real editor + engine: drag-reorder before/after (duplicate + no move → exactly one clean move, stays selected, one save), real-mouse micro-drag gesture proof, external file drop still uploads+inserts (including with an image selected at drop time — the move branch must not hijack it), paste-clone for relative (PM copy) and absolute (native "Copy Image") payloads with zero uploads, and the fix(files): image drag-reorder move + unwedge post-stream editor + bullet/image Backspace fixes #5608 regression battery (reconcile unwedge, click/focus/save-cycle selection) all green on this exact branch.
  • 604 unit tests pass (11 new for the origin-aware helpers, verified failing pre-fix). tsc, biome, api-validation clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…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.
@vercel

vercel Bot commented Jul 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 12, 2026 3:00am

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor

cursor Bot commented Jul 12, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes editor drop/paste image handling and persisted src shapes; behavior is localized to the markdown file viewer but affects save/share tracking if regressions slip through.

Overview
Fixes image drag-reorder in the rich markdown editor when TipTap node-view drags bypass ProseMirror serialization: drops carried browser-native absolute <img src> URLs and synthetic image files, so hosted-image detection failed and the editor duplicated instead of moving.

Adds toSameOriginPath (using window.location.origin, not configured app URL) and threads it through isInlineRouteSrc, hasHostedImageHtml, and findHostedImageAttrs so paste/copy paths match absolute same-origin payloads. Adds htmlReferencesSrc to detect when drop HTML refers to the currently selected image (absolute URL identity, including cross-origin CDN images for reorder).

handleDrop now performs an explicit ProseMirror-style move (delete → map position → insert, keep NodeSelection) when that identity check passes; otherwise existing upload/skip behavior is unchanged.

Reviewed by Cursor Bugbot for commit 29fd4cb. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes image drag and paste handling in the rich markdown editor. The main changes are:

  • Adds same-origin URL normalization for browser-native image payloads.
  • Matches dragged images by resolved absolute URL, including external image URLs.
  • Moves selected image nodes manually on drop with invalid drop targets handled as no-ops.
  • Adds helper tests for absolute, relative, same-origin, and external image URL cases.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx Adds the selected-image drop move path and handles invalid drop targets without mutating the document.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts Adds URL normalization helpers and separates hosted-image recognition from dragged-image identity checks.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts Adds tests for same-origin absolute URLs, hosted image matching, and external image identity matching.

Reviews (3): Last reviewed commit: "fix(rich-markdown-editor): no-op invalid..." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes image paste and drag behavior in the rich markdown editor. The main changes are:

  • Same-origin absolute image URLs are normalized before hosted-image checks.
  • Hosted-image paste and copy detection now handles browser-native absolute payloads.
  • Selected image drag/drop now tries to move the existing image node directly.
  • Tests cover absolute same-origin URL matching and selected-image HTML references.

Confidence Score: 4/5

The selected-image drop path can crash on invalid image drop positions.

  • URL normalization is conservative for cross-origin and malformed image sources.
  • The custom move branch uses dropPoint, but then ignores its null result and inserts at an unchecked coordinate.
  • Invalid editor drop targets can throw during tr.insert instead of being ignored.

rich-markdown-editor.tsx

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.ts Adds same-origin URL normalization and applies it to hosted-image detection and node matching.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx Adds a custom selected-image move path, with one invalid-position case that can throw during drop handling.
apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/image-paste.test.ts Adds coverage for absolute same-origin image URLs and selected-image HTML reference checks.

Reviews (2): Last reviewed commit: "fix(rich-markdown-editor): make drag-reo..." | Re-trigger Greptile

…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).
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@waleedlatif1 waleedlatif1 merged commit f477faa into staging Jul 12, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the fix-image-drag-move branch July 12, 2026 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant