Empty file upload w/fieldsets

Thanks, this sounds very possible due to the way that file uploads are handled. HTML handles file elements differently from any other POST data, not even including any information about the file in the $_POST data. Instead, all information is found in the $_FILES array. Then I'm not sure if it's PHP's handling of files or Drupal's, but the actual location of the file component in the form is lost, so Webform does it's best to put the file back into the correct location in the form.

I'll get to this eventually, but if any savvy dev wants to take a stab at correcting this I'll gladly take patches. Thanks for the report.

dharmatech, thanks for the patch! Unfortunately, this doesn't apply at all to the latest 2.x version. Could you download the development version or checkout the latest copy of the DRUPAL-5--2 branch from CVS?

This patch looks like it would introduce several problems with some complex Webforms. When iterating through fieldsets and setting the values to empty arrays, this only gets the first level of fieldsets, not nested fieldsets (Webform does most operations recursively). Does this affect the functionality of the patch?

Also, the removal of $local_value['pid'] == $cid from the conditional will break webforms where the same formkey is used in different fieldsets, say a situation like this:

Mailing address [mailing_address fieldset]
- street
- town
- state
- zip

Shipping address [shipping_address fieldset]
- street
- town
- state
- zip

Without that conditional, the values from the second fieldset will not be used, because Webform will find the street, town, state, and zip keys from the first fieldset and use them instead.

As for initializing the fieldsets as empty arrays, the foreach should take care of it shouldn't it?

I'm not sure, if you had a fieldset inside a fieldset, the inner fieldset would be located at $form_values['submitted'][$parent_component['form_key']][$component['form_key']]. So, it doesn't look like it would be initialized to an empty array. I'll take a further look at the next patch.

We should definitely add more validation that checks that file keys are unique, PHP doesn't provide us with the nice array structure that we get for $_POST in $_FILES, so they do indeed need to be unique.

Anyway, yes, that's another bug, but let's handle it in another issue.

Sorry I just haven't had a chance to review yet. I'm booked up but I'll try to have a look soon. There are several other fixes that necessitate rolling out a 2.1 version, I'll makes sure that we figure this one out before then.

Yeah, sorry I didn't include this, I was afraid of potential problems it could cause and I'll need to test it a bit more extensively. If you reroll against 2.x dev that would be great.

This patch works in my testing (thanks for putting up with my slow response so far). Could we try to take an alternate approach with this patch though? There are frequent complaints of file components not working in multipage forms, lack of an ajax upload, and failing when a single required field is missing. All of these solutions are going to be handled separately, but all of them are going to require... a companion hidden field (probably containing the file path of the upload file in Drupal 5, or the File ID in Drupal 6). If we add a companion hidden field to all file upload right now (just leave the value empty), this would solve our empty fieldset problem and set us up for future fixes.

I just wanted to note that this bug also affects the 6.x branch (there doesn't seem to be any way to mark a bug as applying to more than one version). The workaround seems simple enough, but it might be worth including something about it in the readme or the project page if it's not able to be fixed soon -- I was searching for this bug in the 6.x issue queue and only found this page with the workaround when I thought to search all versions.

I got around to looking at this again finally. I think this new solution rmakes code more efficient and removes a few hacks. It should also theoretically make it easier to handle uploads in multi-page forms, which would be nice to eventually support.

Rather than adding in more loops, this approach makes such loops unnecessary by providing some information in the $_POST array where the file will ultimately be positioned. A hidden field is output in addition to the file field, then the values from the saved file inserted into the hidden fields location when running the processing. This makes it so that the file data stays in the same place as before, and we eliminate all the guesswork in trying to find where files should belong.

Drupal 5 and 6 patches attached.

I went ahead and committed these patches. Please reopen if these caused new problems or still experience file uploads not being saved when the file element is in a fieldset by itself.