ImageField height_field and width_field does not update after changing the image

[django@…]

Steps to reproduce the bug

1. Create a model with an ImageField img, specifying height_field and width_field options

2. Upload an image through Admin UI

3. Check model_instance.img_width and model_instance.img_height

4. Update the same object with another image of a different dimension through Admin UI

5. 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()

[stephen@…]

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

[anonymous]

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

[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.

[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 .

[django@…]

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

[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()

[django@…]

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

[Jacob]

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

[Jacob]

AFAICT this was fixed by [4609].