Picture is not cropped even though crop: true is set

[robinboening]

The cropping behaviour is broken. I think its broken in more than just one way, but in this issue I will keep it simple and focus on this one behaviour.

You can reproduce it this way:

1. Upload a image with a dimension of 800x600. (https://reimann.reisen/wp-content/uploads/2017/02/reimann-reisen-potsdam-image009-800x600.jpg)

2. Assign the picture in an element with an essence picture.

3. Render the picture in the view with a certain size and cropping set to true.

<%= el.render :picture, size: "400x400", crop: true %>

---

I expect the picture to be cropped to exactly 400x400 pixel from the center because I put crop: true in there and no manual cropping happened in the UI.

What I see instead is a picture that is not cropped at all. Its scaled down to 400x300 pixel.

Technical explanation of the issue:

It seems this issue was introduced with this PR #1084 when the way of using the picture rendering was changed.

Since we merged it in September 2016 we use picture.url instead of the url helpers. That is totally fine. Since then the crop_from and crop_size options are always set in the options hash. This is totally fine as well. If no manual cropping happened in the UI the values of these two options should to be nil (as defined as the default values in the crop method here https://github.com/AlchemyCMS/alchemy_cms/blob/master/app/models/alchemy/picture/transformations.rb#L45) - and this is not true. These values are empty strings instead of nil. This leads to the situation that even if we actually have no crop_from nor crop_size it will set the cropping dimensions with 0x0+0+0.

DRAGONFLY: shell command: 'convert' '/var/folders/0w/8wchqjw1017bc709wz420lx00000gp/T/dragonfly20170713-73677-484pe6.jpg' '-crop' '0x0+0+0' '-resize' '400x400>' '/var/folders/0w/8wchqjw1017bc709wz420lx00000gp/T/dragonfly20170713-73677-ys72ib.jpg'

This leads to the fact that this picture won't be cropped at all but scaled down to 400x300 pixel.

I will provide a failing test to demonstrate it and a fix as well.

[tvdeyen]

@robinboening any news on this issue?

Since 4.0.0.rc1 we do not jsonify options params anymore. Could you revisit this issue with latest master? It should probably be fixed now.

[tvdeyen]

#1320 is maybe related as well. @robinboening could you please check if the issue is still present with latest master and not if it's fixed with PR 1320? Thanks

[tvdeyen]

Confirmed and fixed by #1321

[robinboening]

Thanks for working on this.