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)

  1. CCK Version: Content 5.x-1.6-1
  2. Image Field 5.x-2.0-rc6
  3. 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.
  5. Download method: Public files
  6. Imagefield configuration: all defaults. no limit to upload size/dimensions, not required, no default set, etc.
  7. Expected result: Image upload field
  9. Unexected result: no image upload field (see attached picture)
  10. 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.
CommentFileSizeAuthor
#18 imagefield-226270-18.patch1.94 KBjbrauer
#16 imagefield-fix-max-images.patch1.85 KBjpetso
#15 imagefield-226270-15.patch1.03 KBjbrauer
#11 imagefield-226270-11.patch1.17 KBjbrauer
#10 imagefield-fix-max-images.patch1.38 KBjpetso
#9 imagefield-fix-max-images.patch1.38 KBjpetso
#8 imagefield-fix-max-images.patch1.36 KBjpetso
#4 imagefield-226270-4.patch2.53 KBjbrauer
#2 imagefield-226270-2.patch1.15 KBjbrauer
imagefield.png13.18 KBjbrauer

Comments

jbrauer’s picture

jbrauer commented

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.

jbrauer’s picture

jbrauer commented
Status: Active » Needs work
StatusFileSize
new imagefield-226270-2.patch1.15 KB

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.

jbrauer’s picture

jbrauer commented
Title: Upload file area not shown for imagefield » Upload file area not shown with single image upload
jbrauer’s picture

jbrauer commented
Status: Needs work » Needs review
StatusFileSize
new imagefield-226270-4.patch2.53 KB

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.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

http://drupal.org/node/225584 has been marked as duplicate of this issue.

a_c_m’s picture

a_c_m commented

subscribing, great to see a patch for this rolled into a rc7 release.

jbrauer’s picture

jbrauer commented
Status: Needs review » Needs work

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.

jpetso’s picture

jpetso commented
Status: Needs work » Needs review
StatusFileSize
new imagefield-fix-max-images.patch1.36 KB

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.

jpetso’s picture

jpetso commented
StatusFileSize
new imagefield-fix-max-images.patch1.38 KB

Argh, wrong patch - missing the !$field['multiple'] condition that makes single-value fields work for all "max images" settings.

jpetso’s picture

jpetso commented
StatusFileSize
new imagefield-fix-max-images.patch1.38 KB

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.

jbrauer’s picture

jbrauer commented
StatusFileSize
new imagefield-226270-11.patch1.17 KB

+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.

jpetso’s picture

jpetso commented
Status: Needs review » Needs work

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

Anonymous’s picture

Anonymous (not verified) commented

jpesto's patch is working for me. Thanks! The bug really disrupted my project.

jpetso’s picture

jpetso commented

I hate to nitpick, but it's jpetso. First the t, then the s.

(...ok, nitpicking actually doesn't feel so bad to me ;).

jbrauer’s picture

jbrauer commented
Status: Needs work » Needs review
StatusFileSize
new imagefield-226270-15.patch1.03 KB

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.

jpetso’s picture

jpetso commented
StatusFileSize
new imagefield-fix-max-images.patch1.85 KB

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.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Actually, it's cvs diff -up. See http://drupal.org/patch/create for detailed information about Drupal patch standards.

jbrauer’s picture

jbrauer commented
StatusFileSize
new imagefield-226270-18.patch1.94 KB

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_images

Otherwise we get true and display the you must delete form all the time.

jpetso’s picture

jpetso commented
Status: Needs review » Reviewed & tested by the community

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 :)

bomarmonk’s picture

bomarmonk commented

I have tested this latest patch and it solved my problem. Bravo.

Anonymous’s picture

Anonymous (not verified) commented

my apologies.

Mark Theunissen’s picture

Mark Theunissen commented

Patch works for me too!

shane birley’s picture

shane birley commented

Patched and tested on Drupal 5.7 with ten different sites. Patch appears to have worked great!

dopry’s picture

dopry commented
Status: Reviewed & tested by the community » Fixed

committed to DRUPAL-5--2. Thanks guys!!

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.