#8208 closed (wontfix)
ImageField does not set width/height_field using 'actual' file
Reported by: | Victor Andrée | Owned by: | |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
As mentioned in django-developers, models.ImageField
(actually ImageFieldFile.save
) updates width/height_field using the 'original' (typically uploaded) file, not the one that gets saved. This means that if you don't save the same file as the one being uploaded (because you have resized it), width and height are incorrect.
The code to set width/height is in ImageFieldFile.save
, and happens before the file is saved.
I'm attaching a patch which moves this code below the save call and uses the final file to find the dimensions. I don't know much about Django internals so it might need polishing, then again, it really just moves some code down a few lines and changes one function call.
Attachments (1)
Change History (8)
by , 16 years ago
Attachment: | imagefieldfile.diff added |
---|
comment:1 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Good catch on this one. This definitely needs fixing, but I'm not sure I like the approach used here. I've got another idea in mind that I'll work on a bit today and see if I can get a new patch up tonight.
comment:2 by , 16 years ago
milestone: | → 1.0 |
---|
comment:4 by , 16 years ago
Component: | Uncategorized → File uploads/storage |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:5 by , 16 years ago
Needs documentation: | set |
---|---|
Owner: | removed |
Patch needs improvement: | set |
Status: | assigned → new |
As I mentioned in the thread mentioned in this ticket's description, I think the best way to approach this is to subclass FieldFile
(or ImageFieldFile
, etc) and do your transformations there, before they get passed to the Storage
object. That way, you have access to both the before and after and you could pick which one you want to use when updating information. It might still be worth explaining this stance a bit better in the docs, but I'm not sure I have time to write it at the moment. I'll see what I can do, but if somebody else wants to take a stab at some documentation for this, feel free.
comment:6 by , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Yeah, this patch won't work as-s -- try applying it and running the file_storage
tests to see why. In a nutshell, we need to set the sizes before calling save()
so that width_field
and height_field
work correctly. So Marty's right that if you monkey with the file, you need to do it in some other way.
Marty, feel free to open another ticket for the docs if you get a chance to write 'em.
Patch for ImageField