Adding callback_form_element_process (#process) to Form Element removes system callbacks

Created on 6 July 2022, over 2 years ago
Updated 7 January 2025, 3 months ago

Problem/Motivation

I need to add a custom callback_form_element_process to some form widgets, but this makes other callbacks ignored. Initially this issue is reported in #3294468: Select2 widget fails validation when callback_form_element_process is added to Form Element but seems it's the bug in Drupal Core.

Steps to reproduce

1. Get the clean Recommended Drupal project with "Articles" node bundle, having "field_tags".
Create 2 taxonomy tags: "Tag 1", "Tag 2"
Create an article "Article 1", assign "Tag 1" to it.

2. Set the widget type for "field_tags" to "Select".

3. Set a breakpoint on the function Drupal\Core\Render\Element\Select::valueCallback() (on the first line, for example).

4. Opens the node edit form and stop on the breakpoint.
Check the value of the $element['#process'] array:

var_export($element['#process'], true)
"array (
  0 => 
  array (
    0 => 'Drupal\\Core\\Render\\Element\\Select',
    1 => 'processSelect',
  ),
  1 => 
  array (
    0 => 'Drupal\\Core\\Render\\Element\\Select',
    1 => 'processAjaxForm',
  ),
)"

We have two callbacks, that's ok.

5. Add a callback_form_element_process hook to the "field_tags" field using function my_module_field_widget_form_alter like this:

function my_module_field_widget_form_alter(&$element, \Drupal\Core\Form\FormStateInterface $form_state, $context) {
  if ($context['items']->getFieldDefinition()->getName() !== 'field_tags') {
    return;
  }
  $element['#process'][] = '_my_module_field_widget_form_process';
}

function _my_module_field_widget_form_process(array $element, FormStateInterface $form_state, array $form) {
  return $element;
}

And clear the cache.

6. Open the Node edit form again, stop on the breakpoint, check the $element['#process'] array:

var_export($element['#process'], true)
"array (
  0 => '_my_module_field_widget_form_process',
)"

Oops! We have the our custom callback in the array, but the two system callbacks were disappeared!

Visually on the form this leads at least to disappeared "- None -" item in the Select widget.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

11.0 🔥

Component

forms system

Created by

🇦🇲Armenia murz Yerevan, Armenia

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇩🇪Germany donquixote

    Also, with this proposed change, it is no longer possible to fully replace or remove the callbacks from '#type'.

    A solution could be to introduce a placeholder callback, so that we can:

    $element = [
      '#type' => 'radios',
      '#process' => [
        'my_custom_process_callback',
        Element::PROCESS_CALLBACK_PLACEHOLDER,
        'my_custom_late_process_callback',
      ],
    ];
    

    and we get this:

    $element = [
      '#type' => 'radios',
      '#process' => [
        'my_custom_process_callback',
        [Radios::class, 'processRadios'],
        'my_custom_late_process_callback',
      ],
    ];
    

    The placeholder should be a valid callback that, if not replaced, just returns the first parameter.
    At the same time, it should be a constant that is easy to compare against or look up in an array.

    This leaves some design choices to make:
    - What exactly is the placeholder value? If it is a static method, how do we call the class and method?
    - Where do we do the placeholder replacement? I was thinking of a new service ElementTypePopulator, which handles all of the '#type' replacement, so that we don't have to clutter up Renderer and FormBuilder.

    (I did post some proof of concept code in a slack convo, but I am still too undecided about the class naming.)

  • 🇩🇪Germany donquixote

    It seems for all the callback types #process, #after_build, #pre_render and #post_render, the placeholder can be a simple identify function.
    fn ($x) => $x
    (but as regular named function or static method, not as closure)

  • 🇬🇧United Kingdom joachim

    Can't Element::PROCESS_CALLBACK_PLACEHOLDER be a method that just does this:

    1. look at #type in the render array it's working on
    2. get the Element plugin class for that #type
    3. call ELEMENTCLASS::getInfo()
    4. get the callback names from the returned info array
    5. call them

  • 🇮🇳India mdsohaib4242

    To resolve this, you need to ensure that your custom process callback is added while preserving the existing callbacks. Modify your my_module_field_widget_form_alter function to append the custom callback to the existing #process array, rather than replacing it.

    function my_module_field_widget_form_alter(&$element, \Drupal\Core\Form\FormStateInterface $form_state, $context) {
      if ($context['items']->getFieldDefinition()->getName() !== 'field_tags') {
        return;
      }
      
      if (isset($element['#process'])) {
        $element['#process'][] = '_my_module_field_widget_form_process';
      }
      else {
        $element['#process'] = ['_my_module_field_widget_form_process'];
      }
    }
    
  • 🇩🇪Germany donquixote

    @joachim

    Can't Element::PROCESS_CALLBACK_PLACEHOLDER be a method that just does this:

    1. look at #type in the render array it's working on
    2. get the Element plugin class for that #type
    3. call ELEMENTCLASS::getInfo()
    4. get the callback names from the returned info array
    5. call them

    Yes, this would also do the trick.
    However, it does require a call to \Drupal::service(), if it is to be a constant.
    Or alternatively, we could make this a class with __invoke() and DependencySerializationTrait. Because these things tend to get serialized. Actually not so bad!

  • 🇩🇪Germany donquixote

    call ELEMENTCLASS::getInfo()

    Just to clarify, we would not call it like this, because it is not a static method.
    Instead, we would call `$info = $this->elementInfoManager->getInfo($element['#type'])` as we already do in FormBuilder and Renderer.
    I am working on something, just need to add tests..

  • 🇩🇪Germany donquixote

    I created a new MR with these placeholder objects following the idea by @joachim.

    I want to add tests, but not sure yet where to put them.

  • Pipeline finished with Failed
    3 months ago
    Total: 203s
    #393061
  • 🇩🇪Germany donquixote

    Here we go, some unit tests.
    Let's discuss if this is a good direction, and how we can refine it.

  • Pipeline finished with Failed
    3 months ago
    #393078
  • 🇩🇪Germany donquixote

    We can't use readonly in combination with DependencySerializationTrait.

    I am setting to "Needs review" for one round of feedback, but I am sure we will have to change some parts.

  • Pipeline finished with Failed
    3 months ago
    Total: 129s
    #393139
  • Pipeline finished with Failed
    3 months ago
    Total: 137s
    #393140
  • Pipeline finished with Failed
    3 months ago
    Total: 123s
    #393156
  • Pipeline finished with Failed
    3 months ago
    #393157
  • 🇩🇪Germany donquixote

    Perhaps we want to move the ElementProcessCallback* classes to `Drupal\Core\Form\` namespace?
    For now I had it in `Drupal\Core\Form\Render\Callback`, because form element type plugins are typically in the same namespace as render element type plugins.
    I ma not changing it now because I want to see more feedback first.

  • 🇩🇪Germany donquixote

    Can't Element::PROCESS_CALLBACK_PLACEHOLDER be a method that just does this:

    Yes, this would also do the trick.
    However, it does require a call to \Drupal::service(), if it is to be a constant.
    Or alternatively, we could make this a class with __invoke() and DependencySerializationTrait. Because these things tend to get serialized. Actually not so bad!

    Actually one more problem with this approach.
    If multiple operations (e.g. form alter hooks) want to add '#process' callbacks, while preserving the element type default callbacks, they need to make sure that the element defaults are not executed more than once.

    If the callback is a constant, we can easily check for in_array().
    But if it is an object?
    Actually, in_array(*, *, FALSE) applies weak object comparison, and this actually seems to work in this case.
    But I am a bit afraid of weak object comparisons, I don't trust them.

  • 🇩🇪Germany donquixote

    If multiple operations (e.g. form alter hooks) want to add '#process' callbacks, while preserving the element type default callbacks, they need to make sure that the element defaults are not executed more than once.

    The following pattern works in most cases:

              $element['#process'] ??= [new ElementTypeProcessCallback($this->elementInfoManager)];
              $element['#process'][] = 'my_process_callback';
    
  • 🇺🇸United States nicxvan

    @donquixote which MR? The newer one in draft?

    @mdsohaib4242 that is exactly what the code in the issue summary is doing, that's why this issue exists.

  • 🇩🇪Germany donquixote

    I notice that '#pre_render' does not allow object with __invoke() method, if it is not a closure.

    See 📌 Support object with __invoke() for #pre_render. Active

    (We can eventually handle #pre_render in a separate issue, but for now I find it interesting to include it here as proof-of-concept, to see the bigger picture of where this is going.)

  • 🇩🇪Germany donquixote

    @donquixote which MR? The newer one in draft?

    Well, I am proposing to use the newer MR which is in draft.
    Imo the old MR is a dead end, because it breaks existing behavior and makes it impossible to fully remove/replace the callbacks from #type.

    But I don't want to remove the old one yet, unless everybody agrees on the new direction.
    Having both means we can discuss which direction is preferable.

  • 🇩🇪Germany donquixote

    @mdsohaib4242
    Actually this does not do anything, because at the time you add your own '#process' callback, the '#process' array is still empty.
    The would-be-replaced callbacks are added later by the form builder which is after hook_form_alter() based on $element['#type'].

  • The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Production build 0.71.5 2024