Required flag on file forms breaks on validation

Backlink to open flexinode issue: http://drupal.org/node/42762

(21:46:24) vertice:  the problem is that during the upload process, the stuff gets removed OUT of the form system
(21:46:28) vertice:  and gets stuffed in the session
(21:46:36) vertice:  and every file api module does this on it's own

This is not the place neither the time to fix file API. Everybody on its own. I am outta here. If the change gets in, I will doc the change.

I am not too happy about it, but unless somethign better comes up, I will commit it.

This works. It fixes the issue.

but:
* Its a HACK, spelled in capitals all along. Whether or not we allow such a hack to get in to solve this issue, or not, is not up to me.
* Will it not open up a bit of a security hole? I mean: if we are making exceptions that will allow certain "things" not to pass by the validation, we are moving on slippery ice (or whatever the saying is).

but:
again: it solves the issue. Tested, and confirmed with a few flexinode file fields.

thanks. but I dare not give it a + or - 1. Such issues, IMO are for Dries to decide.....

I forgot to mention, that there are other ways around this issue. For example image.module does this. They are not solutions, but simply ways to not run into this form api bug.

* do not set the required flag on a file field.
* then validate it yourself in a validation hook.

The downside is, that the "required" mark (by default the *) is not printed on the file form.

Maybe we should simply document that you can't use required with file fields. Also, Ber coul dpass an attribute to his home-brew validation routine.

Since this issue does not affect core, I'd like to not patch core, but to simply update the docs. Opinions?

Like the previous posters, I'm not crazy about the idea of introducing such a hack into core.

I may have found a potential workaround, at least as far as flexinode goes (but should be generally applicable).

Can you guys take a look at http://drupal.org/node/42762, #12. It seems to work fine... Am I missing something that makes this a bad approach?

It might be better to ensure that that type of approach is supported via forms system, rather than just bypass required checking for file fields.

What do you think?

I'm OK with this patch for 4.7.0. We can make it work properly for 4.7.1 IMO

We agreed that the solution is that I am doccing right now that #type file can't be required.

I am sure we agreed to lower the priority and move this to the "file system" component. "wont fixes" will be forgotton. This should not be forgotton.

For it remains a bug.

I marked http://drupal.org/node/64984 as a duplicate. There is a patch on that issue.

Attached the patch by quicksketch on #64984

Patch does not apply and a conversion of the patch does not seem to work.

Is a patch applied for 4.7? It seems that is should be, as flexinode is still better at handling files and many sites are using it. This seems like a sad oversight... apply a patch to core that fixes this, please (easy for me to say, since I'm more of a bug hunter than a bug fixer). Just throwing my support at this as loudly as possible.

This is still an issue in 5.x and, presumably, HEAD.

This prevents programmatically creating node content with drupal_execute on imagefields, for example, when the field's #required property is set to TRUE.

...still an issue.

I'm sure it goes without saying, but stating that the #required setting is not allowable for file types is a pretty lousy solution.

it may be a lousy option to say #required is not allowable for file types, but at the moment #required doesn't work for them and I've just spent hours trying to figure out where in my script I was going wrong not getting it working. I had already done my own workaround (also suggested above) of having my own #validate function and doing stuff in there, and not having the field #required, but the fact that the docs say #required is allowed is very confusing. For now at least, the docs must be changed to make #required not an option for 'file' form fields, otherwise other coders will face the same headache in figuring out where they've gone wrong in implementing it.

@jakeg: Please update the docs. Anyone with a CVS account may do so. The file's in contributions/docs/developer/forms_api_reference.html, I believe.

@webchick: thanks, I didn't realise/think I had access. Have updated the docs:
http://cvs.drupal.org/viewvc.py/drupal/contributions/docs/developer/topi...

Subscribing

Patch from post #13 appears to be working in my 5.7 instance.

Ha, this was "fixed" 3 years(!) ago in #64984-4: File Uploads fail when field is set to "required", but it never made it into core. Sad, sad. Here's a reroll of my old patch, not yet tested, but just putting this on the radar.

Updated patch that has been tested.

@jakeg: thanks for fixing up the documentation, it was really important IMHO and I have experienced this headache so I totally agree with #18

subscribing

seems like we could use a unit test for this...

Fixes a bug that has pestered since FAPI 1.0. A test would be nice but squashing the bug is more important.

i'd disagree since the whole point of the test is making sure it says unbroken.

Wow!! Great to see this bug fixed! But I agree that a test is mandatory.

Also, is there by chance a nicer way to get this value than $elements['#parents'][0]? that's not really at all intuitive.

On testing, the previous patch went too far and didn't throw an error when a required file field was actually missing a file. Updated patch, including a test.

The last submitted patch failed testing.

The last submitted patch failed testing.

@Scott - the bot tested today and this patch will not apply cleanly :(

Updated patch for most recent 7.x-dev release.

The last submitted patch failed testing.