[webview_flutter_android] Add headers to NavigationRequest to tell if it isDownloadRequest from onDownloadStart#11502
[webview_flutter_android] Add headers to NavigationRequest to tell if it isDownloadRequest from onDownloadStart#11502wuzhengu wants to merge 2 commits intoflutter:mainfrom
Conversation
… it isDownloadRequest from onDownloadStart
There was a problem hiding this comment.
Code Review
This pull request adds a headers field to the NavigationRequest class and introduces an isDownloadRequest property derived from the content-disposition header. The Android implementation is updated to populate these headers during download events. Feedback indicates that deriving isDownloadRequest from headers creates a fragile dependency and that the logic prepending 'attachment; ' to the content-disposition header may result in invalid header values; it is recommended to use an explicit boolean field instead.
|
The Gemini review is correct; this is not how we would implement this. Also, per https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-support, the first step here would be investigating whether there is a way to support this on iOS, as we prefer not to add platform-specific features when the behavior is something that would apply to other platforms. If you would like to move forward with this, the next step would be to evaluate how this could work on iOS. |
|
Thanks for the feedback. I’d like to clarify a few key points about the implementation:
In short, the modification is safe because it applies only to download-triggered requests, where Content-Disposition is already attachment or empty. It achieves the intended functionality with the smallest possible change, while also providing a foundation for eventual cross‑platform unification. |
I was referring to the other part of the Gemini review, about modifying headers to pass around magic strings.
This is not related to anything in my comment, so I'm not sure why you've included this section.
This is also not responsive to my comment.
You (and/or your AI agent) are certainly welcome to that opinion, but we have a documented policy on this for a reason. If you would like to move forward with this PR, just stating an opinion that our policy is wrong is not productive.
Our bar for contribution does not change based on the age of the issue. |
Add
headersandisDownloadRequesttoNavigationRequest,so we can easily tell if it was made to download an attachment fromonDownloadStart.List which issues are fixed by this PR. You must list at least one issue.
132738
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2