Code

Opened 8 years ago

Closed 7 years ago

#1537 closed defect (fixed)

ImageField height_field and width_field does not update after changing the image

Reported by: django@… Owned by: adrian
Component: Core (Other) Version: magic-removal
Severity: major Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Steps to reproduce the bug

  1. Create a model with an ImageField img, specifying height_field and width_field options
  1. Upload an image through Admin UI
  1. Check model_instance.img_width and model_instance.img_height
  1. Update the same object with another image of a different dimension through Admin UI
  1. Note that model_instance.img_width and model_instance.img_height remain the same

Commenting out the following code from django.core.meta.fields.ImageField seems to have solved the problem, but not sure if I'm breaking something else:

    def save_file(self, new_data, new_object, original_object, change, rel):
        FileField.save_file(self, new_data, new_object, original_object, change, rel)
        # If the image has height and/or width field(s) and they haven't
        # changed, set the width and/or height field(s) back to their original
        # values.
        if change and (self.width_field or self.height_field):
            """
            if self.width_field:
                setattr(new_object, self.width_field, getattr(original_object, self.width_field))
            if self.height_field:
                setattr(new_object, self.height_field, getattr(original_object, self.height_field))
            """
            new_object.save()

Attachments (1)

meta_field_ImageField.diff (1.1 KB) - added by django@… 8 years ago.
Patch for core/meta/fields.py to correct ImageField dimension db updating

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by stephen@…

  • Version changed from 0.91 to magic-removal

the solution above sets the image width and height values to None if no change is made to the image when updating.

The following code (tested only on magic-removal) appears to work for all cases:

    def save_file(self, new_data, new_object, original_object, change, rel):
        FileField.save_file(self, new_data, new_object, original_object, change, rel)
        # get original image dimensions
        original_img_width, original_img_height = 0, 0
        if original_object != None:
            if self.width_field or self.height_field:
                if self.width_field:
                    original_img_width = getattr(original_object, self.width_field)
                if self.height_field:
                    original_img_height = getattr(original_object, self.height_field)
        if self.width_field or self.height_field:
            if self.width_field:
                new_img_width = getattr(new_object, self.width_field)
            if self.height_field:
                new_img_height = getattr(new_object, self.height_field)
            if new_img_width != None or new_img_height != None:
                # change to new values
                if self.width_field and new_img_width != None:
                    setattr(new_object, self.width_field, new_img_width)
                if self.height_field and new_img_height != None:
                    setattr(new_object, self.height_field, new_img_height)
            else:
                # If the image has height and/or width field(s) and they haven't
                # changed, set the width and/or height field(s) back to their original
                # values.
                if self.width_field:
                    setattr(new_object, self.width_field, original_img_width)
                if self.height_field:
                    setattr(new_object, self.height_field, original_img_height)
                                
            new_object.save()

comment:2 Changed 8 years ago by anonymous

ping..
can a committer have a look at this ?

comment:3 Changed 8 years ago by django@…

Testing in trunk Rev#2748, it seems that new_object always has height and width. I can't seem to find a test case where commenting out the two if blocks do not work.

comment:4 Changed 8 years ago by django@…

Actually why is the override even necessary for class ImageField ? Since method_save_file() in meta/__init__ already takes care of setting the width and height in case of a image replacement.

In the case where the image is not updated, there's no reason why the width and height would be lost.

I believe the save_file() override should be removed completely for ImageField .

Changed 8 years ago by django@…

Patch for core/meta/fields.py to correct ImageField dimension db updating

comment:5 Changed 8 years ago by django@…

  • Summary changed from ImageField height_field and width_field does not update after changing the image to [patch] ImageField height_field and width_field does not update after changing the image

comment:6 Changed 8 years ago by stephen@…

I can't seem to find a test case where commenting out the two if blocks do not work.

Maybe it's different in trunk but when testing in magic-removal Rev#2750. the image dimension fields are set to None if you update a model containing an ImageField without updating the image (eg. changing the image description field).

the code below (slightly refactored from my first attempt) works in all cases:


  def save_file(self, new_data, new_object, original_object, change, rel):
        FileField.save_file(self, new_data, new_object, original_object, change, rel)
        # get original image dimensions
        if self.width_field or self.height_field:
            if self.width_field:
                new_img_width = getattr(new_object, self.width_field)
            if self.height_field:
                new_img_height = getattr(new_object, self.height_field)
            if new_img_width != None or new_img_height != None:
                # change to new values
                if self.width_field and new_img_width != None:
                    setattr(new_object, self.width_field, new_img_width)
                if self.height_field and new_img_height != None:
                    setattr(new_object, self.height_field, new_img_height)
            else:
                # If the image has height and/or width field(s) and they haven't
                # changed, set the width and/or height field(s) back to their original
                # values.
                if original_object != None:
                    if self.width_field:
                        setattr(new_object, self.width_field, getattr(original_object, self.width_field))
                    if self.height_field:
                        setattr(new_object, self.height_field, getattr(original_object, self.height_field))
                                
            new_object.save()

comment:7 Changed 8 years ago by django@…

I will revisit this issue once I've had some time with MR.

comment:8 Changed 8 years ago by jacob

  • Summary changed from [patch] ImageField height_field and width_field does not update after changing the image to ImageField height_field and width_field does not update after changing the image

Removing the [patch] label since this given patch no longer works (and may not have been valid in the first place).

comment:9 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

AFAICT this was fixed by [4609].

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.