Hexadecimal (color) values (#EEE) don't work as FAPI select/radios array keys

Created on 20 October 2023, 8 months ago
Updated 16 January 2024, 5 months ago

Problem/Motivation

Hex color values (like #000000) can not be used as keys for Drupal FAPI select / radios.
The options are simply not showing up.

    $form['color'] = [
      '#type' => 'radios',
      '#title' => $this->t('Color'),
      '#default_value' => $this->configuration['color'] ?? '',
      // @todo For unknown reasons keys with a hexadecimal value are sorted out by Drupal:
      '#options' => [
        'red' => $this->t('Red'), // Works, option is displayed
        '#00FFFF' => $this->t('Red Hex'), // Doesn't work, option is missing
        htmlentities('#00FFFF') => $this->t('Red Hex with htmlentities()'), // Doesn't work, option is missing
        Html::escape('#00FFFF') => $this->t('Red Hex with Html::escape()'), // Doesn't work, option is missing
      ]],
      '#required' => TRUE,
      '#weight' => 998,
      '#description' => $this->t('Select a color for this portlet.'),
    ];

Steps to reproduce

Try the code above in a form and see the options are missing

Proposed resolution

  1. Find the root cause
  2. Fix the root cause
  3. Write tests to prove
  4. Release

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

11.0 πŸ”₯

Component
FormΒ  β†’

Last updated about 15 hours ago

Created by

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

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

Comments & Activities

  • 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 does Element::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 (see Radios.php::processRadios($element)). The Element::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 to if (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]))) {
  • πŸ‡¨πŸ‡¦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!

Production build 0.69.0 2024