Opened 16 years ago

Closed 16 years ago

Last modified 13 years ago

#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)

imagefieldfile.diff (1009 bytes ) - added by Victor Andrée 16 years ago.
Patch for ImageField

Download all attachments as: .zip

Change History (8)

by Victor Andrée, 16 years ago

Attachment: imagefieldfile.diff added

Patch for ImageField

comment:1 by Marty Alchin, 16 years ago

Owner: changed from nobody to Marty Alchin
Status: newassigned

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 Jacob, 16 years ago

milestone: 1.0

comment:3 by Julien Phalip, 16 years ago

See #8204 for a related issue.

comment:4 by Jacob, 16 years ago

Component: UncategorizedFile uploads/storage
Triage Stage: UnreviewedAccepted

comment:5 by Marty Alchin, 16 years ago

Needs documentation: set
Owner: Marty Alchin removed
Patch needs improvement: set
Status: assignednew

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 Jacob, 16 years ago

Resolution: wontfix
Status: newclosed

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.

comment:7 by Jacob, 13 years ago

milestone: 1.0

Milestone 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top