States are processed and added to the wrong attribute since #1427838

Created on 28 November 2024, about 2 months ago

Problem/Motivation

Since 🐛 password and password_confirm children do not pick up #states or #attributes Active was committed, states are now incorrectly applied to (at least) 'item' form elements.

Specifically the intention of that issue was to change this code:

    // Elements of '#type' => 'item' are not actual form input elements, but we
    // still want to be able to show/hide them. Since there's no actual HTML
    // input element available, setting #attributes does not make sense, but a
    // wrapper is available, so setting #wrapper_attributes makes it work.
    $key = ($elements['#type'] == 'item') ? '#wrapper_attributes' : '#attributes';
    $elements[$key]['data-drupal-states'] = Json::encode($elements['#states']);

To something that includes the password confirm element, and indeed early on in the work for that issue we had this code:

    // Elements of '#type' => 'item' are not actual form input elements, but we
    // still want to be able to show/hide them. Since there's no actual HTML
    // input element available, setting #attributes does not make sense, but a
    // wrapper is available, so setting #wrapper_attributes makes it work.
    $key = ($elements['#type'] == 'item' || $elements['#type'] == 'password_confirm') ? '#wrapper_attributes' : '#attributes';
    $elements[$key]['data-drupal-states'] = Json::encode($elements['#states']);

However, after some discussion it was decided to not target the elements specifically 🐛 password and password_confirm children do not pick up #states or #attributes Active , but try to identify the types of elements that would likely use wrapper attributes instead of attributes.

This resulted in the following code being committed:

    // Elements that are actual form input elements, use '#attributes'.
    // In cases like 'item' that are not actual form input elements or
    // those like 'password_confirm' that have child elements,
    // use #wrapper_attributes.
    $key = (($elements['#markup'] ?? FALSE) === '' && ($elements['#input'] ?? FALSE) === TRUE) ? '#wrapper_attributes' : '#attributes';
    $elements[$key]['data-drupal-states'] = Json::encode($elements['#states']);

This code broke a unit test, which was then fixed in a follow up commit where the fix was to make the testing form 'item' an input element, with no #markup, this is fine for the unit test, but there are plenty of places in core alone that set '#markup' to be a non-empty string, and it's still valid to expect states to apply on those elements, as they did before.

Steps to reproduce

Have a form with some #states set on a regular, plain '#type' => 'item' form element, with #markup a non-empty value, observe that the attribute isn't correctly added to the wrapper on the page. And thus the states doesn't work.

Proposed resolution

I'm not sure if the suggested conditional was better/correct!

In the first instance, we should fix the tests to include non-empty #markup, and then we can try to improve the code trying to detect the case here.

Remaining tasks

  1. Open a MR to fix the tests.
  2. Fix the conditional, to get the tests passing.
🐛 Bug report
Status

Active

Version

11.1 🔥

Component

forms system

Created by

🇬🇧United Kingdom steven jones

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

Merge Requests

Comments & Activities

Production build 0.71.5 2024