My first issue ever reported, please be kind. For this drupal newbie drupal is like a combination of a candy store and a maze, at the moment I need some guidance in the maze.

If the image style 'original image is used' the image is displayed incorrectly. Looking at the output
the image itself is cropped correctly but the width and height tags are wrong.

My conclusion was that imagefield stores the size info and that they are not updated correctly. Therefore this is a bug, maybe in imagefiled_crop maybe in the core image module, but how to fix it correctly?

A dirty hack around it is to modify image_field_presave() in image.field.inc and set the $item['height'] and $item['width'] unconditionally based on the result of image_get_info(), here is my debug snippet for an idea:

// dirty debug hack bdu
    $info = image_get_info(file_load($item['fid'])->uri);
    if (is_array($info)) {
      if ($item['width'] != $info['width']) {
        // watchdog('bernard', t('width mismatch'), array('item width' => $item['width'],'info width' => $info['width']), WATCHDOG_ERROR);
         $item['width'] = $info['width'];
      }
      if ($item['height'] != $info['height']) {
        // watchdog('bernard', t('height mismatch'), array('item height' => $item['height'],'info height' => $info['height']), WATCHDOG_ERROR);
         $item['height'] = $info['height'];
      }

This is not a proper solution as this causes an extra but not necessary round trip for every image update. The correct solution is to somehow pass the new width and height.

Any suggestions how to implement a proper solution?

Comments

kristofferwiklund’s picture

Seems related to this bug:
http://drupal.org/node/1327888

And there is something that has change in 7.9

bdu’s picture

Thanks, the symptoms are different but probably related root cause, I made a reference.

Greg Varga’s picture

Just a quick note:
This is from Drupal 7.9 change log:
Restored height/width attributes on images run through the theme system.
http://drupal.org/node/1322736

I hope this helps a tiny bit.

bdu’s picture

Status: Active » Closed (duplicate)

For a moment I thought this was fixed after applying core 7.10 but that is not true.

bdu’s picture

bjaxelsen’s picture

Status: Closed (duplicate) » Active

I've made a fix on this in my theme (though it is a hacky solution)

/**
 * Implements theme_image().
 * Needed to make new Drupal version work with image crop module
 */
function mytheme_image($variables) {
  $attributes = $variables['attributes'];
  $attributes['src'] = file_create_url($variables['path']);

  // dirty debug hack bdu
  $info = image_get_info($variables['path']);
  if (is_array($info)) {
    if (isset($variables['width']) && $variables['width'] != $info['width']) {
       $variables['width'] = $info['width'];
    }
    if (isset($variables['height']) && $variables['height'] != $info['height']) {
       $variables['height'] = $info['height'];
    }
  }

  foreach (array('width', 'height', 'alt', 'title') as $key) {
    if (isset($variables[$key])) {
      $attributes[$key] = $variables[$key];
    }
  }

  return '<img' . drupal_attributes($attributes) . ' />';
}
makangus’s picture

Having the same problem after upgrading drupal core to 7.10.

Since I always crop images to the same resolution and I have the media module set up, so the way for me to get around the problem is to create an image style to resize the cropped image to the same resolution. It's not exactly a solution since it basically shifts the responsibility to media module to display the image once the image is cropped

bdu’s picture

Thanks Gergely for pointing me into a direction. This could looks to be a side effect of the improvements made for for #1129642: Populate HTML image tags with dimension attributes (like D6 imagefield) without re-introducing I/O . There the width and height are cached. Not sure if image filed crop should call other/more callbacks or that the library should update the width and height after any transformation.

jbrown’s picture

I would recommend using the 7.x-2.x-dev branch where this problem does not occur.

bdu’s picture

Status: Active » Closed (fixed)

Indeed using 7.x-2.x-dev fixes this issue, closing this one.

bdu’s picture

Status: Closed (fixed) » Active

Reopening this. The 7.x-2.x branch is incompatible with 7.x-1.x. It is no longer a widget but a new field type. And there is no upgrade path. See the remarks on the project page.

bdu’s picture

Project: Imagefield Crop » Drupal core
Version: 7.x-1.0 » 7.10
Component: Code » image system

Did some more research. You will only see this issue if the cropped image is used without further styling, if a style is applied the extra (not necessary) processing produces the correct result, as the image itself is correct.

The image api calls, image_load(), image_crop(), image_scale_and_crop() and image_save() used by the imagefield_crop module work on an image object (with embedded dimensions) and do the correct thing. Image_save() makes sure that it has the correct information stored, as it simply reloads it.

The value callback, imagefield_crop_widget_value() saves the original images, crops a copy and, replaces the fid. And that is the only thing it can do from there as any other change will not be picked up.

Therefore the imagefield_crop module does the correct thing. Also it should not need to know about what properties imagefield wants to cache or not, that is something for imagefield internally. Looks to be introduced by the important performance fix for #1129642: Populate HTML image tags with dimension attributes (like D6 imagefield) without re-introducing I/O. Therefore making this a core issue.

yang_yi_cn’s picture

agree with #12 and I'm currently using the workaround #6.

It looks like the core team realized this and opened #1448124: Image dimensions should be available from file_load() for images, and not stored in field data.

RgnYLDZ’s picture

Got the same problem. No solution works for me.

Following...

RgnYLDZ’s picture

last patch in this post works fine with 7.x-2.x-dev.

Lann’s picture

Currently using workaround #6 on the 7.x-1.x branch. Thanks. I think this defect's priority should be raised because this issue effectively made the module unusable for me.

dddave’s picture

Status: Active » Closed (duplicate)

see #13