Problem

When you have a node with 2 image fields, with the same image (fid) but different required cropping styles, after cropping MC gets confused and marks all the images CROPPED, even though only 1 style was cropped. Saving then, results in avalidation error, because only 1 was cropped, but 2 were marked as "Crop (cropped)".

Reproduce

  1. Create node type with 2 required image fields, with FileField Sources, without any cropping
  2. Create 2 croppable image styles
  3. Create a node with both image fields filled with the same image (hence FileField Sources or alike)
  4. Update the node type fields to have different required cropping styles
  5. Edit the node, add cropping etc

Solution

In theory it's very simple: only update buttons/options that MATCH the just created cropping (fid + style), not just the cropped image (fid).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rudiedirkx’s picture

Version: 7.x-1.5 » 7.x-1.x-dev
FileSize
2.11 KB

Well, that was even easier than expected.

Patch changes a few classes.

David_Rothstein’s picture

Status: Active » Needs review
FileSize
11.05 KB

The above didn't work for me when using the Media module's media library to get the same image to appear more than once on the same form. I had to make some additional changes in the attached:

  1. Make the JavaScript that finds images for a widget grab only the one associated with the widget.
  2. On form submit, handle the possibility that more than one image style can be submitted for the same file.

This is not as big of a patch as it looks like - a lot of it is indentation changes due to a new foreach() loop.

David_Rothstein’s picture

Added a small fix to prevent duplicate images from appearing in the widget sometimes.

rudiedirkx’s picture

I don't do or know Media, so I can't test it...

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

RTBC - looks great to me.

Lennard Westerveld’s picture

Edited: oops i broke it :) ignore my patch

Lennard Westerveld’s picture

Lennard Westerveld’s picture

Added new patch this one is working with the latest dev version

Fabianx’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/manualcrop.helpers.inc
@@ -236,7 +236,7 @@ function manualcrop_croptool_process(&$form, &$form_state, &$element, $file, $se
-    $form_state['manualcrop_data']['images'][$file->fid] = array(
+    $form_state['manualcrop_data']['images'][$file->fid][] = array(

We lost a comment here compared to David's patch.

dqd’s picture

Can someone confirm that the module has supported this ...

a node with 2 image fields, with the same image (fid) but different required cropping styles

... firmly? Otherwise we should rather tag this a feature request instead of a bug? Because this can bring up a module version discussion, depending on if this will have surprising impact on already installed modules and sites.

Lennard Westerveld’s picture

@Diqidoq nope this isn't a feature right now, so it needs to be a new one.

See manualcrop.helpers.inc
'id' => array('manualcrop-' . $crop_type . '-' . $fid),
'id' => 'manualcrop-area-' . $fid . '-' . $style_name,

And

manualcrop.js
ManualCrop.output = $('#manualcrop-area-' + fid + '-' + styleName);

Its right now not mode for it because of the html ids.

douggreen’s picture

Re-rolled and added missing comment mentioned in #10 by @Fabianx