Opened 6 years ago
Closed 5 years ago
#30196 closed Cleanup/optimization (fixed)
Make FileResponse always set Content-Disposition header.
Reported by: | Piotr Kunicki | Owned by: | Piotr Kunicki |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | FileResponse file response Content-Disposition header attachment inline |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
FileResponse currently sets the Content-Disposition header only if as_attachment is true.
Setting it explicitly to, e.g. 'inline; filename="example.png"' in the other case would allow the browser to set a default name for that inline file in case a user attempts to download it with the 'Save image as...' option.
That filename value is also visible in the title of the tab when image is being viewed directly in Firefox (at least v56).
Created a pull request: https://github.com/django/django/pull/11011
Change History (9)
comment:1 by , 6 years ago
Description: | modified (diff) |
---|
comment:2 by , 6 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 6 years ago
Sorry for beginner questions, but: as in, you want me to add some unit tests?
No problem, but wouldn't a single test be enough?
All this change does, after all, is setting Content-Disposition when as_attachment=False, so checking if it works shouldn't require more.
I also noticed there's no unit test checking if setting a custom filename works, so i added it to the same one test.
What else to add?
comment:4 by , 6 years ago
Needs tests: | unset |
---|
comment:5 by , 6 years ago
Easy pickings: | unset |
---|---|
Needs documentation: | set |
comment:6 by , 6 years ago
Needs documentation: | unset |
---|
Added documentation changes to the commit in PR.
comment:7 by , 6 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:8 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
The feature request makes sense but it's still missing tests.