Image crops lost when editing image title

[frisi]

crops get invalidated when the image title/description or other metadata is changed.

this should only happen if the image file itself gets changed.

to reproduce:

• upload image
• define a crop for a scale (eg large) using @@croppingeditor
• include the image in a page template using plone.scale

<img tal:define="scales image_object/@@images;
                         img python:scales.scale('image', scale='large')"
         tal:attributes="src img/url" />

• template shows the right crop
• edit the image and change description and/or title
• view the template again and the image will be scaled instead of cropped

it's important to mention that this only happens for scales.scale and not for the plone.app.imaging traverser:

test1.jpg/@@images/8689307d-f3e7-42c5-9134-f77f6e704cfe.jpeg will show the scaled image

test1.jpg/image_frontpage will still show the cropped image

did not yet investigate if this bug belongs to imagecropping or plone.scale or plone.app.imaging

[petschki]

it's important to mention that this only happens for scales.scale and not for the plone.app.imaging traverser:

test1.jpg/@@images/8689307d-f3e7-42c5-9134-f77f6e704cfe.jpeg will show the scaled image

test1.jpg/image_frontpage will still show the cropped image

plone.scale rerenders the scale if the objects modified date is after the already existing scale modified date (see https://github.com/plone/plone.scale/blob/master/plone/scale/storage.py#L63) in our case this is odd because the cropped scale gets lost. maybe it would be better to save only "scale crop information" in an annotation and read it in plone.scale/plone.app.imaging when creating the scale on first request (@jensens mentioned that in #13). would also make autocropping easier …. but this means to adapt/patch p.a.imaging and plone.scale

[petschki]

until we don't want to change plone core modules, the only solution is to make a method which restores/resaves the scale on ObjectModifiedEvent

Edit by @laulaz (2017-02-15) :

Here is the code to fix this bug in Plone 4 without touching plone.app.imagecropping or other plone packages.

in your configure.zcml :

  <subscriber for="plone.app.imagecropping.interfaces.IImageCroppingMarker
                   zope.lifecycleevent.interfaces.IObjectModifiedEvent"
              handler=".events.apply_crops_after_modify" />

in events.py :

# -*- coding: utf-8 -*-

from plone.app.imagecropping import PAI_STORAGE_KEY
from plone.app.imagecropping.interfaces import IImageCroppingUtils
from zope.annotation.interfaces import IAnnotations
from zope.component import getMultiAdapter
from zope.globalrequest import getRequest


def apply_crops_after_modify(obj, event):
    """
    Bug fixed here : cropped images scales on a content are lost after a
                     modification of this content.
    This is already fixed in Plone 5 but not in Plone 4 :
        https://github.com/collective/plone.app.imagecropping/issues/21
    To fix this, we need to re-generate all the crops of an object just after
    it's modification.
    """
    crops = IAnnotations(obj).get(PAI_STORAGE_KEY)
    if not crops:
        return
    croputils = IImageCroppingUtils(obj)
    request = getRequest()
    cropper = getMultiAdapter((obj, request), name='crop-image')
    for fieldname in croputils.image_field_names():
        for crop_key in crops:
            if crop_key.startswith(fieldname):
                scalename = crop_key[len(fieldname) + 1:]
                cropper._crop(fieldname, scalename, crops[crop_key])

[hvelarde]

@jpgimenez this is going to bite us when we deprecate our image tile in favor of the standard tile (collective/collective.cover#81)

[saily]

crops are also lost when image is copied.
i've done some experimental monkey-patching of plone.scales to avoid that, but needs cleanup and more testing. Feedback is welcome.

    <monkey:patch
        description="Patch plone.scales to create cropped images"
        class="plone.scale.storage.AnnotationStorage"
        original="scale"
        replacement=".storage.patched_scale"
        preserveOriginal="true"
        />

Several ugly things are happening down below. I'd love to see an adapter for plone.scales which queries for a factory how to create crops. What do you think about that?

from plone.app.imagecropping import PAI_STORAGE_KEY
from plone.app.imagecropping.interfaces import IImageCroppingUtils
from zope.annotation.interfaces import IAnnotations
from zope.component import getMultiAdapter


def patched_scale(self, factory=None, **parameters):
    """This is a really dirty hack to re-create blob files if a 
       cropping information was stored in object annotation.
    """
    info = self._old_scale(factory, **parameters)

    # XXX: That's very nasty
    if not factory.func_name == 'crop_factory' \
       and (parameters.get('width') != info.get('width') or
            parameters.get('height') != info.get('height')):

        cropped = IAnnotations(self.context).get(PAI_STORAGE_KEY)
        if not cropped:
            return info

        utils = IImageCroppingUtils(self.context)

        cropping_view = getMultiAdapter(
            (self.context, self.context.REQUEST), name='crop-image')

        for fieldname in utils.image_field_names():
            for image_name, box in cropped.items():
                scale = image_name.split('_').pop()
                cropping_view._crop(fieldname, scale, box)

    return info

[hvelarde]

any news on this issue?

[frisi]

just stumbled over a discussion on plone.scale (plone/plone.scale#3) which is related to this

[frisi]

@tomgross and i fixed this ticket in #38

for dexterity, this currently means to implement another interface.
(see FHNW@e2b510d)

@joka as dexterity pro: are you ok with that? personally i'd love to see that we can find a way to make dexterity content that implement IImageCropping also implement IImageCroppingScale.

if we can't find a way to make that happen, we should think about moving this interface out of the browser package to plone.app.imagecropping.interfaces and add some documentation to the readme

[frisi]

@saily reading through the comments here i see you mentioned copying an image too. i did not test that yet. would be great to get your feedback if this fixes your issues too.

[saily]

Let's write a test to check that, should be easy to implement. i'll try to find some time for that... anyway, thanks for fixing

[saily]

Hi @frisi, unfortunately this does not fix the copying issue. I've opened a pull #40 which could be a nice base to fix that. i'm closing this right now, because the main problem seems to be fixed in #38