Problem/Motivation

WebformElementManager::getSortedDefinitions is like this:

public function getSortedDefinitions(?array $definitions = NULL, $sort_by = 'label') {

However, CategorizingPluginManagerTrait has this:

public function getSortedDefinitions(?array $definitions = NULL, string $label_key = 'label') {

Likewise, the same Webform class has:

public function getGroupedDefinitions(?array $definitions = NULL, $label_key = 'label') { but the core trait has:

public function getGroupedDefinitions(?array $definitions = NULL, string $label_key = 'label') {.

Steps to reproduce

Proposed resolution

I'll create an MR that adds the type hint and change the name from sort_by to label_key for the first method, and adds the type hint for the second. There appear to be about seven similar cases.

Remaining tasks

Test the MR.

Issue fork webform-3618362

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

Comments

tolstoydotcom created an issue.

tolstoydotcom opened merge request !928

mably’s picture

mably commented

This MR is confirmed working on Drupal 12. I applied its diff to webform 6.3.0 as a Composer patch on the next-major CI lane of the Personal Data Vault module, alongside the three other webform Drupal 12 fixes, and ran that module's whole suite against core 12.x-dev (dev-main, b1b76c0): 436 tests pass, 0 failures, 0 errors.

Without it the element manager's signatures are incompatible with the core 12 trait it inherits from, which is a fatal at plugin-manager construction rather than a warning, so this is one of the changes webform needs before it can run on Drupal 12 at all.

For anyone else testing: the diff applies cleanly to released 6.3.0, and because it only adds a native string type that existing callers already satisfy, it needs no Drupal 10 conditional. Evidence: https://git.drupalcode.org/project/pdv/-/jobs/11717269

AI-Generated: Yes (Claude Code was used to help draft this comment and to run the Drupal 12 verification it describes. I reviewed it before posting. The merge request is not mine.)