Make download directory configurable#145
Conversation
|
@codex pls review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c51b1d2cb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const stored = await api.storage.local.get(DOWNLOAD_DIRECTORY_STORAGE_KEY); | ||
| const directory = normalizeDownloadDirectory(stored?.[DOWNLOAD_DIRECTORY_STORAGE_KEY]); | ||
| if (!directory) return original; | ||
| const candidate = original || filenameFromDownloadUrl(url) || 'download'; |
There was a problem hiding this comment.
When a Firefox user has downloadDirectory set and a download call does not supply an explicit filename, this line always fabricates one from the URL or download. For download_files/download_resource_from_page URLs such as /export?id=1 that rely on Content-Disposition for the real name and extension, passing this filename to browser.downloads.download bypasses Firefox's normal filename selection and saves files as Work/WebBrain/export or Work/WebBrain/download instead of the server-provided filename.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15472fc13a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const suggestedFilename = (i === 0 ? singleFilename : null) | ||
| || await discoverHttpDownloadFilename(url); |
There was a problem hiding this comment.
In Firefox's download_files path, when the new setting is blank, filenameInConfiguredDownloadDirectory() returns suggestedFilename unchanged, so this line now performs a credentialed HEAD request and then sets opts.filename even though the user did not configure a subdirectory. For endpoints such as /export?id=... where HEAD omits Content-Disposition but GET supplies it, this overrides the browser's server-selected filename with the URL basename, and one-shot/signed URLs also see an extra request. Only discover and supply a filename when a configured directory actually needs a basename, or when the user supplied args.filename.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e964d5350
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return filenameFromContentDisposition(response.headers?.get?.('content-disposition')) | ||
| || filenameFromHttpUrl(responseUrl); |
There was a problem hiding this comment.
When a configured download directory is set, a URL whose HEAD request succeeds without Content-Disposition falls back to filenameFromHttpUrl(responseUrl) and then passes that path to downloads.download. In that case Firefox no longer gets to choose the filename from the actual GET response, so endpoints that only send Content-Disposition on GET (or otherwise pick a better filename during the real download) are saved under a generic URL basename such as WebBrain/download; leave filename undefined unless a real filename was supplied or discovered from the response headers.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ae8546a27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const response = await fetch(target, { | ||
| method: 'HEAD', | ||
| credentials: 'include', |
There was a problem hiding this comment.
When a Firefox user has a download directory configured, URLs without an explicit filename now make this authenticated HEAD request before the real browser.downloads.download call. For signed export/download links that are valid for a single request or mark the token used on first access, this probe can consume the URL and make the subsequent GET fail even though the same download worked before the directory setting was enabled.
Useful? React with 👍 / 👎.
| const basename = String(filename ?? '') | ||
| .replace(/\\/g, '/') | ||
| .split('/') | ||
| .pop() | ||
| .replace(/[\u0000-\u001f\u007f]/g, '') | ||
| .trim(); |
There was a problem hiding this comment.
With a configured Firefox download directory, server-provided Content-Disposition names are passed through this helper and then supplied as an explicit downloads.download({ filename }); unlike the browser’s normal filename choice, the WebExtension API rejects explicit filenames containing characters such as :, ?, quotes, or leading dots. A common header like filename="Q1: report.pdf" therefore makes the download fail only when the directory setting is enabled, instead of letting Firefox sanitize the suggested basename.
Useful? React with 👍 / 👎.
|
@codex pls review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
downloads.download({ filename })call sitesWhy
Users currently cannot choose where WebBrain-created downloads are grouped. This adds an optional relative subdirectory without overriding the browser or operating system download root.
Firefox does not expose Chrome's filename-determination event, so its implementation applies the configured path before each download call instead of exposing a setting that would have no effect.
Impact
Existing behavior is unchanged unless a user enters a relative subdirectory. Clearing the setting restores the system default behavior.
Validation
npm test— 884/884 unit tests passedgit diff --check