-
Notifications
You must be signed in to change notification settings - Fork 15.8k
fix(app): prefer dropped files over file URIs #19994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { describe, expect, test } from "bun:test" | ||
| import { getDroppedPromptData } from "./drop" | ||
|
|
||
| describe("getDroppedPromptData", () => { | ||
| test("prefers dropped files over file uri text", () => { | ||
| const file = new File(["png"], "drop.png", { type: "image/png" }) | ||
| const dataTransfer = { | ||
| files: [file] as unknown as FileList, | ||
| getData: () => "file:/tmp/drop.png", | ||
| } | ||
|
|
||
| const result = getDroppedPromptData(dataTransfer) | ||
| expect(result.files).toEqual([file]) | ||
| expect(result.filePath).toBeUndefined() | ||
| }) | ||
|
|
||
| test("falls back to file uri text when no files are present", () => { | ||
| const dataTransfer = { | ||
| files: [] as unknown as FileList, | ||
| getData: () => "file:/tmp/drop.png", | ||
| } | ||
|
|
||
| expect(getDroppedPromptData(dataTransfer)).toEqual({ | ||
| files: [], | ||
| filePath: "/tmp/drop.png", | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,17 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type DropDataTransfer = Pick<DataTransfer, "files" | "getData"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function getDroppedPromptData(dataTransfer: DropDataTransfer | null | undefined): { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| files: File[] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filePath?: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const files = Array.from(dataTransfer?.files ?? []) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (files.length > 0) return { files } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const plainText = dataTransfer?.getData("text/plain") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const filePrefix = "file:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (plainText?.startsWith(filePrefix)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { files: [], filePath: plainText.slice(filePrefix.length) } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { files: [], filePath: plainText.slice(filePrefix.length) } | |
| let filePath: string | |
| // Handle standards-compliant file:// URIs (e.g. file:///tmp/a%20b.txt, file://server/share/a.txt) | |
| if (plainText.startsWith("file://")) { | |
| try { | |
| const url = new URL(plainText) | |
| const decodedPath = decodeURIComponent(url.pathname) | |
| if (url.host) { | |
| // UNC-style path: //server/share/path | |
| filePath = `//${url.host}${decodedPath}` | |
| } else { | |
| // Local path: /tmp/a b.txt | |
| filePath = decodedPath | |
| } | |
| } catch { | |
| // Fallback to legacy behavior if URL parsing fails | |
| filePath = plainText.slice(filePrefix.length) | |
| } | |
| } else { | |
| // Internal drag format: file:${path} | |
| filePath = plainText.slice(filePrefix.length) | |
| } | |
| return { files: [], filePath } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current unit coverage only exercises the custom
file:/tmp/...style string. If this helper is intended to support real file URIs from the browser (file:///..., encoded characters, UNC/host forms), adding explicit test cases for those formats would help prevent regressions once parsing is corrected.