Problem/Motivation
ElementInfoManager::buildInfo() calls each form element plugin's getInfo(), then unconditionally overwrites #value_callback for any FormElementInterface implementation with [$definition['class'], 'valueCallback']. Any element that declares its own #value_callback in getInfo() has that value silently discarded.
This masked a separate bug in \Drupal\Core\Render\Element\File::getInfo(). It declared #value_callback as [[static::class, 'valueCallback']], an incorrectly double-wrapped array. Because the overwrite above always ran first, this invalid value was never actually used, so the bug had no visible effect until now.
This surfaced while wiring \Drupal\file\Element\ManagedFile::getInfo() to declare its own #value_callback (see #3616654: [PP-2] Convert ManagedFile::valueCallback() logic into an injectable service), since that declaration was likewise being discarded.
Steps to reproduce
Proposed resolution
Change ElementInfoManager::buildInfo() to only assign the default #value_callback when the element's own getInfo() has not already declared one. Remove the redundant, malformed #value_callback declaration from File::getInfo(), since File::valueCallback() already exists and is exactly what the default assignment would wire up.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3616663
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:
- elementinfo-11x
changes, plain diff MR !16921
- 3616663-elementinfomanager-overwrites
changes, plain diff MR !16700
Comments
Comment #3
kim.pepperComment #4
nicxvan commentedI think this deserves a CR. I took a quick pass, but it feels pretty bare, so it could use another pass.
This is a pretty straightforward fix and has a test so I think it should be ready once we square away the CR.
Comment #5
kim.pepperAdded a CR. We don't usually add them for bug reports, but there might be some unexpected callback being executed after this change where it wasn't previously.
Comment #6
nicxvan commentedYes, but if you squint it's also kind of a new feature too.
We can leave it unpublished if the committers don't think we need one.
I marked my cr obsolete, yours is much better.
I think this is ready! L
Comment #8
catchI think the CR is more useful than some other ones we've made, went ahead and published.
Committed/pushed to main and 11.x, thanks!
Comment #12
catchIt looks like this caused phpstan failures on 11.x, reverting for now - let's do a backport MR to confirm.
Comment #15
catchBackport looks good now.
Committed/pushed to 11.x, thanks!
Comment #18
catchOr not let's try that again.