Posted by jbrauer on February 25, 2008 at 12:39am
| Project: | ImageField |
| Version: | 5.x-2.0-rc6 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
After upgrading to rc6 the imagefield upload area no longer appears. The same is true on a test site with Drupal 5.7 and imagefield-2-rc6 being installed without any previous version.
Relevant details from the test site (with far fewer modules installed)
- CCK Version: Content 5.x-1.6-1
- Image Field 5.x-2.0-rc6
- permissions on tmp and files are 777 (on this localhost test) on the public system they have been working and still work for uploads etc.
- Download method: Public files
- Imagefield configuration: all defaults. no limit to upload size/dimensions, not required, no default set, etc.
- Expected result: Image upload field
- Unexected result: no image upload field (see attached picture)
- To reproduce: I was able to reproduce with a new install of imagefield rc6 tarball and enabling content and imagefield. I then added an image field called img to the Story content type.
| Attachment | Size |
|---|---|
| imagefield.png | 13.18 KB |
Comments
#1
This is a result of
if ($field['multiple'] && count($items) < $field['widget']['max_number_images']) {The workaround until a patch gets rolled (I'm working on one) is that multiple images has to be enabled and the limit has to be set above 0. Setting to 0 causes this to be false.
#2
Here's a quick patch. I'm pretty sure this isn't the ideal way to handle it and it introduces a text bug for settings where multiple's aren't allowed but it's a place to work from.
#3
#4
Another patch that resolves the issue of text for situations where multiples are not allowed or they are and the max images is set to 1. The old "replace" text has been removed because we never get there. The only way to replace a single image now is to delete the existing image first.
#5
http://drupal.org/node/225584 has been marked as duplicate of this issue.
#6
subscribing, great to see a patch for this rolled into a rc7 release.
#7
This doesn't handle the problem of not being able to see the uploaded image if there is a single image and because the edit form is missing it is not possible to delete the image.
#8
I might be wrong, but I think it's as simple as this patch. Works for me for single and multiple fields, tested with 0, 1 and 2 max allowed images. I think we should keep the "replace" code because it's better for usability - single-image fields should always replace the file instead of asking the user to manually delete it.
#9
Argh, wrong patch - missing the !$field['multiple'] condition that makes single-value fields work for all "max images" settings.
#10
Ok, one more: the above patch had one condition too much, and removing that made it clear that there are too much negations in the if condition. Therefore, let's swap the if and else branches, and have a "positive" condition instead.
#11
+1 this is much better...
I did add back the handling for the special case where multiple is selected with a limit of 1 image so the text doesn't read "1 images" etc.
#12
There's actually a function in Drupal that allows strings to be translated while respecting singular/plural differences (even those for languages with more than two plural forms): format_plural() is what we want to use here. Probably by making "1 image" a placeholder for the main string and thus keeping only one copy. I don't have the time to roll the patch right now, feel free to do it first :D
#13
jpesto's patch is working for me. Thanks! The bug really disrupted my project.
#14
I hate to nitpick, but it's jpetso. First the t, then the s.
(...ok, nitpicking actually doesn't feel so bad to me ;).
#15
Thanks jpetso! I'd spent too much time looking for that the other day but with the function got it.... here's a patch with it included.
#16
Ok, this patch thing just didn't work as I wanted it to yesterday. Here's the patch that I had previously wanted to upload (but uploaded a copy of the one before that instead), plus incorporating format_plural as posted by JBrauer. I believe this patch is now what we want to apply.
@JBrauer: please generate your patches with the "-u" option, that makes nice readyble "unified" diffs with pluses and minuses.
#17
Actually, it's
cvs diff -up. See http://drupal.org/patch/create for detailed information about Drupal patch standards.#18
Thanks for the patch tip... I think this one is done correctly (with -up).
One change from patch #16 -
$max_images <= count($items)as the test instead of:
count($items) < $max_imagesOtherwise we get true and display the you must delete form all the time.
#19
Oops... I did that in yesterday's patch, but forgot to redo it while reconstructing that one (and of course failed to test the last one that I posted). So your version is now really nice and correct - I'll justify marking it as RTBC with the new "reviewed and tested by the community" meaning. Great teamwork :)
#20
I have tested this latest patch and it solved my problem. Bravo.
#21
my apologies.
#22
Patch works for me too!
#23
Patched and tested on Drupal 5.7 with ten different sites. Patch appears to have worked great!
#24
committed to DRUPAL-5--2. Thanks guys!!
#25
Automatically closed -- issue fixed for two weeks with no activity.