Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.I'm currently working on a complex FAPI element for file uploads (see the File project). It seems that FAPI currently maps just about everything on a one-to-one (or one-to-many) basis between form elements and form values. However the File upload widget I'm working on is reverse of the normal situation, where several form elements are required to produce a single value (a many-to-one relationship). The file upload widget contains a hidden element, and upload field, and several buttons, but it only returns exactly one value (a file FID). This patch simply prevents the Form API from attempting to use form_set_value() on elements within the file upload widget, since it shouldn't be attempting to set a value on a single integer as if it were an array of values.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | form_api_complex_return.patch | 971 bytes | quicksketch |
| cck_default_field.patch | 1.1 KB | quicksketch |
Comments
Comment #1
quicksketchArg, completely wrong patch. Here we are.
Comment #2
quicksketchComment #3
drewish commentedsubscribing.
Comment #4
quicksketchComment #5
quicksketchEaton and chx pointed me to the password_confirm element, which also needs this same functionality. The trick is setting the value in an #element_validate through form_set_value() rather than using a #value_callback. So I'm marking by design, since this is not necessary.
Comment #6
quicksketchOkay, after spending many hours trying to figure this out with form_set_value(), I don't using that approach will work for the needs of the managed_file element.
Here's the reason why it won't work for managed_file, but why it does work for password_confirm.
The basic code flow for building up a form with a complex widget in it is like this:
- Get the form definition array.
- Make sure that all #value fields are populated (run #value_callbacks if needed).
- Run #process functions, expanding out complex widgets.
- Run element-level validation (this is where password_confirm runs
form_set_value()).- Run form-level validation
- Run form-level submit handlers and save the data.
Now, in the case of complex widgets, using form_set_value() will only work if the entire form is going to be successfully be completed. If an error occurs or if the form just needs to be rebuilt, that form_set_value() doesn't do much good because while it affects $form_state['values'], it does not affect $form['element']['#value']. Meaning that the value of that field will be lost. We don't notice this in the password_confirm element, because password fields don't need default values anyway.
However, in the case of file uploads, loosing the file that you uploaded if there's an error is not an acceptable outcome. But worse yet, the form *has* to be rebuilt when clicking the Upload button, meaning the form_set_value() approach will not work at all for this need.
So I'd like to open this patch back up for consideration (still doesn't cause any problems and all tests pass), because #value_callbacks are run before the #process functions, this makes it so that $element['#value'] will contain the proper value and the process functions can respond appropriately to this value. In the case of form_set_value(), it is run too late in the cycle to do process functions any good.
Comment #7
webchickThis needs an ok from eaton/chx.
Comment #8
chx commentedSo yeah password confirm widget uses #process to expand the elements. If it would not be passwords but something else (say a bunch o' radios or checkboxes) then their value would be filled in by subsequent calls to form_builder. You can set #value in the #process callback or if that's not practical then you can do it in #after_build. We disliked #after_build and given that you only ever needed the #value of these in validate/submit, it was fine to reset the parent value in validation based on the already filled in children values. But again, we could have used #after_build. I know I wanted to deprecate that, but see, we never did. Can't you simply use that, then? Sorry for sending you on goose chases it was my fault for not understanding completely of what's going on but #after_build sees the children values and you can rather simply set #value (and call form_set_value) in there because it is called after the element and its children are fully built.
Comment #9
tic2000 commentedComment #10
dopry commented@chx,
iirc, you suggested #after_build when I was working on the same thing, but I was neverable to get it to work properly. I think it had something to do with validation errors. I put in over a week in pursuing #after_build based answers to this issue. Most of CCK's widgets can and I feel should be complex, and kludgy #after_build / more processing callbacks in an already complex form api does not make it any better for developers or the people who have to pick up their code when they can no longer work on it.
@quicksketch,
I'll try to review this week.
Comment #12
chx commentedWell then this needs a test which is also a use case so that I can like show you guys how to do it without the patch or I can see that it is, indeed, not feasible.
Comment #13
eaton commentedI'm sympathetic to dopry and quicksketch. Honestly, #after_build is crap and we wanted to remove it entirely -- until we realized there aren't provisions for what it actually needed to do. This isn't code freeze - if there is a legitimate path to making these kinds of operations simpler for developers without compromising security, it's a Good Thing.
Agreed that a simple "focused" test case for this would help quite a bit. Getting one's head deep into FAPI for something this tangly takes a big chunk of time...
Comment #14
quicksketchThis is no longer needed as I managed to make the form_set_value() in #element_validate approach work after Dries committed #546568: Consistently pass in $form_state to #value_callback functions. You can check it out the latest CVS version of the File project.