Problem

  • Form constructor functions have no way to figure out the original $form_id name for which the form is currently built for.

Goal

  • Provide the original $form_id in $form_state['build_info']['form_id'], just like 'base_form_id' is provided there.

Details

  • With D7, we introduced the $form_state['build_info'] container, which holds all the build information for a form; e.g., the $args originally passed into drupal_get_form(), the base form ID in case the $form_id does not map directly to a function name and hook_forms() is invoked, etc.
  • We totally forgot to record the very basic $form_id itself.
  • A form constructor function is invoked directly from drupal_retrieve_form() and only gets $form and $form_state as arguments. The $form_id is not supplied anywhere.
  • Only after the form has been constructed, drupal_prepare_form() adds the $form['form_id'] element to the form. Before drupal_prepare_form(), both $form and $form_state are relatively "empty".
  • If a form constructor calls into other functions, then those other functions do not have a way to figure out the original $form_id, unless it was explicitly specified by the form constructor somewhere in $form or $form_state.

Related issues

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Status: Needs review » Reviewed & tested by the community

Seems like an obvious win. Code is clean. Makes sense for this to be the very first thing stuffed into $form_state by this function (I applied the patch and looked at the full context, and there's definitely no more obvious place for this). Bot is happy. RTBC.

Thanks,
-Derek

sun’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
tim.plunkett’s picture

For those confused (like me) by sun's comment, this was committed to D8 in #1599554-36: Tutorial/guidelines for how to convert variables into configuration

sun’s picture

Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
608 bytes

Identical patch.

David_Rothstein’s picture

Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Needs work

This makes a lot of sense to me, but shouldn't we be documenting this new parameter?

drupal_build_form() has a massive list of $form_state parameters documented, including everything inside 'build_info', so I think we need to list this there too.

Moving back to 8.x for that first, since I think if we're going to add this to the data structure in a stable Drupal 7 release we should make sure it's correctly documented when we do so.

sun’s picture

Status: Needs work » Needs review
FileSize
710 bytes

Docs for D8.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

Dries’s picture

I believe I need to commit both #0 and #6. However, #0 no longer applies, and #6 has the documentation only. Looks like a re-roll may be in order?

sun’s picture

#0 was committed as part of #1599554: Tutorial/guidelines for how to convert variables into configuration, so only #6 is required for D8.

Dries’s picture

Version: 8.x-dev » 7.x-dev

Committed the documentation to 8.x. Moving back to 7.x for the documentation.

sun’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.14 KB

#4 was not committed to D7 yet.

Attached is the combined backport.

David_Rothstein’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me; the only new thing from the previous RTBC patch is the code comment, so moving it back to RTBC and I should be able to commit it in a few days.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

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