File Uploads fail when field is set to "required"

hystrix - May 22, 2006 - 22:16
Project:Drupal
Version:4.7.2
Component:forms system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:duplicate
Description

First of all, thank you Bèr Kessels (and whoever else helped) for releasing a 4.7 version of this highly used module.

Steps to reproduce:

- in custom content type, "add file" field and set to "required field"
- create content of the custom type, add file, click submit
- file uploads to $tmp, then drupal error message states "$fieldname is a required field"
- file never makes it from $tmp to $files

I was running the cvs version of flexinode, and I thought at first that this was related to issue 64478 and/or issue 40811. But in the new 4.7 release of flexinode, file uploads are working perfectly other than if they are set to be a required field.

#1

laryholland - May 27, 2006 - 20:03

This issue has been confirmed by myself as well. However, if you do not use the required field it works appropriately. Another way is by not using the field at all and then using "Attach New File" under File Attachment Options does work.

I suspect because there is a button that attaches prior to "form processing." The form processing aspect of the original feature seems to interfere with actually getting to the upload part of the code. I think this could be solved by adding the same "attach" button that is located on the File Attachment Option located at the bottom of the page to the content type feature.

#2

Mirrorball - May 28, 2006 - 21:53

I think this is a Drupal bug. I'm writing a module and I get the same error when I set a file upload field to 'required.'

#3

Mirrorball - May 28, 2006 - 21:55
Project:Flexinode» Drupal
Version:4.7.x-1.x-dev» 4.7.1
Component:Field type: file» forms system

#4

quicksketch - June 2, 2006 - 22:21
Version:4.7.1» 4.7.2
Status:active» patch (code needs review)

I've confirmed this problem and prepared a patch to fix the problem in form.inc. The patch includes an additional check on fields to check if they have '#type' == 'file' and then checks the upload accordingly.

AttachmentSize
form-inc-require-file-field.patch923 bytes

#5

bomarmonk - June 7, 2006 - 17:53

I tested this patch with flexinode, a required field, and Drupal 4.7.2. I encounter the folowing errors:

Warning: require_once(./includes/form.inc) [function.require-once]: failed to open stream: Permission denied in C:\Program Files\xampp\htdocs\mysite\includes\common.inc on line 1337

Fatal error: require_once() [function.require]: Failed opening required './includes/form.inc' (include_path='.;C:\Program Files\xampp\php\pear\') in C:\Program Files\xampp\htdocs\mysite\includes\common.inc on line 1337

I hope someone can get this working. If you are using flexinode, and have a file-type for users to upload, you don't want them uploading blanks. I appreciate the help!

#6

quicksketch - June 7, 2006 - 20:03

The patch works fine for me. Here's instructions for replacing the code manually: At line 212 of form.inc, replace

<?php
     
if ($elements['#required'] && empty($elements['#value']) && $elements['#value'] !== '0') {
       
form_error($elements, t('%name field is required.', array('%name' => $elements['#title'])));
      }
?>

with

<?php
     
if ($elements['#required'] && empty($elements['#value']) && $elements['#value'] !== '0') {
       
// One more specialized check for file type uploads, which store their value in the $_FILES array
       
if ($elements['#type'] !== 'file' || file_check_upload($elements['#parents'][0]) == false) {
         
form_error($elements, t('%name field is required.', array('%name' => $elements['#title'])));
        }
      }
?>

#7

bomarmonk - June 7, 2006 - 23:27

Yep, I hand edited the patch into form.inc, and no more problems. It's working like a charm with required file fields. There must be something off with the patch (or at least how I was applying it with CYGWIN-- no malformed patch messages or otherwise, though)

#8

bomarmonk - June 10, 2006 - 21:50

Ah, it seems that I still am required to upload an image, even if one is already attached to the flexinode for the required field. I still can edit these flexinodes without resubmitting images that are already uploaded. Not good!

#9

bomarmonk - June 19, 2006 - 06:14

Nevermind... I think I overlooked something, perhaps accidently reversing this patch and not realizing it. Applied the patch again and things seem to work. Can this be committed for the next maintenance release of Drupal 4.7? It seems fairly important.

#10

bomarmonk - June 19, 2006 - 10:38

However, are these related problems with this patch? I get the following error when resubmitting an edited flexinode (with an image field):

imagedestroy(): supplied argument is not a valid Image resource in C:\Program Files\xampp\htdocs\alpinecounty\includes\image.inc on line 229.

imagegif(): supplied argument is not a valid Image resource in C:\Program Files\xampp\htdocs\alpinecounty\includes\image.inc on line 300.

imagecopyresampled(): supplied argument is not a valid Image resource in C:\Program Files\xampp\htdocs\alpinecounty\includes\image.inc on line 226.

imagecreatetruecolor() [function.imagecreatetruecolor]: Invalid image dimensions in C:\Program Files\xampp\htdocs\alpinecounty\includes\image.inc on line 225.

#11

llyra - July 21, 2006 - 22:53

Installed patch and it seemed to fix this issue as when I hit "Preview" it took me to the preview screen rather than bringing up the required field error. All other fields are also required in this custom node type, but they are text and URL fields. On the Preview screen all fields appear EXCEPT the image field. So, from the preview page, I tried to upload the image again and now I get the same error: "Art Image field is required." So, I still cannot get an image to load to the custom 'image field' created using Flexinode. Any help please?? Thanks!

#12

nedjo - July 30, 2006 - 17:38
Status:patch (code needs review)» duplicate

Duplicate of http://drupal.org/node/59750. Maybe the patch should be moved there.

 
 

Drupal is a registered trademark of Dries Buytaert.