- Issue created by @Anybody
- π©πͺGermany Anybody Porta Westfalica
This is really a mystery, did anyone else run into this already?
I'm unsure if this might be related and the keys are run throuch xss:filter in core perhaps, sorting that options out entirely?
π The XSS filter should allow more HTML entities Needs work - π«π·France vbouchet
I partially reproduced the issue. I am not able to reproduce with select. However, I am able to reproduce with radios.
I debugged the
FormBuilder::doBuildForm()
method. At some point, it doesElement::children($element)
to process the children. For select, it does not do anything because the options are simply attributes of the element. However, for the radios, the options are individual elements (seeRadios.php::processRadios($element)
). TheElement::children($element)
method has a condition to process or not a children:if (is_int($key) || $key === '' || $key[0] !== '#') {
. Because of the last condition, the options starting with # are not returned by the method and then are not processed. I confirmed by hijacking the condition toif (is_int($key) || $key === '' || $key === '#FFFFFF' || $key === '#000000' || $key[0] !== '#') {
and the options are properly displayed.I see 2 options to fix this issue:
- Update the
Radios::processRadios($element)
method to update the few places the $key (which is the option value) is used by something else (for exemple 'option-' . $key) so it is not stripped later as it does not start with # anymore. - Update the Element::children($elements) method to compare the $key with the keys in $options. Something like
if (is_int($key) || $key === '' || $key[0] !== '#' || (!empty($elements['#options']) && isset($elements['#options'][$key]))) {
- Update the
- π¨π¦Canada Charlie ChX Negyesi πCanada
There are two things here.
1. Updating the radio element to change #return_value will not work with forms which define singular radio elements like core/modules/views/src/Plugin/views/style/Table.php does. If I were fixing this -- but I do not -- I would indeed change Element::children but I would rather add a new #also_children key and change Element::children to take it into account like this:
public static function children(array &$elements, $sort = FALSE) { // Do not attempt to sort elements which have already been sorted. $sort = isset($elements['#sorted']) ? !$elements['#sorted'] : $sort; $also_children = $element['#also_children'] ?? []; ... if (is_int($key) || $key === '' || $key[0] !== '#' || isset($also_children[$key])) {
and in processRadios just set it to a copy of #options. This way singular radio elements and heaven knows what else can also enjoy the fix (Table.php has machine name keys so it doesn't suffer from this but there are a lot of forms out there).
2. However, this functionality is old. Like, beyond ancient. This is so old it's not even my code, no, this is adrian's original form API, this was introduced in https://www.drupal.org/project/drupal/issues/29465#comment-341700 β how come we didn't come across this until now? Is there some unspoken expectation of radio options keys being machine name? I see a test for a less than sign for XSS purposes and so that expectation certainly doesn't stand. We certainly need much more robust testing to make sure that expectation didn't seep into theming and such. I asked on twitter and got confirmation for Drupal 6: https://twitter.com/jrbeaton/status/1747046198731538798 so it was possible to add these options for some time now and yet... I neither can recall nor can Google find any mention of this until now. Bewildering!