Imagecrop does not work with drupal 7.20

Looks good we tested and reviewed the patch.

After applying the patch it still seemed to be broken. After cropping an image it would break the image src url. It seemed to be adding ?time again instead of &time. After a little debugging I was able to get this working without the patch and just changed line # 172 in imagecrop.admin.inc

//$image_url .= ($clean_url || !strpos($image_url, '?') ? '?' : '&');
    $image_url .= '&time=' . $_SERVER['REQUEST_TIME'];

I did notice the patch fixed some things like. Refreshing of the imagefield on the node form which was in imagecrop.js
I was unable to figure out if the other stuff in the patch was even needed.

Patch also not working for me.

I took advice from #3, but it made more sense for me to make the line read:
$image_url .= !strstr($image_url, '?') ? '?' : '&';

I'll roll a patch with #1 and my change..

Have tested the patch #5 on a couple of Drupal 7 sites which were having the image problem described above. Both sites cropping is working well. Would be great to see the patch rolled into a release soon. Thanks for this!

#5 worked for me as well with Drupal 7.21.

Question: is there any way to force an update if a new crop setting is applied to the user edit/view screen once saved? Otherwise the browser cache is displaying the old image.

The patch in #5 did not work for me. I still don't see the link to create image crop in user profile.

This is #5 with some fixes. Works for me on Drupal 7.22.

Tested #9 in 2 websites with 7.22 and it works.
Thanks to all involved.

And pleeeease someone there, could you please give us a production or at least a new rc4 version??
It will be great after near 2 years from rc3.

I'm sorry, but after some days testing, I can say this module doesn't fully work for me:
- Now, with drupal 7.22, it doesn't display croping box, when I click on "Edit this style". I give you a screenshot.
- Changing scale, doesn't resize image

I've applied patch #9.

Does it work for you?

Many thanks for any help!

Josean

@josean the issue you are having seems to be unrelated to this issue. If I had to guess it would be some kind of module conflict probably causing some type of jQuery error. It looks like your using chrome in the screen shot. Check the console for JS errors.

The reasons i'm guessing it is unrelated to this issue is because the image would not render at all. It seems like the image is rendering for you but jCrop is not working.

This issue has to do with using tokens to generate images.

Fantastic, gooddesignusa!!

You are right, it was a js error in imagecrop.js
I did a mistake editing this file when I applied #9 patch. Now it's corrected and it works!

Many thanks for your help, you have a new friend :-)

I've to assume I'm not very used editing javascript and it would be a help if someone publics patched files instead of only patch files.
As it will help to anyone like me, I attach here patched files in a zip.
Original files come from last imagecrop 7.x-1.x-dev version, now 7.x-1.0-rc3+56-dev from 2012-Oct-30.

Best regards,

Josean.

I think we can close this.

@josean Please do not close the issue until it gets committed. It makes it harder for users to find this much needed patch.
I do think its safe to say we can mark this as reviewed and tested.

Hi,

I have Drupal 7.22 and I am having trouble getting imagecrop to work.
I would like to users to be able to crop their user profile picture.
How do I apply the patch? Can you point me to documentation to how to set up the module?

Thanks

Works for me on Drupal 7.22 and 7.x-1.x-dev with patch from #9.

@radicontrolled and @josean, please Google for: drupal apply patch.

Thanks, aasarava.
The first time I had a look in documentation I didn't see any automated way to apply patches (my PC is windows).
As you suggested, in drupal.org/patch/apply is documented a very easy unix command to do so:
$ patch -p1 < path/file.patch
I run it directly on server, and thats all. Fast and error free.

gooddesignusa, thanks for your correction. I didn't know which is the correct protocol. I take note.

Thanks again.

I"m trying the patch in #9 and getting the same results as #11. But I'm also not getting any JS errors. not sure what I did wrong.

Updated module with drush dl imagecrop --dev && drush updb
Applied patch.

I also tried the zip in #13 but it's only the modified files and not the whole module.

I'm on Drupal 7.22. Any ideas what I'm doing wrong?

Edit: Even applying the patch manually it doesn't seem to be working.

Works great for me -- using Drupal 7.22, imagecrop 7.x-1.x-dev, and the patch from #9.

#9 Patch Applied cleanly but am not able to crop the images correctly as it appears as though I cannot set the pixels in the settings and the image is cropped in correctly.

Bump to major. This make this great module almost useless.

To help people testing. The -dev version with patch from #9.

Don't forget to update your db.

Pretty sure this new dev version works. I had a cropped image that wasn't working and I simply refreshed the page after updating the code and I could now see the cropped image...nice job!

There's another instance of the same problem in imagecrop.admin.inc, see https://drupal.org/node/2088015

In response to the comments on here since the patch was posted on #9:
#10,22,26 - successful test
#11-19,25 - support questions / user error
#20-21 - probably user error, but can't be sure
#23 - separate issue
#24,27 - issue management

In summary, the code in the patch looks good (my code review) and we've had 3 people (4 including myself) report that #9 works, and one (#20) report that it doesn't but with no futher information. Other than the fact that it's missing the instance of the problem in imagecrop.admin.inc I'd set this to RTBC.

Looking at it again #9 does have the changes to imagecrop.admin.inc, I'm not sure why I thought it didn't yesterday. The relevant change is

-    $image_url .= ($clean_url || !strpos($image_url, '?') ? '?' : '&');
+    $image_url .= (!$clean_url || strpos($image_url, '?') !== FALSE) ? '&' : '?';

I still think checking for clean_urls is redundant if you are going to look for a ? in the URL, but negating it makes more sense: If clean URLs are disabled then you will know you will have a ? in the URL.

The change to strpos is code style thing - it is logically identical except in the instance that the first character is a question mark, in which case strpos will return its position (0), which will evaluate to false. $image_url should never start with a question mark though, so while the new code is following best practise it shouldn't actually make any difference.

Therefore I'd say this has been pretty well reviewed -> RTBC.

There were some minor whitespace errors in #9, so here's a patch that's identical except for those.

There were some minor whitespace errors in #9, so here's a patch that's identical except for those.

What were the whitespace errors? I'm using the Vim plugin for Drupal which complains at you when you have whitespaces that do not conform to the coding standards so I thought all the whitespace formatting was correct. If it isn't then I may need to do some tweaking to the config.

Actually you're correct - #9 fixes some whitespace in the repository that didn't comply with the coding standards (missing new lines at the end of files). Although technically you should commit changes like that as separate patches which would make #28 correct...

These are minor technicalities though. Lets get one of these committed (probably #9) so the module works on the current version of Drupal.

Committed and pushed to 7.x-1.x-dev, thanks!