Spin-off from #597108: Consistent array key syntax in $form_state (underscores vs. spaces):

1) hook_forms() is "almost" a form registry, but only invoked in case a $form_id does not exist as function.

2) #571086: Allow to specify a 'wrapper callback' before executing a form builder function will allow to define a "wrapper callback" for a $form_id in hook_forms(). For that to work, the $form_id must not exist as callable function, so hook_forms() is invoked at all.

3) #93587: Using drupal_not_found() in "pulled" forms results in double 404 message identifies a common problem: Anyone and anything can invoke drupal_get_form($form_id) and there won't be any access checks. And there is no way a form builder could tell that access is denied (403) - or a form wasn't built at all (404).

4) #367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'] introduced $form_state['include_file'], which is required to rebuild a form from cache on a different path. Currently, that value is auto-determined based on the menu router item of the current path - but that certainly is very limiting.

5) Forms should be treated as a representation of a certain object. node_view() displays a node, node_form() is the same node, but represented in an edit form.

I believe we could solve many problems by introducing and requiring hook_form_info() for all forms:

/**
 * Implement hook_form_info().
 *
 * This is almost the same as hook_forms(), but of course, without $form_id.
 */
function hook_form_info($args) {
  $forms['node_form'] = array(
    'form callback' => 'node_form', // optional?
    'form arguments' => array('static arg'), // Like current hook_forms(), prepended to passed $args.
    'wrapper callback' => 'node_form_wrapper', // optional.
    'access callback' => 'node_edit_access',
    'access arguments' => array($args[0]),
    'file' => 'node.pages.inc',
    // ...more?
  );
  return $forms;
}

Comments

sun’s picture

Version: 7.x-dev » 8.x-dev
sun’s picture

joachim’s picture

> 5) Forms should be treated as a representation of a certain object. node_view() displays a node, node_form() is the same node, but represented in an edit form.

That's an interesting idea. I'm not sure what benefits it gives us, but it sounds good :)
How would you represent that in hook_form_info / hook_entity_info?

aidanlis’s picture

Moving toward standard CRUD helpers for objects makes sense, both at the database level and in the UI.

fago’s picture

Subscribing + agreed that we need something like that.

Some issues I ran over with the current API:

1. There is currently no way to know which include file to include to make use of a form as this info lives in hook_menu(). This makes cleanly re-using forms quite impossible without having extra-knowledge.
We should decouple the include-loading part from the menu system so forms can be rendered and processed on its own.

2. In case you want to re-use a form it's fun that you cannot go with the base-form id. You need to find out the specific form-id (i.e. incorparting the entity bundle) first, however the used pattern may differ so you cannot generically know it. Then, once you figured the specific-form id out the system has to figure out the base-form-id for you first, so it can find the implementation.
Thus, it's an unnecessary crux to have to use the specific form-id when calling drupal_get_form(). Instead we should go always with the base form id for identifying a form and let the form implement-er come up with the specific form id afterwards.

eaton’s picture

Sun, thanks for jumping onto this. It's something that's been debated in back-channelly discussions for a number of years now, ever since hook_forms() made its way in in D5 as a workaround. Short version? I'm very much in favor of it. I'm not familiar enough with some of the config management decisions that have been made to know whether info hooks are the right pattern for this going forward, but that approach (or the modern drupal equivalent) would be a big improvement.

The current magic-naming Form definition mechanism we use is a holdover from the "hooks are everything" days of Drupal 4.6, when FAPI was originally created. There was a great deal of concern that forcing people to explicitly define their forms would receive a ton of pushback. The result was a large number of magic function names. Over time we realized that we needed more explicit specifications, and those were grafted on top of the existing magic naming conventions -- submit and validate hooks, form building functions, etc. In my experience, it's a real point of confusion for people who are learning the API.

I'll reply quickly to a couple of the specific points:

1) hook_forms() is "almost" a form registry, but only invoked in case a $form_id does not exist as function.

Yep, this sucks. We should change hook_forms() to hook_form_info() or the D8-appropriate equivalent, and require that all forms be defined with it. The current ugly mechanism of 'dynamic' forms parsing unimplemented form_id's in hook_forms() could be replaced by a combination of hook_form() and a unique key specified when the form is built, or possibly the form build parameters.

2) #571086: Allow to specify a 'wrapper callback' before executing a form builder function will allow to define a "wrapper callback" for a $form_id in hook_forms(). For that to work, the $form_id must not exist as callable function, so hook_forms() is invoked at all.

I'll look closer at that issue in particular, but it seems relatively unobjectionable. This is the kind of stuff that should really be in the form's metadata rather than the definition structure for the form itself. IE, the 'wrapper callback' would make sense to move into the proposed info hook that would replace hook_forms().

3) #93587: Using drupal_not_found() in "pulled" forms results in double 404 message identifies a common problem: Anyone and anything can invoke drupal_get_form($form_id) and there won't be any access checks. And there is no way a form builder could tell that access is denied (403) - or a form wasn't built at all (404).

The problem of forms access information being on the menu definition rather than the form itself has tripped up quite a few developers. Mirroring the access callback and param structure from hook_menu would probably make sense, though we'd have to think through the implications. Should menus inherit the callbacks of the forms they display? Would devs need to specify the access information in both hook_menu and hook_forms/hook_form_info/etc?

4) #367567: Use AJAX framework for "Add more" / load include file containing form / introduce $form_state['build_info'] introduced $form_state['include_file'], which is required to rebuild a form from cache on a different path. Currently, that value is auto-determined based on the menu router item of the current path - but that certainly is very limiting.

Another thumbs up. Explicit is better than implicit when it comes to this kind of stuff, and if we want sensible defaults, we should provide them rather than assuming them based on other data.

5) Forms should be treated as a representation of a certain object. node_view() displays a node, node_form() is the same node, but represented in an edit form.

I'd say this should be optional rather than inherent, and it definitely shouldn't be automatic. There are lots of situations where forms are representative of a cluster of objects, or a part of one object, or no particular objects at all depending on what their intent is. I think it would be useful if each entity's info also specified the form ID to edit or create it, but I'm worried about what the ripple effects would be if it went any farther than that.

In the code below, I'd actually recommend that 'form callback' be required. Make it *EXPLICIT* rather than magical, or we'll be right back at square one. While we're at it, I wouldn't mind eliminating automagical submit and validate functions, either. ;-)

/**
 * Implement hook_form_info().
 *
 * This is almost the same as hook_forms(), but of course, without $form_id.
 */
function hook_form_info($args) {
  $forms['node_form'] = array(
    'form callback' => 'node_form', // optional?
    'form arguments' => array('static arg'), // Like current hook_forms(), prepended to passed $args.
    'wrapper callback' => 'node_form_wrapper', // optional.
    'access callback' => 'node_edit_access',
    'access arguments' => array($args[0]),
    'file' => 'node.pages.inc',
    // ...more?
  );
  return $forms;
}
Crell’s picture

If we're going as far as turning forms into a defined thingie in an info hook that defines lots of callbacks explicitly...

In Drupal 8, that's a plugin. Looking at the code at the bottom of #11, my mind immediately says "Oh, ctools procedural plugins, which in core D8 should be OO plugins using the system that's been developed."

Since I want to make at least the form API engine itself OO so that it can be managed through the DIC, that fits very well. Form-as-object can exist even alongside an array-based definition if we want to keep that for now.

plach’s picture

geek-merlin’s picture

@crell #12:
>Since I want to make at least the form API engine itself OO

is there an issue for that (could not find)
and
will we finally get render objects for what we know as render arrays?

TIA!

Crell’s picture

I don't think there's an issue yet. There's some chatter across various issues in CMI. I doubt that we'd convert render arrays to render objects without a massive refactoring anyway.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

apaderno’s picture

Version: 8.9.x-dev » 9.1.x-dev

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.