Using 'manual crop' style, and leaving the minimum height and/or width fields blank results in missing image attributes

Thanks four your patch, I'll commit it as soon as you're other patch makes it to the core.

No problem, but do I have to review both the 7.x and 8.x patch (because this is a Drupal 8.x issue)?
I'm not an experienced core contributor :-)

Thanks, the patch works well for D7.

Edit: Sorry, i disabled error messages. The following warning will be shown:

Notice: Undefined index: path in manualcrop_resize_dimensions() (line 970 of /sites/all/modules/manual-crop/manualcrop.module).

Sorry, i forgot to patch the core, but $data['path'] is always NULL. It works anyway. ;-)

The patch is not correct. Replace ...

image_dimensions_scale($dimensions, $width, $height);

... with ...

image_dimensions_scale($dimensions, $data['width'], $data['height']);

The Patch does not work with the latest Dev version.

Patch works for me on 7.x-1.3, but then I get the following error on all pages with a cropped image:
Notice: Undefined index: path in manualcrop_resize_dimensions() (line 960 of /var/www/vhosts/*******/httpdocs/sites/all/modules/manual-crop/manualcrop.module).

Just figured that this is quite a major bug since it simply causes images not to load on ALL IE browsers.

Setting the following properties in CSS fixes the problem in the mean time:

width: auto;
height: auto;

I don't think the patch is quite right. In some uses, i.e. depending on what other image effects are applied to the image style, it will appear correct. However, the issue with the $data['path'] = NULL, as raised in other issues / previous comments, means that the width and height of the manual crop cannot be queried in the manualcrop table in the database.

The issue is that the core image module is not set up to pass the path of the image to the effect callback. This probably could be patched, but the signature of the callback function would need to be changed, so that the path could be passed to the function. This would cause problems for all modules that implement a 'dimensions callback' function. So not sure how to resolve this issue for this patch.

One way may be to use hook_theme_registry_alter to change the theme_image_style callback, so that a new callback function could be created to work out the correct width/height attributes.

This is updated version of the patch against the latest dev (6 Mar 2012).
It adds 3 different dimension callbacks for crop, crop and scale, and for reuse effects. The latter callback simply calls image_style_transform_dimensions() recursively. Please test & review.

NOTE: This patch requires applying core patch from #1364670-9: ImageStyle::transformDimensions unable to deal with all effects.

Patch from #16, against the latest dev.
See the NOTE from #16.

any update on this issue?

If someone is still looking for a quick fix for empty IMG tag dimension attributes, here's my solution.

function THEME_preprocess_image(&$vars) {
  // image has lacking dimensions
  if (empty($vars['height']) && empty($vars['width'])) {
    // get image size
    $image_size = getimagesize($vars['path']);
    // set measurements
    $vars['width'] = $image_size[0];
    $vars['height'] = $image_size[1];
  }
}

Updated patch which also covers "Auto reuse" feature.
Please test & review

p.s. oops, I created patch with my credentials in autopilot. You are free to not use this data. Sorry.

Re-rolled patch for 7.x-1.4+109-dev.

One change in crop style was changed.

And add some more doxygen and fix code style.

Let's commit it!

Just applied this patch, as we needed it to get the "img with srcset" functionality from the picture module: https://www.drupal.org/node/2450863#comment-10116088. Do you think this will be committed soon? Also we see an error with one of our image styles that has a "Manual Crop: Reuse cropped style" effect:

Notice: Undefined index: path in manualcrop_reuse_transform_dimensions() (line 1037 of manualcrop.module).

Not sure if anyone else has come across that.

#26: This needs a core patch for D7 (and D8).

Thanks for the patch. It works perfect. I've attached a re-roll of #24 against 7.x-1.x with some minor Drupal coding standard improvements.
Would be nice to have this committed, since having width and height attributes on images really improves the end user experience (since the browser can allocate the space even before the image is downloaded).

thanks BarisW and all

we'd love to see this patch applied too.

each time i apply it, though ... i have to modify a couple of lines to stop it erroring
1018: $crop = manualcrop_load_crop_selection(isset($data['path']) ? $data['path'] : null, $data["style_name"]);

and

1039: image_style_transform_dimensions($data['reuse_crop_style'], $dimensions, isset($data['path']) ? $data['path'] : null);

i guess this must be some idiosyncrasy of our system ... as no one else seems to mention it.

The manualcrop_crop_and_scale_transform_dimensions dimensions callback is invoked by image_style_transform_dimensions which passes only the style name and the original dimensions of the image. This means the $data['path'] will always be an undefined index, and manualcrop_load_crop_selection($data['path'], $data["style_name"]); will always return false. This means manualcrop_crop_and_scale_transform_dimensions can only ever return the dimensions configured in the style settings, so I've simplified the code for that function and removed the "Undefined index" error message it was causing. This creates the known limitation, which I think belongs in another issue since solving it requires changing something in Drupal core to make image_style_transform_dimensions pass along the image path:

// Known limitation: in the crop_and_scale transformation the dimensions
// assigned below will not be correct if upscaling is disallowed and the image
// is smaller than the crop area. The workaround is to ensure the image field
// enforces a minimum image size.

The image_style_transform_dimensions takes only two parameters, so I removed the third on a few lines.

Tested the patch in #30. The width and height tags appear, and I see no notices. Great work!

Wow. Long time coming. :)

Ping

Ping+

I have 'Manual Crop: Automatically reuse cropped style' set for my image style, with a fallback style that uses 'Scale and Crop' to 575x334. I offer 3 'Manual Crop: Crop and Scale' presets at 3 different sizes. What I'm finding is that with this patch, the correctly sized image is loaded into the page, but the height/width attributes are always set to the fallback image style dimensions. In the code, it looks like manualcrop_auto_reuse_transform_dimensions() never gets a style from _manualcrop_get_auto_reuse_style_name() and always uses the fallback image dimensions.

Nobody has mentioned having a fallback image style set, so I'm setting status back to 'Needs work'. If there is a misconfiguration on my end, please let me know. Thanks!