Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Certain form #validate or #element_validate handlers need to be able to alter the $form structure.
They currently cannot, since $form, resp. $elements, is not passed by reference.
One popular example: A form containing a #required element, and a CAPTCHA element. The user successfully champions the CAPTCHA, but fails to enter something into the #required element. In this case, the validation handler of the CAPTCHA needs to be able to alter $form or $element to hide the CAPTCHA.
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | drupal.form-validate-reference.18.patch | 8.92 KB | sun |
| #14 | drupal.form-validate-reference.14.patch | 8.92 KB | sun |
| #4 | drupal.form-validate-reference.4.patch | 7.94 KB | sun |
| #3 | drupal.form-validate-reference.3.patch | 6.63 KB | sun |
| drupal.form-validate-reference.0.patch | 1.12 KB | sun | |
Comments
Comment #1
sunAdditionally, to continue that example:
In case the validation fails, a validation handler needs to be able to reset the element's #value to an empty string. In this example, if the submitted CAPTCHA value was incorrect, the user has to input a new CAPTCHA anyway, so the validation handler needs to alter the element in the form structure, not just $form_state['values'].
Clarification on the last bit: $form_state['values'] can be altered via http://api.drupal.org/api/function/form_set_value/7, but doing so has no effect on the actual form element that will be rendered.
Comment #2
Dries CreditAttribution: Dries commentedI agree that this is much needed. It has been painful having to work around this. I think it could be useful to document this change though (if only to make sure we don't revert it) and/or to write a small test for it. Otherwise looks RTBC.
Comment #3
sunAdded tests. Looks ready to fly for me.
Comment #4
sunWe can go one step further and even test a validation handler that sets #access => FALSE on an element and the value should persist throughout subsequent validations + rebuilds.
This is currently not the case. Interestingly, this brings me back to #622922: User input keeping during multistep forms and seems to be a pretty simple test-case for it.
Comment #5
Dries CreditAttribution: Dries commented'alteration' is a fun word, a bit archaic even. It's correct though so no need to change this.
I think this is RTBC.
Comment #6
webchickI agree this would be convenient. Only thing is I'm pretty sure this was done as a deliberate decision so that validation was just for validating, since we used to be able to do changes to forms in #validate back in 4.7/5-ish.
It'd be nice to have chx or Eaton chime in here.
Comment #7
sunReally? I just checked in D5 as well as 4.7.6 and it wasn't possible in there.
If it was possible earlier, I guess that it was either mistakenly removed, or it was perhaps removed because people mistakenly thought they would be able to alter the submitted form values by altering $element['#value'], i.e. not realizing that any alterations to the form structure will of course only have an effect on the form in case of a validation error, in which case the form will be automatically rebuilt.
Are you sure you don't mean form_set_value(), which allows to alter the $form_state['values'], as mentioned in #1 already?
Comment #8
effulgentsia CreditAttribution: effulgentsia commentedIf it was an intentional decision to not pass $form by reference, I suspect it was because if a validation function makes a change to $form and there are validation errors, then the changed $form isn't cached, so when the user re-submits it, the input would be processed against a stale cached form build. If this change goes in, we would need to change the caching logic in drupal_build_form(). I suspect the intended way to accomplish the use-case described in this issue is to use $form_state['rebuild']. However, there are currently some major problems with form rebuilding. One of them is being worked on in #622922: User input keeping during multistep forms. But another one is that drupal_build_form() only calls drupal_rebuild_form() if there were no validation errors, which seems to cancel out any possibility for a validation function to trigger a rebuild. However, image_style_form_add_validate() and book_admin_edit_validate() attempt (unsuccessfully because of previous sentence) to trigger a rebuild from a validation error. So, basically, WTFs all over the place.
Seems like we need help from chx, Eaton, or another FAPI guru to explain exactly what form rebuilding was intended for, and if we need to fix it to work from validation handlers as well as submit handlers, and if not, then how else can the CAPTCHA use-case in this issue be solved.
Comment #9
sunTrying to trigger a rebuild via $form_state['rebuild'] is an entirely different issue. On that note, setting $form_state['rebuild'] AND setting a form validation error is senseless, because form errors overrule any rebuilding - the form is straight output with the same populated values again.
With regard to this - and that's what this patch is doing - form validation handlers need to be treated like #process handlers. They are the last instance in the form processing flow, and they need to be able to still alter the form elements.
However, as with everything else in form_builder() and drupal_process_form(), all of these handlers always run from scratch again, also for cached forms. Which means: A #process or #element_validate or #validate handler, which performs alterations to a form, has to execute its own conditional tasks each time.
Only the original $form, as returned by the form constructor function, plus any alterations by hook_form_alter() implementations, are cached.
Comment #10
effulgentsia CreditAttribution: effulgentsia commentedWorks for me. Thanks for the explanation. I'll keep that in mind when working on the rebuild issues.
Comment #11
sunOn a related note, comment_form() is entirely incompatible with form caching.
Why? As mentioned before: When caching is enabled, the constructed $form plus any alterations are cached and NOT rebuilt. When caching is enabled, comment_form() never has a "Submit" button.
Tagging.
Comment #12
chx CreditAttribution: chx commentedI am not changing the status but I am mailing Heine and want to talk to him before let this through.
Comment #13
chx CreditAttribution: chx commentedSuppose you change the form. if you do not change the form_state['values'] then you might get unvalidated form_state['values'] to the form submit.
However, this is not new. after_build already let's you do that. So I am blessing this if you add a warning to drupal_validate_form warning not to do that. (Optionally you can commit such a warning to the fapi docs in contrib for after_build too).
Edit: i did not need to send the mail to Heine because as I was trying to explain what caused me such unease I finally realized it and this was it.
Comment #14
sun@chx: Does the added PHPDoc meet your approval?
Comment #17
Dries CreditAttribution: Dries commentedFound a small typo:
"an validation error" should be "a validation error". I can fix that when I commit the patch though.
Comment #18
sunFixed that typo.
Comment #19
chx CreditAttribution: chx commentedI believe we are good to go.
Comment #20
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks sun and chx.