Problem/Motivation

ImageCache allows default presets to be defined by modules using hook_imagecache_default_presets. The imagecrop module lacks support for those presets in at least two ways:

  1. The presetid of default presets are the same as their presetname and are therefore non-numeric. However, the presetid column of the imagecrop table only holds integer values and will hold 0 when trying to save a string to it. Therefore, imagecrop doesn't properly retain cropping information for more than one default preset.
  2. The default presets are not stored in the ImageCache database tables. The imagecrop module cleans up the imagecrop table by deleting records that refer to ImageCache presets that no longer exist. It does this by joining in the imagecache_actions table, which does not contain any of the default presets, causing this cropping information for those presets to be deleted.

To reproduce the problem, write a small module that publishes more than one default ImageCache presets and try to make them work with the imagecrop module.

Proposed resolution

  1. The imagecrop module should store its cropping information not by presetid but by presetname. (The ImageCache module seems to assume the presetname is unique, even though it does not enforce it with a UNIQUE index on its table.)
  2. In imagecrop_cron() the imagecache_presets() function should be used instead of joining the imagecache_actions table.

Remaining tasks

Maybe, a bug should be filed with ImageCache to get rid of presetid in favor of presetname as a preset's unique identifier.

Comments

nils.destoop’s picture

Had the same bug in D7 version. I just changed all the code today.
Should check how easy it is, to change the full d6 version to presetnames.

nils.destoop’s picture

Status: Active » Needs review

A fix has been committed to dev. Can you test?
There is an update that needs to be run, so remember to backup first / try on localhost.

nils.destoop’s picture

Status: Needs review » Closed (fixed)