Forms API and #required file fields
The Forms API does not currently support file fields with #required => TRUE (and is documented as such in the Forms API Reference). I suggest that with a little effort it can and that to make the API more consistent and useful it should. I discovered this limitation because 4.7 flexinode allows file, image, and perhaps other file-based fields to be specified as required and attempts to impose the required-ness by setting #required => TRUE in the form element. The result is a flexinode type that cannot be created or edited because all #required file fields are always reported as not filled in, even if they are. Clearly, this is a bug in flexinode, not in the Forms API which is just behaving as specified. However, the fact that flexinode tried to work this way argues that the Forms API SHOULD support it, because it is a logical and consistent way to interpret Forms API semantics. The reason it doesn't work is that form_builder() in form.inc only looks in $_POST for values to store in $form['#value']. Since file upload data is in $_FILES, $form['#value'] is never set for file fields. When it is #required, _form_validate() reports an error. form_builder() already has a switch based on $form['#type']. We could add something like: case 'file': $form['#value'] = $_FILES['edit']['name'][$form['#key']]; break; The new $form['#value'] would then have a filename if one were uploaded or nothing if one were not. This would be sufficient to support the #required field. Since currently $form['#value'] never contains anything for file fields, storing a filename there can't hurt anybody. This requires that $form['#key'] be set so a form element knows its own name in the $_POST and $_FILES arrays. But that is easy enough to do during the recursion in form_builder(): // Recurse through all child elements. $count = 0; foreach (element_children($form) as $key) { // tell each element its own key name $form[$key]['#key'] = $key; .... } Comments? I'll implement this if the change will be accepted. Barry
Supporting #required for files get strange with previews.... After a file is attached _FILES[name][key] is empty.
On 02 Jun 2006, at 7:16 PM, Barry Jaspan wrote:
The Forms API does not currently support file fields with #required => TRUE (and is documented as such in the Forms API Reference). I suggest that with a little effort it can and that to make the API more consistent and useful it should. quite frankly. the implementation of the file field needs to be completely redone.
The issue with the file field is that it provides the input, but it never has a value, hence FAPI has nothing to validate the required against. The only thing it can even remotely handle at the moment is if you have uploaded a file in the previous request. As each of the modules that implement the file field implement the back end handling differently, it's a lost cause. What really needs to happen is that all the magic that happens in upload.module needs to become part of the file field. The files table needs a secondary 'realm' key, and you need to be able to upload a file simply by adding a file field, with a realm, and optionally a unique identifier (node id, user id, whatever) To do this in the form layer is no simple task however, since we lack context. Also, all the preview stuff , and not having a proper id at the right time, makes all this stuff far more complex than it should be. =( -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
On Fri, 2006-06-02 at 23:14 +0200, Adrian Rossouw wrote:
On 02 Jun 2006, at 7:16 PM, Barry Jaspan wrote:
The Forms API does not currently support file fields with #required => TRUE (and is documented as such in the Forms API Reference). I suggest that with a little effort it can and that to make the API more consistent and useful it should. quite frankly. the implementation of the file field needs to be completely redone.
The issue with the file field is that it provides the input, but it never has a value, hence FAPI has nothing to validate the required against.
The only thing it can even remotely handle at the moment is if you have uploaded a file in the previous request. As each of the modules that implement the file field implement the back end handling differently, it's a lost cause.
What really needs to happen is that all the magic that happens in upload.module needs to become part of the file field. The files table needs a secondary 'realm' key, and you need to be able to upload a file simply by adding a file field, with a realm, and optionally a unique identifier (node id, user id, whatever)
To do this in the form layer is no simple task however, since we lack context. Also, all the preview stuff , and not having a proper id at the right time, makes all this stuff far more complex than it should be. =(
I've been working on filesystem.module... Its primarily a scratch space where I can work out what I think the file handling should be. It solves several problems by removing the nid field from the files table and using the uid... It also provides a form #type of filesystem_file which does all the upload black magic. currently you can upload a file with it. I'm looking for people interested in developing it with me. Things I really want are 1) a complete set of simpletests (any takers? I've made the skeletons.. ) for both the low level api, and the module. 2) people to give me feedback on how I'm implementing filehandling. .darrel.
-- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
FYI.... Check out http://groups.drupal.org/node/225 -- discussion on reworking core upload to make it immensely scalable. On Jun 2, 2006, at 3:47 PM, Darrel O'Pry wrote:
I've been working on filesystem.module... Its primarily a scratch space where I can work out what I think the file handling should be.
It solves several problems by removing the nid field from the files table and using the uid...
It also provides a form #type of filesystem_file which does all the upload black magic.
On 3-Jun-06, at 8:03 AM, Laura Scott wrote:
FYI....
Check out http://groups.drupal.org/node/225 -- discussion on reworking core upload to make it immensely scalable.
Err....no, not really. Some thoughts on techniques if you need massive scalability, plus some messing about with how can we utilize Amazon S3. There is continuing discussion in the File API working group on various techniques. -- Boris Mann Vancouver 778-896-2747 San Francisco 415-367-3595 SKYPE borismann http://www.bryght.com
On Jun 3, 2006, at 11:44 AM, Boris Mann wrote:
On 3-Jun-06, at 8:03 AM, Laura Scott wrote:
FYI....
Check out http://groups.drupal.org/node/225 -- discussion on reworking core upload to make it immensely scalable.
Err....no, not really. Some thoughts on techniques if you need massive scalability, plus some messing about with how can we utilize Amazon S3.
I stand corrected. What I get for lazy reading. Laura
On Jun 3, 2006, at 5:58 PM, Laura Scott wrote:
On Jun 3, 2006, at 11:44 AM, Boris Mann wrote:
On 3-Jun-06, at 8:03 AM, Laura Scott wrote:
FYI....
Check out http://groups.drupal.org/node/225 -- discussion on reworking core upload to make it immensely scalable.
Err....no, not really. Some thoughts on techniques if you need massive scalability, plus some messing about with how can we utilize Amazon S3.
I stand corrected. What I get for lazy reading.
Forgive the run-on response... Here's what gave me the impression:
And the problem with filemanager/attachment is that it needs to be reworked as a patch/patches against core. It's fine (perhaps) for specific web sites, but we want this general functionality in Drupal core. (http://groups.drupal.org/node/225#comment-1582)
So NOT reworking upload, but getting some sort of scalable fileserving into core. +++1. I would like to help however I can as soon as I can. Laura
participants (5)
-
Adrian Rossouw -
Barry Jaspan -
Boris Mann -
Darrel O'Pry -
Laura Scott