ActiveForm dropDownList does not set the selected option properly if the underlying model's attribute is of boolean type

[adnandautovic]

What steps will reproduce the problem?

We have a model

class MyModel extends Model
{
    public ?bool $value = false;
}

and in some view, we render an an ActiveForm

<div>
    <?php $form = ActiveForm::begin([
        'action' => '/some/path',
        'method' => 'GET'
     ]) ?>

    <?php $form->field($model, 'value')->textInput()->label(
        'text')->dropDownList([true => 'Yes', false => 'No'], ['prompt' => 'Please select']) ?>
</div>

What is the expected result?

I expect the rendered dropDownList to have a selection of 'Yes' when $value is true and 'No' when $value is false.

What do you get instead?

The dropDownList always shows the prompt instead.

Additional info

Q	A
Yii version	2.0.45
PHP version	8.1.2
Operating system	Debian

The problem is to be found in

yii2/framework/helpers/BaseHtml.php

Lines 1916 to 1918 in 2874e07

	$attrs['selected'] = $selection !== null &&
	(!ArrayHelper::isTraversable($selection) && !strcmp($key, $selection)
	|| ArrayHelper::isTraversable($selection) && ArrayHelper::isIn((string)$key, $selection, $strict));

and the fact that arrays with boolean keys are indexed with 0 and 1 in php.
When getting to this point

gettype($key) // integer
$key // 0
gettype($selection) // boolean
$selection // false

and

!strcmp($key, $selection)

does not give back that they are equal.

[WinterSilence]

An array keys can be int or string. Your boolean keys converted to strings.

[adnandautovic]

An array keys can be int or string. Your boolean keys converted to strings.

Sorry, I'm not sure that I understand your comment.
In the code snippet I quoted, $key is read from the first argument of dropDownList, while $selection is read from the model's attribute.
The keys of the array [true => 'Yes', false => 'No'] are converted to integers by php (1 and 0). However, in the function renderSelectOptions, no conversion of $selection takes place if it is of boolean type.

I am under the impression that such a conversion would make sense here, since the conversion is also done for boolean array keys by php.

[WinterSilence]

['No', 'Yes'] converts to [['value' => '0', 'text' => 'No'], ['value' => '1', 'text' => 'Yes']], prepend prompt: [['value' => '', 'text' => 'Please select'], ['value' => '0', 'text' => 'No'], ['value' => '1', 'text' => 'Yes']]

(string)false is '' i.e. prompt value

[WinterSilence]

you can use AttributeTypecastBehavior to normalize values

[adnandautovic]

['No', 'Yes'] converts to [['value' => '0', 'text' => 'No'], ['value' => '1', 'text' => 'Yes']], prepend prompt: [['value' => '', 'text' => 'Please select'], ['value' => '0', 'text' => 'No'], ['value' => '1', 'text' => 'Yes']]

(string)false is '' i.e. prompt value

That does not appear to be quite correct. In your explanation, $key and $selection are mixed up. Let me try to make it more clear:
When I call dropDownList([true => 'Yes', false => 'No'], ['prompt' => 'Please select']), it is the same as if I had called dropDownList([1 => 'Yes', 0 => 'No'], ['prompt' => 'Please select']), because php immediately does the conversion of array keys.

The cast (string) false therefore never occurs.

Here,

yii2/framework/helpers/BaseHtml.php

Line 1914 in 2874e07

$attrs['value'] = (string) $key;

the $key is cast to a string, which yields '0' and that is fine. After this line,
$attrs = [ 'value' => 1, 'selected' => false ].

And updating 'selected' fails, because this comparison fails:

yii2/framework/helpers/BaseHtml.php

Line 1917 in 2874e07

(!ArrayHelper::isTraversable($selection) && !strcmp($key, $selection)

Since $key (being the integer 0) and $selection (being the boolean false) do not compare to equal with strcmp, which makes sense of course.

At the end of the function, the $lines array contains

$lines = [
'<option value="">Please select<\/option>',
'<option value="1">Yes<\/option>',
'<option value="0">No<\/option>']

The correct value would be

$lines = [
'<option value="">Please select<\/option>',
'<option value="1">Yes<\/option>',
'<option value="0 selected">No<\/option>']

and actually, I can force this in the call to dropDownList:

<div>
    <?php $form = ActiveForm::begin([
        'action' => '/some/path',
        'method' => 'GET'
     ]) ?>

    <?php $form->field($model, 'value')->textInput()->label(
        'text')->dropDownList([true => 'Yes', false => 'No'],
                ['prompt' => 'Please select',
                'options' => [
                        '1' => [
                        'value' => '1',
                        'selected' => $model->value
                        ],
                        '0' => [
                            'value' => '0',
                            'selected' => !$model->value
                        ]
                ]
]) ?>
</div>

But that is infeasible in practice, because there are many calls like this and adding this options array makes very verbose what I think should be the default behaviour. Furthermore, handling the case where $model->value is null manually would be even more complicated, as I mustn't pass 'selected' if $model->value isn't set.

(It is only checked whether the key is set:)

yii2/framework/helpers/BaseHtml.php

Line 1915 in 2874e07

if (!array_key_exists('selected', $attrs)) {

[WinterSilence]

@adnandautovic

(string)false is '' i.e. prompt value

I mean your boolean attribute converts to string value, (string)null is too ''

yii2/framework/helpers/BaseHtml.php

Line 1875 in 2874e07

$selection = array_map('strval', ArrayHelper::toArray($selection));

in this case ActiveField::checkbox() looks more appropriate

[adnandautovic]

I mean your boolean attribute converts to string value, (string)null is too ''

Actually, it does not:

yii2/framework/helpers/BaseHtml.php

Lines 1874 to 1876 in 2874e07

	if (ArrayHelper::isTraversable($selection)) {
	$selection = array_map('strval', ArrayHelper::toArray($selection));
	}

$selection is not traversable, since it is not an array, but a boolean.

in this case ActiveField::checkbox() looks more appropriate

Unfortunately, this does not work. The model represents a filter and there are three possible selections: None (display all), Yes (display only things where condition is true) and No (display only things where condition is false).

[WinterSilence]

@adnandautovic you right, but prompt is first option in list i.e. selected by default (when $selected is null).
your bool attribute in any case converts to string in !strcmp($key, $selection)

[adnandautovic]

@WinterSilence Yes, it gets converted to string in !strcmp($key, $selection) and that is precisely where the problem lies.
My suggested fix would be to add

if (is_bool($selection)) {
    $selection = (int) $selection;
}

just after

yii2/framework/helpers/BaseHtml.php

Line 1914 in 2874e07

$attrs['value'] = (string) $key;

because that is exactly what php does for an array with boolean keys.

[WinterSilence]

@adnandautovic users must normalize sended values ​​themselves, but I think possible fix:

 $attrs['selected'] = $selection !== null &&
      (!ArrayHelper::isTraversable($selection) && !($strict ? $selection == $key : strcmp($key, $selection))
      || ArrayHelper::isTraversable($selection) && ArrayHelper::isIn((string)$key, $selection, $strict));

[adnandautovic]

@WinterSilence The normalization takes place in the model's rules, where casting to bool takes place (since the underlying GET request does not pass a true boolean to php).

Anyway, the fix you suggested would also be quite fine. I just think the ! is misplaced and it should read as follows instead (strcmp returns 0 if it thinks the values are equal):

 $attrs['selected'] = $selection !== null &&
      (!ArrayHelper::isTraversable($selection) && ($strict ? $selection == $key : !strcmp($key, $selection))
      || ArrayHelper::isTraversable($selection) && ArrayHelper::isIn((string)$key, $selection, $strict));

[WinterSilence]

The normalization takes place in the model's rules, where casting to bool takes place (since the underlying GET request does not pass a true boolean to php).

value converts to bool when you load request data. also you can declare attribute setter/getter to convert value.

[adnandautovic]

I just realized I had given the issue a misleading name. The problem was not incorrect setting of the default value ('Please select'), but incorrect setting of the selected value ('Yes') or ('No').

[adnandautovic]

value converts to bool when you load request data.

I'm sorry, but I don't think this is correct in my case. At some point, I call $model->load(Yii::$app->request->get(), '') and the value I recieve is a string, which is either '', '0' or '1', depending on what was selected in the active form.

Now, during the call to $model->setAttributes inside of $model->load, these would get casted to booleans. In fact, I overwrite the setAttributes function to have more control over the type casting (my real model is more complicated and has more variables, I just want to give a rough idea of what is going on):

class MyModel extends Model
{
    public ?bool $isPropertySet = false;

    private const BOOLEAN_ATTRIBUTES = ['isPropertySet'];


    public function setAttributes($values, $safeOnly = true)
    {
        if (is_array($values)) {
            $values = $this->normalizeParams($values);
            parent::setAttributes($values);
        }
    }

    private function normalizeParams(array $params): array
    {
        foreach ($params as $name => $value) {
            if (in_array($name, self::INTEGER_ATTRIBUTES)) {
                $value = self::filterInteger($value);
            } elseif (in_array($name, self::BOOLEAN_ATTRIBUTES)) {
                $value = self::filterBoolean($value);
                // ...
            } elseif ($value === '') {
                $value = null;
            }
            $params[$name] = $value;
        }
        return $params;
    }

    public static function filterBoolean(string|bool $value): bool|null
    {
        if (is_string($value)) {
            $boolValues = ['0' => 0, '1' => 1, 'false' => 0, 'true' => 1];
            return $boolValues[strtolower($value)] ?? null;
        } 
        return $value;
    }
}

@adnandautovic users must normalize sended values ​​themselves,

Therefore, I already normalize the values I recieve and that is not the underlying issue here.

also you can declare attribute setter/getter to convert value.

I do not think that would help in this case, because I already convert the values to what I need them to be. Furthermore, defining setters and getters for the many attributes that are affected by this does not seem reasonable to me when the renderSelectOptions could just be fixed, instead.

[WinterSilence]

...or use checkbox input as all normal developers :)

[adnandautovic]

...or use checkbox input as all normal developers :)

A checkbox can represent two states, checked and unchecked.
A boolean attribute can represent three states: true, false and unselected.

As previously elaborated, I need to represent three states. If you have an idea on how to achieve that with a checkbox, feel free to let me know.

In any case, I think this is a place to discuss a bug in the yii framework and as far as I'm aware, there is a code of conduct in place.

[WinterSilence]

A boolean attribute can represent three states: true, false and unselected.

I got your idea, but it's not boolean, it's enum or something like this, use getter/setter:

<?=$form->field($model, 'value')->label('text')->dropDownList(['null' => 'Please select', 'true' => 'Yes', 'false' => 'No'])?>

protected ?bool $value = null;

public function getValue(): ?bool
{
    return $this->value;
}

public function setValue(bool|null|string $value)
{
    $this->value = is_string($value) ? json_decode($value) : $value;
}

[adnandautovic]

@WinterSilence Thank you for the suggestion, but, you see, these getter and setter functions do not solve the problem in the renderSelectOptions function.

The setting of the value works perfectly fine (although your code for doing so is shorter than what I wrote above when there is only one boolean variable in the model). It is when the value is read inside of renderSelectOptions and the boolean false is compared to the integer 0 inside of strcmp that the comparison fails. Have you looked at the function in a debugger?

And will my pull request #19331 be considered as a fix? Currently, it is not clear to me whether this issue has been recognized/accepted as a bug.

[WinterSilence]

@adnandautovic

And will my pull request #19331 be considered as a fix?

I do not know, but fact that strict mode is not used for single selection is error.

I'm think check must be like this (fixed error in my previous solution):

$attrs['selected'] = false;
if ($selection !== null) {
    if (ArrayHelper::isTraversable($selection)) {
        $attrs['selected'] = ArrayHelper::isIn($key, $selection, $strict);
    } else {
        $attrs['selected'] = $strict ? strcmp((string)$key, (string)$selection) == 0 : $key == $selection;
    };
}

[WinterSilence]

namespace yii\helpers;
class Html extends BaseHtml
{
    public static function renderSelectOptions($selection, $items, &$tagOptions = [])
    {
        if (ArrayHelper::isTraversable($selection)) {
            $selection = array_map('strval', ArrayHelper::toArray($selection));
        }

        $lines = [];
        $encodeSpaces = ArrayHelper::remove($tagOptions, 'encodeSpaces', false);
        $encode = ArrayHelper::remove($tagOptions, 'encode', true);
        $strict = ArrayHelper::remove($tagOptions, 'strict', false);
        if (isset($tagOptions['prompt'])) {
            $promptOptions = ['value' => ''];
            if (is_string($tagOptions['prompt'])) {
                $promptText = $tagOptions['prompt'];
            } else {
                $promptText = $tagOptions['prompt']['text'];
                $promptOptions = array_merge($promptOptions, $tagOptions['prompt']['options']);
            }
            $promptText = $encode ? static::encode($promptText) : $promptText;
            if ($encodeSpaces) {
                $promptText = str_replace(' ', ' ', $promptText);
            }
            $lines[] = static::tag('option', $promptText, $promptOptions);
        }
        unset($tagOptions['prompt');

        $options = ArrayHelper::remove($tagOptions, 'options', []);
        $groups = ArrayHelper::remove($tagOptions, 'groups', []);
        $options['encodeSpaces'] = ArrayHelper::getValue($options, 'encodeSpaces', $encodeSpaces);
        $options['encode'] = ArrayHelper::getValue($options, 'encode', $encode);

        foreach ($items as $key => $value) {
            if (is_array($value)) {
                $groupAttrs = isset($groups[$key]) ? $groups[$key] : [];
                if (!isset($groupAttrs['label'])) {
                    $groupAttrs['label'] = $key;
                }
                $attrs = compact('options', 'groups', 'encodeSpaces', 'encode', 'strict');
                $content = static::renderSelectOptions($selection, $value, $attrs);
                $lines[] = static::tag('optgroup', "\n" . $content . "\n", $groupAttrs);
            } else {
                $attrs = isset($options[$key]) ? $options[$key] : [];
                $attrs['value'] = (string) $key;
                if (!array_key_exists('selected', $attrs)) {
                    $attrs['selected'] = false;
                    if ($selection !== null) {
                        if (ArrayHelper::isTraversable($selection)) {
                            $attrs['selected'] = ArrayHelper::isIn($key, $selection, $strict);
                        } else {
                            $attrs['selected'] = $strict ? $key === $selection : $key == $selection;
                        };
                    }
                }
                $text = $encode ? static::encode($value) : $value;
                if ($encodeSpaces) {
                    $text = str_replace(' ', ' ', $text);
                }
                $lines[] = static::tag('option', $text, $attrs);
            }
        }

        return implode("\n", $lines);
    }
}

Demo:

class Demo extends \yii\base\Model
{
    public ?bool $boolValue = null;
}

$model = new Demo();
$form = \yii\widgets\ActiveForm::begin();
echo $form->field($model, 'boolValue')
     ->label('text')
     ->dropDownList(
        ['No', 'Yes'],
        ['prompt' => 'Please select']
    );

$model->boolValue = false;
echo $form->field($model, 'boolValue')
     ->label('text')
     ->dropDownList(
        ['No', 'Yes'],
        ['prompt' => 'Please select']
    );

[adnandautovic]

I do not know, but fact that strict mode is not used for single selection is error.

Ah, yes, if we compare to yii\helpers\BaseArrayHelper::isIn, there the non-strict check is done with == and the strict check is done with ===:

    public static function isIn($needle, $haystack, $strict = false)
    {
        if ($haystack instanceof Traversable) {
            foreach ($haystack as $value) {
                if ($needle == $value && (!$strict || $needle === $value)) {
                    return true;
                }
            }
        } elseif (is_array($haystack)) {
            return in_array($needle, $haystack, $strict);
        } else {
            throw new InvalidArgumentException('Argument $haystack must be an array or implement Traversable');
        }

        return false;
    }

and if it is not a Traversable, for expamle in_array(false, array('1.10', '', 1.13)) yields true.

$attrs['selected'] = false;
if ($selection !== null) {
    if (ArrayHelper::isTraversable($selection)) {
        $attrs['selected'] = ArrayHelper::isIn($key, $selection, $strict);
    } else {
        $attrs['selected'] = $strict ? strcmp((string)$key, (string)$selection) == 0 : $key == $selection;
    };
}

This is just a more verbose form of the previous version with additional type casting and inversion of the strict.
Is it better to be more verbose here?

And one more thing: If we compare to yii\helpers\BaseArrayHelper::isIn above, there is no usage of the strcmp function. Therefore, it would be more consistent to write

$attrs['selected'] = $strict ? $key === $selection : $key == $selection;

don't you think? Otherwise the function returns that integer 1 and string '1' are equal, even with strict=true.

[WinterSilence]

@adnandautovic

Is it better to be more verbose here?

what's you mean?

Therefore, it would be more consistent to write

yes, I was thinking same, but maybe we miss reason to using strcmp