Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#8208 closed (wontfix)

ImageField does not set width/height_field using 'actual' file

Reported by: vicvicvic Owned by:
Component: File uploads/storage Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

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 vicvicvic 7 years ago.
Patch for ImageField

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by vicvicvic

Patch for ImageField

comment:1 Changed 7 years ago by Gulopine

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to Gulopine
  • Patch needs improvement unset
  • Status changed from new to 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 Changed 7 years ago by jacob

  • milestone set to 1.0

comment:3 Changed 7 years ago by julien

See #8204 for a related issue.

comment:4 Changed 7 years ago by jacob

  • Component changed from Uncategorized to File uploads/storage
  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 7 years ago by Gulopine

  • Needs documentation set
  • Owner Gulopine deleted
  • Patch needs improvement set
  • Status changed from assigned to 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 Changed 7 years ago by jacob

  • Resolution set to wontfix
  • Status changed from new to 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.

comment:7 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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