Collapsed fieldgroups break crop area display.

I'm having the same issue.

On step 3 while editing the node the crop initializes properly if the field is unfolded before the image is fully loaded. If I wait for the image to load first and then unfold the crop is broken.

So crop only works if the image is visible (unfolded) while the load finishes.

Same here. Can't collapse a field with imagecrop -- the image crop window dissapears..

subscribing

subscribing

I believe the same problem is occurring if you use vertical tabs. In this case, the fieldgroup doesn't even need to be collapsible. It seems that the crop area mechanism somehow relies on the image being visible at the time it initializes. Hmmm...

@bleen18, you are correct. I haven't found a way around this yet. If you are capable, or no someone who is, I will appreciate any help on this.

So here is the issue ... when an image (or any html element) is hidden js *always* returns both its width & height as 0. That means that in the case when an imagefield_crop widget is in a collapsed fieldset or a non-active vertical tab all the Jcrop functionality thinks that the cropbox image is 0x0. FAIL.

So we have a few a options:

1. Don't attach the Jcrop behaviors until after the cropbox image is visable. The only way to do this would be to attach behaviors to the correct fieldset... this will get uber complicated, especially when you factor in vertical tabs, and Im not sure how to do it
2. Temporarily show the fieldset when the page loads, attach the Jcrop behaviors, hide fieldset again.

This patch does the latter. Note that the "rehiding" part is not at all elegant as it requires a setTimeout to give Jcrop time to finish attaching all the behaviors. This is VERY ugly, but I dont see how else to make this work.

(patched against RC2)

Thanks for this patch. I am not sure the "better than nothing" is good here, so let's see what are our options.

I haven't found a way to attach to an event that fires when the image is shown (regardless of how). Is there anything of this kind?

As for option #1 - why would it complicate things? the patch does not solve the vertical tabs case (as far as I understand from reading the code. Correct me if I am wrong). Would you mind give a go to this option? you just need to find the enclosing fieldset (like in the current patch), and attach to the click event. I am not sure how to ensure not to conflict with the Drupal built in collapse handling though...

What do you say?

The patch in #7 does work with v-tabs because vtabs uses fieldsets too ... but I agree that its not an ideal solution.

As for attaching the Jcrop behaviors on a click event attached to the fieldsets... this has a few complications:

• we will need the code to be aware that sometimes the img_crop widget will be in a fieldset and sometimes not and theoretically, there can be one widget in a fieldset on the same page as one not in a fieldset
• we also need the code to add the Jcrop behaviors only on the first click ... otherwise Jcrop will reset the crop area every time you open/close the fieldset/vtab
• Drupal's standard collapse fieldset is setup differently then vtabs and we would need different code to attach the behaviors to each

These are just the ones off the top of my head... I'm not sure how I would even do the first one. The second and third are doable, but UGLY.

Thoughts?

subscribing

Hello,

Thanks for the patch, #7 works ok with vertical tab modules.
However it is confusing clients that the cropped area is reset to the upper left corner and the cropped preview is also showing only the upper left corner cropped area which confuses the client to think that they need to do the crop again.

@leon85321: can you post a screen shot of what your are talking about ... I'm not following

The patch in #7 doesn't seem to help me.

I simply get the Crop Area set to 0x0 with both handles on top of each other. I've tried with/without the patch and with/without JQuery Update (2.x).

nicholasThompson:

any chance I can get a look at the site you're testing on? I'm often on IRC (bleen) ... ping me if thats possible.

I have the same problem with 5.x-1.3 version as described in #13: 'Crop Area set to 0x0 with both handles on top of each other'. I'm also using hidden fieldsets in tabs.

@bleen18 -

Sure and here are the screenshots.
The 1st attachment is the node view after first uploaded it and cropped.
The 2nd attachment is in the edit view after the file was cropped and went to edit form.

You can see that the selected area is now to the top left corner. However as long as I do not touch that cropping area, the actual cropped image in the node view will not change after saving the editing form.

Thanks,

Leon

subbing

Does anyone tried changing CSS for elements included in fieldset.collapsed?

I included this css and it fixed the problem:

html.js .filefield-element fieldset.collapsed {
    overflow: hidden;
}

html.js .filefield-element fieldset.collapsed div.fieldset-wrapper,
html.js .filefield-element fieldset.collapsed .form-item {
    display: block;
    position: relative;
    width: 100%;
}

I have to change line 25 in imagefiled_crop.js from:

var preview = widget.parent().find('.widget-preview');

to:

var preview = widget.parents('.clear-block:first').find('.widget-preview');

because of additional div.fieldset-wrapper added by collapsible.js, too.

Does this solution works for someone else?

Not for me :(

#18 does not work for me either ... daniula, what browser are you using?

Hi! I was playing with JQUERY and I made something that fixes the problem... at least for me...
I used some of the JQUERY code in this page: http://remysharp.com/2008/10/17/jquery-really-visible/
I made modifications ONLY at the beginning of imagefield_crop.js
Basically, it starts all javascript stuff only when the crop wrapper is really visible... and that fixes the problem.
BTW, this fix is based on comment 7 option 1, from Bleen18.

/* $Id: imagefield_crop.js,v 1.1.2.3.2.12 2010/06/11 14:56:03 yhager Exp $ */

Drupal.behaviors.imagefield_crop = function (context) { 
  // wait till 'fadeIn' effect ends (defined in filefield_widget.inc)
  var attachJcropContext = function() { attachJcrop(context); }
  setTimeout(visible_crop,1000);

  function visible_crop () {
    if (!($('.jcrop-preview-wrapper').is(':hidden') || $('.jcrop-preview-wrapper').parents(':hidden').length)) {
      attachJcropContext();
    } else {
      setTimeout(visible_crop,500);
    }
  }

  function attachJcrop(context) {

If this works then maybe someone can make a patch...
Hope that helps...

The fix in #21 solved the problem for me. Sorry, have yet to make a patch...

to clear up for anyone, edit imagefield_crop.js,

comment out line 6 like so:

<?php
//setTimeout(attachJcropContext, 1000);
?>

Then add the following right below

<?php
  setTimeout(visible_crop, 1000);
  function visible_crop () {
    if (!($('.jcrop-preview-wrapper').is(':hidden') || $('.jcrop-preview-wrapper').parents(':hidden').length)) {
      attachJcropContext();
    } else {
      setTimeout(visible_crop,500);
    }
  }
?>

+1

Your solution worked perfect for me - awesome!

Thanx

Looks close, seems to work okay for single image fields, but it's not working right if you allow for multiple image uploads via the "unlimited"- add another item setup. If it allows for multiple uploads I just get a blank space where the preview and cropper should be.

#22 Did the trick for me!

Im not sure how this can work for you guys, but for me with the latest dev. The javascript isn't even added to the fields.

So this patch does not work for.

Setting to critical, this module must work with groups/fieldsets

PS; im using CCK3

I've also tried the patch with the latest dev, also using vertical tabs - if anything it's worse, definitely doesn't function within vertical tabs.

When you first upload a file and crop, it seems to work ok, but once you return to it after it's been hidden by the vertical tabs on page load, you get what's included in my screenshot.

I made this work for my personal setup by altering the javascript in the patch a bit. The class .vertical-tabs-group_profile_photo is the class of my fieldset in vertical tabs that contains the image crop field I've been trying to get working. I couldn't make it work with the generic values that referenced the wrapper fields parent fields unfortunately.

  setTimeout(visible_crop, 1000);

  function visible_crop () {
    if (!($('.vertical-tabs-group_profile_photo').is(':hidden'))) {
      attachJcropContext();
    } else {
      setTimeout(visible_crop,500);
    }
  }  

If someone can figure out why the general use code thinks the parent field is still hidden, even when it's not, maybe we can get this patch working

That patch in #26 (based on #22) worked for me. Thanks.

I found a solution that works quite well for me so far.

I've had the same problem: I'm using the field_group module with horizontal tabs. A Cropbox that appeared within a (at first) hidden tab, would not display correctly (zero size) when shown.

The reason why it is messed up is because it gets dynamically generated with incorrect size values. The values are obtained from a jquery object which is not completly initialized at that point (obviously due to its hidden state).

My solution takes care that the Cropbox is created properly in the first place when the Jcrop behaviors are attached. It simply initializes the dimensions of the jquery object from the img element. When the img element is not loaded yet a temporary image is created to get the dimensions (happens in IE 7).

As soon this was working, another little problem came up. When the crop area was moved (grabbed) the very first time it would jump right to the top left corner (of the cropbox). After that it was working as expected. I fixed this by (re-)initializing the offset when a drag process starts.

These changes have to be made to the (external) Jcrop plugin though. I think this could be even considered a bug in Jcrop.

Please test this and if you can confirm and agree, I'll report it to the maintainer of Jcrop.

Although I'm workin with 7.x-1.x-dev it should apply to other versions equally since only the Jcrop plugin is affected.

Tested with the following browsers:

• Firefox 3.6.17 (Ubuntu 10.04)
• Chromium 11.0.696.71 (Ubuntu 10.04)
• Internet Explorer 7.0.5730.13 (Windows XP)

The patch only modifies the file Jcrop/js/jquery.Jcrop.js. The minimized jquery.Jcrop.min.js is there also, but is not used (from my observations). As well as the packed jquery.Jcrop.pack.js in 6.x. I think if they are used or for consistency they should also be fixed sooner or later (or removed).

Here is the patch from #35 for 6.x-1.x

Not sure if the version of jcrop has changed between branches, but that patch wouldn't apply cleanly in 6.x.

Yes, indeed 6.x-1.x uses Jcrop v0.9.8 and 7.x-1.x uses v0.9.9. Didn't check that, thanks.

#35/36 is a much better solution since it doesn't leave the browser in an infinite loop waiting for the cropbox element to be visible. Unfortunately, it does not seem to work (tested on 6.x-1.x + Chrome 13 + both vertical tabs and collapsed fieldset). The patch applies, but it has no visible effect on the problem.

Closed #779044: When outside main Vertical Tab it fails. and #1030746: Works fine first time - broken when edited as duplicate of this one since this one has some semi-working patches at least.

Just a quick update.

I did some testing with a Drupal 6.22 installation and imagefield_crop-6.x-1.x-dev which indeed still has the problem.

First I tried to update Jcrop from v0.9.8 to v0.9.9 (as in D7). That did not help. After some some debugging I noted that there is some strange (jQuery-)behaviour happening.

Then I was a bit surprised to find that Drupal 6 uses the 3 year old jQuery v1.2.6 from 2008-05-24 (misc/jquery.js).

Drupal 7 is using jQuery v1.4.4 from 2010-11-11.

After copying the misc/jquery.js v1.4.4 from D7 to D6 (with Jcrop back to v0.9.8) the problem was solved!

But I'm not sure if it can be done that easily, without breaking other things.

I tried the jQuery Update module.

But all versions jquery_update-6.x-1.1 (has same jQuery v1.2.6 as current D6 core), jquery_update-6.x-2.0-alpha1 (jQuery v1.3.2), jquery_update-6.x-2.x-dev 2011-Apr-20 as well as the latest 6.x-2.x git branch (jQuery v1.3.2) do not fix it.

The only jQuery version so far (that I've tested), that fixes the problem, is v1.4.4.

Before I spend more time with it I'd like to see if anybody else has any other idea.

applied the patch of #36 by hand it works for http://drupal.org/node/1030746

I can't get the patch in 36 to work against 6.x-1.x either. Upgrading Jquery isn't a good option because a lot of other modules and core things depend on it. It could be possible to try to load it alongside the existing jquery using the noConflict mode as outlined here:http://drupal.org/node/1058168 but that's probably not ideal since it will bloat the page running 2 copies of jquery. Ideally we need to track down the issue and make it at least jQuery 1.3.2 compatible for the drupal 6 branches so it can run with jquery update.

#33 worked for me and I made a slight modification of the first line of the function to allow this to work for any hidden fieldset that may contain images.
Change:
if (!($('.vertical-tabs-group_profile_photo').is(':hidden'))) {

to:
if (!($('.jcrop-preview-wrapper').parents('fieldset').is(':hidden'))) {

And it should work for any fieldset that contains the cropper.

Tested working on Chrome 13 with 1.0-RC2 and Patch from #36 applied. (Not sure if that patch application matters or not though)

@alanpuccinelli: I agree with #42 but cannot follow #43. I can't find that line.

Sorry to be unclear, that was an edit of the suggestion in #33.

Oh sorry, my fault, I didn't notice the "#33".

Ok, but this is still more of a workaround than a solution. First because it's using a timeout and second because it requires to manually add code somewhere (if I understand that right).

Sorry again, your proposed change does not require to manually add code somewhere, that was the idea. But it's still the timeout that doesn't qualify it, in my opinion.

Thank vood002 (#22), it works for me.

Jox's patch in #35 works against the 7.x-2.x-dev (Aug 22nd) version on Drupal 7.7

I also tested it with the Jquery Update module enabled (Updates jQuery to jQuery 1.5.2 and jQuery UI 1.8.11.) and it works.

Tested in....
Chrome 13.0.782.215 m
Firefox 6.0.1
IE 9.0.8....

There was one problem. After saving a new image to the crop area and saving the node, the first person to view it (probably the person who just saved the node) will see a full resolution image. The second person visiting the page will see the correctly cropped photo. This side effect is less severe than the problem it fixes.

@animelion: thanks for your review. I have never experienced the problem you describe. Might that be some browser cache issue? Since the cropped image will have the same filename as the original image (the original will be renamed to the same name with "_0" attached).

When I try to reproduce it like you describe, the node is displayed right after saving it and always shows the correctly cropped image.

Does the problem occur for you really anytime you try it? Or sometimes it does and sometimes not?

Could you try to give more details in order to reproduce the problem?

Thanks jox, played around some more and the problem only happens if the Jquery Update module is installed.

@animelion: unfortunately even with Jquery Update enabled, I'm not able to reproduce it...

Some questions:

1. Does this happen on all browsers?
2. Does this happen reliably (really every time)?
3. How is the node and the cropped image displayed? How is the content type and display settings configured?. What kind of additional fields are contained?
4. What happens if you reload the page when it falsely displays the full resolution image?
5. What happens if you right click on the falsely displayed full resolution image and choose "View image" (and possibly reload that image)?
6. What further additional modules do you have installed?
7. (If you have not done that already.) Would it be possible for you to create a fresh drupal install with only the related modules and try to reproduce it that way? Maybe we can find out if some other module is involved.

After uninstalling and re-installing the jquery update module the problem seems to have gone away. If it "crops" up again I'll be sure to follow up and do the additional troubleshooting you requested.

1. Only tried in chrome
2. No, it has stopped actually
3. Whatever defaults the module configures. No additional fields other than the nested field groups.
4. The correct image would appear
5. Haven't been able to reproduce so I can't test this
6. A bunch, will post if the problem crops up again
7. If the problem crops up again I will give this a go

That's strange. But it makes me think it might not be related to my fix, but not sure.

Anyway thanks for your report, and get back if you experience it again.

#26 works for me. Thank you. I am attaching the same patch for 6.x-1.0

@jox, that your comment in #40 relate to the patched jcrop (from #35/#36). I prefer to patch jcrop if it solves the problem, but will retort to the looping method if it doesn't. Have you tried introducing that patch to Jcrop upstream?

If #35/#36 do not work for this case, I will implement one of the solutions in the patches here. Also found this: http://plugins.jquery.com/project/in-viewport-event, which might also be used.

hello,

#55 works for me but a new problem exists.
If we edit multiple uploaded images, the patch works well for the first image but not for others. On the first image, the crop is present in the middle of the source image, it's ok. But for others images the crop takes the already cropped images.

Do you have a solution?

Hello,

nobody has this problem ? (see #57)

Thanks.

#57 #58
Hello,

The problem is the file name with the "FileField Paths" module.
if we apply the following patch "http://drupal.org/node/1172724" everything works normally.

@yhager #56: sorry for the delay. I have just contacted the author of Jcrop describing the problem and introducing the patch.
I am now waiting for a reply and will inform you about any news.

Great news. Kelly Hallman, author of Jcrop, was so kind to add the fix (#35/#36) to his project:

https://github.com/tapmodo/Jcrop/commit/6150bbad59283a33bcf8d36c7e3f8d7a...

He created also a little demo for this "new feature":

http://aji.tapmodo.com/~khallman/Jcrop/demos/jquery-ui.html

He announced the release 0.9.10 which includes this patch (among other improvents and fixes).

(https://github.com/tapmodo/Jcrop/commits/master.)

It should be available very soon:

http://deepliquid.com/content/Jcrop.html

These are great news @jox! Thanks very much for pushing this.
I haven't tested yet, but this might even allow us to remove the notorious delay we have in the code as a guess for the time the image is loaded.

Once the release for Jcrop 0.9.10 is public, I'll update it. (if you notice it before me, feel free to nudge here)

Hi, i am relatively new to drupal and have never applied a patch before. Any directions would be much appreciated.

@Lankyman85: http://drupal.org/patch/apply

Thanks for the reply, i ended up installing imagefield focus which works pretty much the same as image crop.

The latest Jcrop (v0.9.10), is available: http://deepliquid.com/content/Jcrop.html

And it totally fixes this issue!

Confirm that dropping the new v.9.10 Jcrop into imagefield_crop-6.x-1.0-rc2 fixes the problem. Moving to needs review so the module author can action.

Dropping in new v.9.10 Jcrop fixed my issue for Drupal 7.

Had the same problem, but with a custom image field attached to a user (not profile2).
Imagefield crop works fine, but when you put the field in a vertical tab Jcrop is not loaded properly while editing an already uploaded image.

Had the same problem. It's kind of ridiculous that it isn't patched yet, so I will submit this proper patch.

The only change I made here was to update the JCrop library to 0.9.12 (added the MIT license too)
Others have said that this fixes the problem for them.

Please check to see if there are any cases where the updated library breaks the system.

* patch applies to 7.x-1.x (7.x-2.x is still in dev)
** I will check the 7.x-2.x to see if it could use the update as well.

status change to trigger review

status change to trigger review

I confirm Jcrop 0.9.12 does include my fix (it does since 0.9.10) and works generally well with imagefield_crop (7.x-1.1).

Imagefield_crop still includes Jcrop 0.9.9 in all versions (latest 7.x-1.x and 7.x-2.x).

@yhager (or some other maintainer): It would be great if you could update the library sometime.

Thanks.

Not complaining about jaxxed getting (the) credit. It's not entirely fair though, that the one who did the significant part of the work was ignored. See Commit messages - providing history and credit

Thanks for commiting anyway.

Thanks jox. I have no means to discredit you in any way, but I cannot credit every contributors in the thread. I am crediting him just because he is the patch creator. Thank you for coordinating with Jcrop creator.

I'm not every contributor in the thread. I'm the one who spend a couple of hours to fix the issue (programmatically). The coordinating was just some followup favor.

@joetsuihk: Let me add something. I think what you said is not perfectly true: "I cannot credit every contributors in the thread". Since you as a maintainer are actually encuraged to do so. The docs (link above) say "If others have contributed to the change you are committing, take the time to give them credit". Of course you're not supposed to add everybody in the thread. Thats why it says "take the time". It's quite common to see several contributors in the commit messages.

Only adding the contributor that posted the last patch is just to simple. Often enough patches are resubmitted by others due to changes in the original code, since issues sometimes take ages to finally get committed. They deserve credit, but the original author not less.

Some more relevant quotes from the docs:

"Each commit message should contain at least one contributor name, even if it refers to yourself."

"If a very very large list of people (eg. more than 10-12) contributed to a change, pick the major contributors and add et al to represent the rest"

"it's very valuable to know who actually wrote or contributed a certain change"

Please don't get me wrong. I do not insist by all means in getting this credit and I'm not angry or something. It would just be nice to see some consent so others get their deserved credit when you do further commits in the future.

@jox, I'm sorry your name was dropped from the credit. We value and appreciate patches. It's heartwarming when someone else spends time to fix issues that are dear to them and then we can all benefit.

We can't edit the comment AFAIK. We'll strive to give credit where it's due in the future.

We hope you'll accept this apology and continue contributing to this module. It definitely needs some love.

Thanks

Thanks yhager. That's some nice words. It's all right. I'll gladly contribute more if there's a chance. :-)

Confirming that #69 works to fix the issue. Thanks to all for your hard work!

There is a patch for 7.x-2.x here: #2054767-4: Update Jcrop to v0.9.12