Discovered in #1324618: Implement automated config saving for simple settings forms:

Problem

  • When setting $form['#tree'] = TRUE; and $form['#parents'] = array('foo'); on the top-level $form itself, Form API is unable to find its own required form_id, form_build_id, and form_token values in the submitted form values.

This is an edge-case usage scenario, for which I doubt it makes sense to write tests for.

CommentFileSizeAuthor
#5 drupal-1591726-5.patch1.59 KBtim.plunkett
#2 drupal8.form-tree-parents.2.patch1.61 KBsun
drupal8.form-tree-true.0.patch1003 bytessun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Missing form_id, form_build_id, and form_token when #tree is TRUE on $form itself » Missing form_id, form_build_id, and form_token when using custom #parents and #tree = TRUE on $form itself

Sorry, forgot the totally important detail about also having custom #parents on the $form itself.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal8.form-tree-parents.2.patch1.61 KB

Discussed with @chx that enforcing explicit #parents is better than #tree FALSE.

We also discussed whether the other form processing/validation code shouldn't rather check for $form['form_token']['#value'] instead of relying on submitted form values, but that would inherently add a dependency on the $form structure. Out of the two choices, enforcing #parents made more sense.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

Good to go.

dries’s picture

dries commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

I reviewed this patch but it was not 100% clear. Given that both chx and sun approved it, and that all tests pass, I committed it anyway.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new drupal-1591726-5.patch1.59 KB

Rerolled.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Thanks!

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Fixed

I had trouble getting my head around this one too. Ultimately, I think it just comes down to the fact that there's lots of code in the form API that looks for these things in the top level of $form_state['input'] or $form_state['values'], so we need to ensure that they will actually always be in the top level.

Discussed with @chx that enforcing explicit #parents is better than #tree FALSE.

Out of curiosity, why? I would have found this code a lot easier to understand with #tree set to FALSE, because that's the standard way to accomplish the above. Is the concern that someone altering the form might be likely to change #tree by accident?

Anyway, this patch works fine as is, so although it seems like there might be room for a followup to clarify the code comments, etc.... I committed it to 7.x. Thanks!

http://drupalcode.org/project/drupal.git/commit/121b348

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.