API page: http://api.drupal.org/api/drupal/includes%21form.inc/function/drupal_bui...
Enter a descriptive title (above) relating to drupal_build_form, then describe the problem you have found:
The description of the $form_state['rebuild'] should read "However, a validation handler may already set $form_state['rebuild'] to cause the form processing to bypass submit handlers and rebuild the form instead, when there are no validation errors."
Instead of "However, a validation handler may already set $form_state['rebuild'] to cause the form processing to bypass submit handlers and rebuild the form instead, even if there are no validation errors."
Since the form is not rebuilt if there is a validation error. The current description implies that the form will be rebuilt whether or not a validation error occurs.
Comments
Comment #1
jhodgdonWell, I think the "even if" was meant to imply "Normally, a validation handler only takes and action if there is a validation problem, but in this case, even though there is no validation problem, it has taken an action". I agree that the wording could be improved though. I'm not sure the suggested wording is the best idea though? Maybe we should just take out the "when there are no validation errors" clause?
In any case, it needs to be fixed in 8.x first and then backported when that fix is accepted.
Comment #2
lemming commentedI'm okay different wording as long as it makes it clear that the rebuilt only occurs when there are no validation errors.
I've found that when there is a validation error, the form does not get rebuilt, regardless of how this flag is set. This is confirmed by the logic in the drupal_process_form() function starting line 925
The current wording "even if" implied that the rebuilt action would be taken, despite valiation errors. It needs to be clear that rebuild only happens if no validation errors occurred. Unless of course, the behavior gets changed in Drupal 8.
Comment #3
jhodgdonAh, I did not know that! Thanks for the detective work.
So... This whole description of the rebuild parameter is very awkward. Maybe we can make it all clearer?... I think the word "already" is throwing me off -- can we get rid of that and/or rewrite the whole sentence so it is both correct and clear?
Comment #6
jp.stacey commentedIs
FormState::$rebuildthe equivalent to $form_state['rebuild'] in 8.x?https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Fo...
It looks like the equivalent logic to that quoted in comment #2 is around line 616 in
FormBuilder:https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Fo...
i.e:
There's a large comment block immediately before this logic, which I think is trying to solve the same thing as the original documentation bug reported.
Because form state has become an object with behaviours, I think 8.x and 7.x need different strategies:
8.x:
1) Improve the docblocks of
FormState::setRebuild()/FormState::isRebuilding()to explain more about the parameter, but that it's basically a "dumb" property with no behaviours of its own, adding a@see FormBuilder::submitFormto reference the actual behaviour.2) Improve the comment block immediately before that logic, to explain what the logic is doing and
@see FormState::...7.x:
1) Improve the docblock of
drupal_build_form()based on 8.x's improvements.Comment #7
jp.stacey commentedSorry, I should add: does that analysis make sense? Moving this to 8.4.x
Comment #15
wdseelig commentedThis is old [Drupal 7], but should be said somewhere.
If you set $form_state['rebuild'] to TRUE in your submit routine, then a return from that routine WILL cause the form to be rebuilt [i.e., your mymodule_form($form, &$form_state) function will be re-run. HOWEVER, $form_state['rebuild'] will be set to FALSE before you do this rebuild. So if you want to change the way the form looks on a rebuild, you have to test something other than $form_state['rebuild']. In my use case, I'm testing $form_state['input']['mytextfield'] to see if I'm building for the first time or rebuilding.