feat(web): attach images dragged from another browser tab#2108
feat(web): attach images dragged from another browser tab#2108roni-estein wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
When a user drags an image straight from another browser tab (e.g. the ChatGPT web UI) onto the composer, browsers hand the destination `text/uri-list` / `text/html` with the image URL — not a real `File`. The composer's drop handler only accepted `DataTransfer.files`, so nothing happened. This fetches the URL as an image and feeds it through the existing attachment pipeline. Finder drops still work; plain-text drags are still left to the Lexical editor. - `isComposerAttachmentDrag` / `extractDraggedImageUrls` / `deriveImageFilenameFromUrl` are pure helpers in `composer-logic.ts`. - `fetchDroppedImageAsFile` (`lib/fetchDroppedImage.ts`) downloads the URL, explicitly checks `response.ok` and the blob's image MIME type, and returns `null` on any failure so one bad URL can't poison a batch. - The drop handler preserves the existing sync path for real files and falls back to an async URL fetch only when `dataTransfer.files` is empty. A single toast is shown when image-shaped URLs were present but none could be fetched (typically CORS). Tests cover uri-list/html extraction, dedup, RFC 2483 comment lines, data:/blob: handling, MIME-based filename derivation, and every `fetchDroppedImageAsFile` failure mode (network, non-2xx, non-image, body read error).
Cursor Bugbot caught the `|| "png"` on `const ext = subtype || "png"` being dead code: `subtype` already had the same fallback on the line above, so `ext === subtype` always. This is the same "redundant fallback that reads defensive but isn't" shape worth remembering for future reviews. Also moved the extension lookup below the "already has an extension" early return — we only need an inferred extension when we're about to append one. No behavior change. Existing tests still cover both branches.
65ba5eb to
68e330f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 68e330f. Configure here.
ApprovabilityVerdict: Needs human review This PR introduces a new user-facing feature (drag-and-drop images from browser tabs) with ~400 lines of new logic for URL extraction, async fetching, and error handling. Additionally, there are two unresolved review comments identifying potential bugs in HTML entity decoding and URI error handling that could cause the feature to fail silently. You can customize Macroscope's approvability policy. Learn more. |

What Changed
Dropping an image straight from another browser tab onto the composer now works. Today, dragging an image from e.g. the ChatGPT web UI onto the composer silently does nothing — you have to drop the image into Finder first, then drop the file from Finder into T3 Code.
Why
The composer's drop handler in
ChatComposer.tsxshort-circuits on the very first line of every drag handler:That gate is correct for Finder drops (which expose a real
File), but wrong for cross-tab browser drags. When a browser is the drag source, it puts the image intoDataTransferastext/uri-list+text/html(an<img src="…">). There is noFile— the destination has to fetch the bytes itself.The drop zone never even highlighted, so the user couldn't tell whether the gesture was recognized.
Why not existing PRs
dataTransfer.files.Neither touches the "Files" gate, and neither helps a drag that contains no
Fileat all. The two surfaces are complementary.How it works
Three small pure helpers in
composer-logic.ts, one async helper inlib/fetchDroppedImage.ts, and a ~30-line change to the existing drop handler.isComposerAttachmentDrag(types)— acceptsFiles(as before) ortext/uri-list. Gating ontext/uri-listrather than any text type is deliberate: plain-text drags (e.g. selecting and dragging text from another input) don't includetext/uri-list, so the Lexical editor's native text-drop behavior is preserved.extractDraggedImageUrls(dataTransfer)— extracts candidate URLs in priority order:text/uri-list(the standard; strips#comment lines per RFC 2483)text/html(<img src>parsed for the URL)text/plain(last-resort fallback, only used if the two above yielded nothing)Only
http(s):,data:image/*, andblob:URLs pass — everything else (file:,about:, random text) is dropped so the caller can safelyfetch()the result.deriveImageFilenameFromUrl(url, mime)— strips query strings, takes the last path segment, falls back todropped-image.<ext>when the URL has no usable name, and infers the extension from the MIME type when missing.fetchDroppedImageAsFile(url)— downloads the URL and returns it as aFilesuitable for the existing attachment pipeline. Explicitly handles every failure mode (network/CORS reject, non-2xx status, non-image MIME, body read error) by returningnullso a single bad URL can't poison a batch.Drop handler update — when
dataTransfer.filesis empty but image-shaped URLs are present, fetch them asynchronously and feed the results through the existingaddComposerImages(...)path. If every fetch fails (typically a CORS block on the source host), a single toast tells the user to fall back to the Finder round-trip that already works.Order of preservation:
text/uri-list, handler bails).Defensive coding notes
This PR was written with explicit attention to three patterns that have bitten me (and been caught by bots) on recent PRs:
text/plainfallback only fires when bothtext/uri-listandtext/htmlproduced zero URLs. There's a dedicated test (does not fall back to text/plain when uri-list/html already yielded URLs) that would fail if the guard regressed to something likeif (uris.length === 0 || ...).fetchDroppedImageAsFileexplicitly checksresponse.ok(fetch resolves on 4xx/5xx; treating a 403 HTML error page as image bytes would otherwise wrap it as a "file"), explicitly checksblob.type.startsWith("image/"), and the caller gates the toast +addComposerImagesonfetched.length > 0. A dedicated test covers the 403 case.Tests
apps/web/src/composer-logic.test.ts— new coverage for all three pure helpers:#comment lines<img src>extraction from html, both"and'quoteddata:image/andblob:URL acceptancefile:,about:, anddata:text/plainapps/web/src/lib/fetchDroppedImage.test.ts— all four failure modes covered:Full suite: 897/897 tests pass.
UI Changes
None when no image is dragged. New behavior only activates when
dataTransfer.filesis empty and image-shaped URLs are present.Error path (image URL but fetch blocked, typically CORS on the source host):
Validation
bun fmt— cleanbun lint— zero new warnings from the changed files (pre-existing warnings unrelated)bun typecheck— clean across@t3tools/scripts,@t3tools/web,@t3tools/desktop, rootbun run test— 897/897Checklist
Note
Medium Risk
Adds new drag/drop behavior that triggers client-side
fetch()es of dropped URLs and alters event gating, which could introduce edge-case UX regressions or unexpected network requests (though failures are handled and surfaced to users).Overview
Enables attaching images by dragging them from another browser tab into the chat composer, not just dropping local files.
Updates composer drag handlers to recognize URL-based drags (
text/uri-list), extract image URLs fromDataTransfer, and asynchronously download them intoFiles via a newfetchDroppedImageAsFilehelper; if all downloads fail (commonly due to CORS), the user gets an error toast.Adds tested helpers in
composer-logic.tsfor drag-type gating, URL extraction/deduping, and deriving filenames from URLs, plus unit tests for the new fetch helper’s success and failure modes.Reviewed by Cursor Bugbot for commit 68e330f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Support attaching images dragged from another browser tab in the chat composer
onComposerDropin ChatComposer.tsx to handle URL-based drags (e.g. dragging an image from another tab) in addition to file drags.extractDraggedImageUrlsto parse image URLs fromtext/uri-list,text/html<img src>, andtext/plainin the drag payload, andisComposerAttachmentDragto gate drag-over highlighting on bothFilesandtext/uri-listdrags.fetchDroppedImageAsFilein lib/fetchDroppedImage.ts to fetch each extracted URL and convert it to aFile; non-image, failed, or non-OK responses returnnull.dropEffectnow activates fortext/uri-listdrags but not for plain-text drags.Macroscope summarized 68e330f.