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.

Files: 
CommentFileSizeAuthor
#5 drupal-1591726-5.patch1.59 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 39,097 pass(es).
[ View ]
#2 drupal8.form-tree-parents.2.patch1.61 KBsun
PASSED: [[SimpleTest]]: [MySQL] 36,587 pass(es).
[ View ]
drupal8.form-tree-true.0.patch1003 bytessun
PASSED: [[SimpleTest]]: [MySQL] 36,602 pass(es).
[ View ]

Comments

Title:Missing form_id, form_build_id, and form_token when #tree is TRUE on $form itselfMissing 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.

StatusFileSize
new1.61 KB
PASSED: [[SimpleTest]]: [MySQL] 36,587 pass(es).
[ View ]

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.

Status:Needs review» Reviewed & tested by the community

Good to go.

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.

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.59 KB
PASSED: [[SimpleTest]]: [MySQL] 39,097 pass(es).
[ View ]

Rerolled.

Status:Needs review» Reviewed & tested by the community

Thanks!

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.

Issue summary:View changes

Updated issue summary.