Cropping informations are not cleared after an image change

[laulaz]

Steps to reproduce :

1. Add an image on a content
2. Crop this image with plone.app.imagecropping cropping editor
3. Change the image for something else
4. The croppings informations are not cleared, and the resulting scale(s) are still cropped on the new image (on a "random" area, depending on the old image)

Of course if the cropped scale is shown on your content view, you can see the problem directly (and re-crop the image manually), but if it is not the case (ex: scales used elsewhere), you can get very awkward results !

We should IMHO use a subscriber to get changed image fields and clear the cropping annotation storage for that fields when this happens.

@petschki @MrTango what do you think ?

Demo with plone.app.imagecropping 3.0.3 :

20231025_032704937.mp4

[MrTango]

Good point it would better, if this was reset. We have the events already, so it's probably not to hard to fix this.

[laulaz]

Good point it would better, if this was reset. We have the events already, so it's probably not to hard to fix this.

Are you talking about ICroppingInfoChangedEvent ? this will not help us, as we need to subscribe to an object/field change event and check if an image field was changed ... or maybe I missed something here ?

I have not searched if we have a useful event for images field changes.

For now, in a customer project, we have uggly "generic" code working. It subscribes to IObjectModifiedEvent (modified_content function), then checks for fields in the event.descriptions to see if some fields are NamedBlobImage and, if so, clear all the related cropped scales it finds.

from plone.app.imagecropping import PAI_STORAGE_KEY
from plone.app.imagecropping.storage import Storage
from plone.namedfile.field import NamedBlobImage
from zope.annotation.interfaces import IAnnotations
from zope.lifecycleevent.interfaces import IAttributes
from zope.schema import getFields

import logging
logger = logging.getLogger("our.package")


def remove_cropping(obj, field_name, scales):
    storage = Storage(obj)
    for scale in scales:
        storage.remove(field_name, scale)


def check_image_field(obj, interface, class_name, field_name):
    """Check if the field is an image field to see if we have cropping
    information to remove"""
    if field_name.startswith(f"{class_name}."):
        field_name = field_name.split(".")[-1]
    fields = getFields(interface)
    if field_name not in fields:
        # strange! the field is not found in schema class
        logger.warning(f"{field_name} not found in {interface.__identifier__}")
        return
    field = fields[field_name]
    if not isinstance(field, NamedBlobImage):
        # the field is not an image, nothing to do
        return
    annotations = IAnnotations(obj)
    scales = annotations.get(PAI_STORAGE_KEY)
    scales_to_purge = []
    for scale in scales:
        if scale.startswith(field_name):
            scale_suffix = scale.split(f"{field_name}_")[-1]
            scales_to_purge.append(scale_suffix)
    if scales_to_purge:
        remove_cropping(obj, field_name, scales_to_purge)
        logger.info(f"{scales_to_purge} croppings were PURGED for {field_name} !")
    else:
        logger.info(f"No existing cropping to purge for {field_name}.")


def modified_content(obj, event):
    if not hasattr(event, "descriptions") or not event.descriptions:
        return
    for d in event.descriptions:
        if not IAttributes.providedBy(d):
            # we do not have fields change description, but maybe a request
            continue

        class_name = d.interface.__identifier__.split(".")[-1]
        for field_name in d.attributes:
            check_image_field(obj, d.interface, class_name, field_name)

[petschki]

Since plone.namedfile==6.2.0 theres a new internal _modified on the image field which can be checked if it has changed. See plone/plone.namedfile#150 ...

[laulaz]

Since plone.namedfile==6.2.0 theres a new internal _modified on the image field which can be checked if it has changed. See plone/plone.namedfile#150 ...

Would have been great to have an event there, to avoid having to parse all fields 😄