Regression: hook_preprocess_form_element_label() does not have #type defined

Created on 24 December 2015, about 9 years ago
Updated 5 July 2024, 6 months ago

I need to add a class to form_element_label if it is of type radios or checkboxes. Since D8 this is no longer possible as $variables['element']['#type'] seems not defined in hook_preprocess_form_element_label().

function mytheme_preprocess_form_element_label(&$variables) {
  $element = $variables['element'];

  switch ($element['#type']) {
    case 'radios':
    case 'checkboxes':
      // Add custom label class to element labels.
      $variables['attributes']['class'][] = 'my-label';
      break;
  }
}

Can I add the #type somehow back in my theme or can core add it back, please?

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Theme 

Last updated 2 minutes ago

Created by

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

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.

  • 🇳🇿New Zealand quietone

    There has been no discussion here for 8 years and much has changed since then. However, since I don't know the theme system I am asking if this is still relevant.

    Is this relevant to Drupal 10?

    Since we need more information to move forward with this issue, I am setting the status to Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

    Thanks!

  • Status changed to Active 8 months ago
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Although this issue was filed when Force Awakens was #1 at the box office. I confirmed this is still an issue in mid-2024. As the long lifespan of this issue suggests, it's probably not an urgent matter but it would me nice for label preprocess to know the type of element they belong to.

    Comment #2 seems like the right place to get this attribute added. If there are still memory issues like the ones reported 8 years ago, perhaps some xdebugging can sniff out the culprit.

    As mentioned above, this will of course need some tests. I'm guessing there is an existing test that can get a few lines added so it's not necessary to write a whole new dang test to see if #type is there.

  • The change that we are making to include '#type' in the keys for array_intersect_key is causing an infinite loop. This is because $element contains a reference to $variables['label'], and modifying $variables['label'] inside the function is triggering the function to be called again, leading to an infinite recursion.
    The way i was able to figure that out was because adding

    unset($element['#type']);
      $variables['label'] += array_intersect_key($element, array_flip(array('#type', '#id', '#required', '#title', '#title_display')));

    code makes it work load the page which was not loading earlier.
    Not sure if the solution in this patch is the best solution. If it is i can add the test for the same with the patch applied.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    his is because $element contains a reference to $variables['label'], and modifying $variables['label'] inside the function is triggering the function to be called again, leading to an infinite recursion.

    This was spotted in #2 as well. The next step should be figuring out why adding #type is causing this recursion.

    What is it about this additional array property causing this recursive loop? Until we know that, we won't know if the solution in #2 is the right approach. Knowing this might reveal a way to get #type in without the need to rename - it may also reveal an underlying bug or optimization opportunity.

    You don't need to fully know the render system to know how this works. The process of investigating these types of bugs is what leads to the fuller understanding. I recommend looking for logic that checks for either being present or not empty. These regexes can help you find those uses:

    • isset\(.*\['#type'\]\)
    • !empty\(.*\['#type'\]\)

    . There will still be several results, but you can rule out any uses found inside a module, theme or test since the recursion happens on a minimal install. Look at what the remaining uses do and put breakpoints in places you believe the recursion might be happening. There's a good chance you'll find the cause, but if you don't you can still document which regex-matching areas you checked and what your findings were - that information is still a helpful step forward.

  • After some debugging and reading about the rendering system i came up with what could be the reason for infinite loop.
    - In the Drupal theming system, variables like $variables['label'] are passed to template files for rendering. When you're preparing variables in the
    template_preprocess_form_element function, you're essentially preparing data to be used in the template file associated with the form element.
    - When you add #type directly to $variables['label'], you're essentially associating the type of the form element with the label of the form element.
    - The rendering of form elements in Drupal is a recursive process. When rendering a form element, Drupal may need to render its children or
    related elements, which in turn may need to render their children, and so on. If, during this process, Drupal encounters the #type property
    associated with a label, it may interpret this as a signal to render another form element, leading to a recursive loop.
    - Drupal's rendering system may keep trying to render the label as a form element due to the presence of #type, leading to excessive resource
    usage and potentially crashing the system.
    So according to what i understood with the above mentioned points i think the approach we are trying to use in #2 🐛 Regression: hook_preprocess_form_element_label() does not have #type defined Active may not be the correct way to add type to label.We should keep that as a separate property.

    I recommend looking for logic that checks for ['#type'] either being present or not empty. These regexes can help you find those uses:

    I was not completely able to get what needs to be done in this so i tried with what i understood and documented my understanding on this.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    If we're running into an infinite recursion issue, I recommend first finding where the recursion is occurring. This

    We already know that there is an infinite recursion problem when #type is set in the render array.

    This is most likely due to a condition that executes code only when #type is present. The two most likely ways PHP would check for the presence of #type are isset($foo['#type']) or !empty($foo['#type'])

    We can use a regular expression to find places where these conditions might be set
    [^\!]isset\(.*\['#type'\]\) Will look for an "isset" call without a preceding exclamation point, followed by an array path that ends #type. The regex could be even more precise, but just this narrows things down to only 9 places this could be happening.


    Most of the results can be ruled out because they are in modules/themes that don't need to be running for this error to occur, they are in tests, or it belongs to a class for a feature that is unrelated to the issue.

    Expand the results and you can rule out additional possibilities:

    This leaves only 5 possible places, making it pretty easy to check each with an xdebug breakpoint, and see which condition is getting hit infinite times.

    If I place the breakpoint in the Renderer.php condition, it appears to loop infinitely when it hits an element with a form_element_label #theme.

    Without even having to know that much about the render system, just a little bit of PHP / regex / xdebug we can see the recursion is happening in this block in Renderer.php

     // If the default values for this element have not been loaded yet, populate
        // them.
        if (isset($elements['#type']) && empty($elements['#defaults_loaded'])) {
          $elements += $this->elementInfo->getInfo($elements['#type']);
        }
    

    Is this best accomplished by changing the condition, or by changing what this->elementInfo->getInfo($elements['#type']); returns, or something entirely different?

  • Pipeline finished with Failed
    7 months ago
    Total: 528s
    #176901
  • Pipeline finished with Success
    7 months ago
    Total: 563s
    #177144
  • Status changed to Needs review 7 months ago
  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    Can we update the issue summary to the standard issue template.

    Have not reviewed yet.

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    MR has a question about why this particular solution works and evidence that it will not result in other problems. This may require a bit of research, which is a great way to get more familiar with how core works.

    This will also need a test to confirm #type is getting passed to the label elements. Forms in general have many tests, to there may be an existing test that you can use this so it's only necessary to add a few lines instead of having to write an entirely new test.

  • Pipeline finished with Success
    7 months ago
    Total: 595s
    #177780
  • Pipeline finished with Canceled
    6 months ago
    Total: 497s
    #216404
  • Pipeline finished with Canceled
    6 months ago
    Total: 499s
    #216403
  • Pipeline finished with Success
    6 months ago
    Total: 549s
    #216419
  • Pipeline finished with Success
    6 months ago
    Total: 577s
    #216421
Production build 0.71.5 2024