\Drupal\Core\Form\FormHelper::processStates checks whether the #type property is 'item'. However, the #type property is (afaik) optional and can be undefined for render items that aren't form elements. Compare with \Drupal\Core\Render::doRender, which checks:

    if (isset($elements['#type']) && empty($elements['#defaults_loaded'])) {
      $elements += $this->elementInfo->getInfo($elements['#type']);
    }

So \Drupal\Core\Form\FormHelper::processStates might also need an isset() check there. Since type-less render elements aren't form elements, they also need the #wrapper_attributes set.

Steps to reproduce

Proposed resolution

TBD

Remaining tasks

Update IS
Add test
Review

CommentFileSizeAuthor
#22 2846320-nr-bot.txt179 bytesneeds-review-queue-bot
#2 0001-Issue-2846320-drupal_process_states-causes-notices-o.patch1.08 KBcburschka

Issue fork drupal-2846320

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka created an issue. See original summary.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Status: Active » Needs review
FileSize
0001-Issue-2846320-drupal_process_states-causes-notices-o.patch1.08 KB

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

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now 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.

LoMo’s picture

LoMo CreditAttribution: LoMo as a volunteer commented
Status: Needs review » Reviewed & tested by the community

@cburschka The patch looks good to me and currently still applies on the 8.4.x-dev branch. I think this is basic enough (and logical enough) that it's reasonable to set status to RTBC, but it could be good on issues like this to provide "steps to replicate". Then I'd feel better about saying I'd "tested" the fix.

Anyway, I do think that your logic and fix both make sense.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler as a volunteer commented
Status: Reviewed & tested by the community » Needs review

#type is actually not optional. You can omit it for '#type' => 'markup' by simply using #markup, but other than that it always needs to be specified. Can you elaborate on how you hit this problem?

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...

Specifying #theme is usually mutually exclusive with specifying #type...

I can't quite recall the exact code that triggered the E_NOTICE right now, but it was probably a form containing a generic render element. I'll see if I can get a simple reproducible example.

LoMo’s picture

LoMo CreditAttribution: LoMo as a volunteer commented

@tstoeckler You are probably right, but there are plenty of examples in core of such "defensive" coding that use isset() before attempting to access something like $element['#type'], and core/lib/Drupal/Core/Render/RendererInterface.php includes the following comment:

   *   - If this element has #type defined and the default attributes for this
   *     element have not already been merged in (#defaults_loaded = TRUE) then
   *     the defaults for this type of element, defined by an element plugin,
   *     are merged into the array.

… so one might think that the #type could be optional. In any case, I do feel that it's always better practice to use code that tests whether an array element exists before attempting to access its value. ;-)

@cburschka, assuming #type is non-optional, perhaps getting a notice is not really a bug. I still think your patch can't hurt.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler as a volunteer commented

I agree that we should be safe in either case, but I'm just not sure whether elements without a #type should have #attributes set or #wrapper_attributes.

Edit: On the other hand, in this particular case, I'm not actually sure whether it's valid that this is called on something that is not a render element. I don't think we support hiding regular output of a #theme element with #states.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented

Hi, I finally recovered the sample code that triggered this. Basically, we had an ordinary #theme element that needed to be toggled with a #state:


  $form['pattern'] = [
    '#title' => t('Pattern'),
    '#type' => 'textfield',
    '#states' => [
      'visible' => [
        ':input[name="pattern_enabled"]' => ['checked' => TRUE],
      ]
    ]
  ];

  $form['token_help'] = [
    '#theme' => 'token_tree_link',
    '#token_types' => 'all',
    '#states' => [
      'visible' => [
        ':input[name="pattern_enabled"]' => ['checked' => TRUE],
      ]
    ]
  ];

In addition to triggering the notice, this code isn't currently working, as there is no element to set the required attributes on.

This can be fixed by adding '#type' => 'item' to the element, without affecting any other functionality. I'm not sure if there is any way to improve this - implicitly giving everything with #states the fallback type 'item' sounds a bit too complex.

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

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now 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.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
CreditAttribution: chrisfromredfin at Redfin Solutions, LLC commented

I am seeing this with the token_tree_link theme element.

$elements = [
  '#theme' => 'token_tree_link',
  ...
];

Also:
$elements["#states"]["visible"]["input[name$=\"[third_party_settings][linked_field][linked]\"]"]

... so it seems like there's a need to process states but for a token tree (not a field element). This happens when hitting the gear from Manage Display of a node.

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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.

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.

needs-review-queue-bot’s picture

needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commented
Status: Needs review » Needs work
FileSize
2846320-nr-bot.txt179 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

mstrelan’s picture

mstrelan CreditAttribution: mstrelan at PreviousNext commented
Title: drupal_process_states causes notices on type-less elements. » FormHelper::processStates causes notices on type-less elements.
Issue summary: View changes
Issue tags: +Bug Smash Initiative

The function drupal_process_states() has moved to \Drupal\Core\Form\FormHelper::processStates, see Procedural function drupal_process_states() is deprecated.

arunkumark made their first commit to this issue’s fork.

arunkumark opened merge request !3707

arunkumark’s picture

arunkumark
Location Coimbatore
CreditAttribution: arunkumark as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review

The drupal_process_states() has been deprecated on Drupal 9.x version. The change record is available Procedural function drupal_process_states() is deprecated.

The patch has been rerolled and MR created to apply to the latest Drupal core version.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs tests, +Needs issue summary update

The proposed solution should be added to the issue summary.

A test case will also be needed.

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.

FeyP’s picture

FeyP CreditAttribution: FeyP as a volunteer and at werk21 commented
Status: Needs work » Closed (duplicate)
Parent issue: » #2700667: Notice: Undefined index: #type in Drupal\Core\Form\FormHelper::processStates()

Thanks for working on this issue. This looks very similar to #2700667: Notice: Undefined index: #type in Drupal\Core\Form\FormHelper::processStates() . The other issue doesn't have an MR already, but is older, has a patch with a kernel test, has way more followers and even more recent activity, so I'm going to close this one as a duplicate. It would be great, if you could continue working on this in the other issue. Feel free to reopen, if I missed something. Thanks again.