fix(media): force file download via fetch+blob instead of window.open (EVO-999)#44
Open
marcelogorutuba wants to merge 1 commit into
Open
fix(media): force file download via fetch+blob instead of window.open (EVO-999)#44marcelogorutuba wants to merge 1 commit into
marcelogorutuba wants to merge 1 commit into
Conversation
… (EVO-999) Replace openAttachmentInNewTab with fetch → blob → <a download> so that clicking the download button on a file attachment triggers a real download instead of opening the file in a new browser tab. Falls back to openAttachmentInNewTab only when fetch fails (e.g. CORS). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements a more robust file download mechanism for chat message attachments by replacing window.open-based behavior with a fetch-to-blob download flow, with a graceful fallback to the previous implementation when fetch fails, and cleans up an unused import. Sequence diagram for the new attachment download flowsequenceDiagram
actor User
participant MessageFileComponent
participant BrowserFetchAPI as BrowserFetchAPI
participant Server
participant BrowserDownload as BrowserDownload
participant FallbackWindowOpen as FallbackWindowOpen
User->>MessageFileComponent: click download button
MessageFileComponent->>MessageFileComponent: downloadFile(attachment)
MessageFileComponent->>MessageFileComponent: validate and trim url
alt url is empty
MessageFileComponent-->>User: no action
else url is valid
MessageFileComponent->>MessageFileComponent: resolve filename
MessageFileComponent->>BrowserFetchAPI: fetch(url, mode: cors)
alt fetch succeeds
BrowserFetchAPI->>Server: HTTP GET url
Server-->>BrowserFetchAPI: file response
BrowserFetchAPI-->>MessageFileComponent: Response
MessageFileComponent->>BrowserFetchAPI: response.blob()
BrowserFetchAPI-->>MessageFileComponent: Blob
MessageFileComponent->>BrowserDownload: URL.createObjectURL(blob)
MessageFileComponent->>BrowserDownload: create <a> with href and download
MessageFileComponent->>BrowserDownload: link.click()
BrowserDownload-->>User: file is downloaded
MessageFileComponent->>BrowserDownload: URL.revokeObjectURL(blobUrl)
else fetch throws error
MessageFileComponent->>FallbackWindowOpen: openAttachmentInNewTab(url, filename)
FallbackWindowOpen-->>User: open file in new tab or previous behavior
end
end
Flow diagram for the updated downloadFile logicflowchart TD
A[Start downloadFile with attachment] --> B[Extract and trim url from attachment.data_url]
B --> C{Is url non-empty?}
C -- No --> D[Return without action]
C -- Yes --> E[Determine filename from attachment.fallback_title or translation]
E --> F[Call fetch with url and mode cors]
F --> G{Did fetch succeed?}
G -- Yes --> H[Get blob from response]
H --> I[Create blobUrl with URL.createObjectURL]
I --> J[Create anchor element]
J --> K[Set anchor.href to blobUrl and anchor.download to filename]
K --> L[Trigger anchor.click to start download]
L --> M[Revoke blobUrl with URL.revokeObjectURL]
M --> N[End]
G -- No (throws) --> O[Call openAttachmentInNewTab with url and filename]
O --> N[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the new
downloadFileimplementation, consider checkingresponse.ok(and possibly status codes) before callingresponse.blob()so that HTTP errors don’t result in attempting to download an error page as a file. - You might want to move the
URL.revokeObjectURL(blobUrl)call into afinallyblock (after thelink.click()) to ensure the object URL is always cleaned up even if an error occurs after it is created.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the new `downloadFile` implementation, consider checking `response.ok` (and possibly status codes) before calling `response.blob()` so that HTTP errors don’t result in attempting to download an error page as a file.
- You might want to move the `URL.revokeObjectURL(blobUrl)` call into a `finally` block (after the `link.click()`) to ensure the object URL is always cleaned up even if an error occurs after it is created.
## Individual Comments
### Comment 1
<location path="src/components/chat/messages/MessageFile.tsx" line_range="30-31" />
<code_context>
+ link.download = filename;
+ link.click();
+ URL.revokeObjectURL(blobUrl);
+ } catch {
+ openAttachmentInNewTab({ url, filename });
}
};
</code_context>
<issue_to_address>
**question:** Consider whether to restore user feedback for the download fallback path.
Previously, when `openAttachmentInNewTab` fell back to a download flow, we showed a `downloadStarted` toast. With this change, that feedback is lost if the direct fetch fails and we call `openAttachmentInNewTab`. If you want to preserve that behavior, consider either propagating a signal from `openAttachmentInNewTab` or showing a generic info toast in this catch branch.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+30
to
+31
| } catch { | ||
| openAttachmentInNewTab({ url, filename }); |
There was a problem hiding this comment.
question: Consider whether to restore user feedback for the download fallback path.
Previously, when openAttachmentInNewTab fell back to a download flow, we showed a downloadStarted toast. With this change, that feedback is lost if the direct fetch fails and we call openAttachmentInNewTab. If you want to preserve that behavior, consider either propagating a signal from openAttachmentInNewTab or showing a generic info toast in this catch branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MessageFile.tsx: replacedopenAttachmentInNewTab(which usedwindow.open) withfetch → blob → <a download>so that clicking the download button triggers a real file download instead of opening the file in a new browser tabopenAttachmentInNewTabonly whenfetchfails (e.g. CORS restriction)toastimportValidation
evo-ai-frontend-community: pnpm exec tsc -b --noEmit→ no errorsChanged Files
src/components/chat/messages/MessageFile.tsxRelated PRs
Linked Issue
Summary by Sourcery
Ensure chat message file attachments are downloaded as files instead of being opened in a new browser tab, with a graceful fallback when direct download is not possible.
Bug Fixes:
Enhancements: