Skip to content

Conversation

@ivoaz
Copy link

@ivoaz ivoaz commented Jan 14, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #17292
License MIT
Doc PR

When false value is used as a choice value, it is converted to an empty string,
which conflicts with the empty value, if the choice is optional. If the choice
is not optional, then there may be problems with html validation, since required
choice does not allow empty value to be submitted.

This patch disables value casting to string, if the choices contain false value.

…ArrayList

When `false` value is used as a choice value, it is converted to an empty string,
which conflicts with the empty value, if the choice is optional. If the choice
is not optional, then there may be problems with html validation, since required
choice does not allow empty value to be submitted.

This patch disables value casting to string, when the choice contains values,
that would be converted to an empty string.
@xabbuh
Copy link
Member

xabbuh commented Jan 14, 2016

I am not sure if this is the right fix but we would need a test in any case that ensures we do not break the behaviour again in the future.

@xabbuh xabbuh added the Form label Jan 14, 2016
@ivoazm
Copy link

ivoazm commented Jan 15, 2016

Actually, when using false as a key of an array, it is converted to 0, not empty string, like casting to string does.

Better solution would be then to convert false value to 0, instead of casting it to string. This would be consistent with how it worked before with choices_as_values => false.
What do you think?

@xabbuh
Copy link
Member

xabbuh commented Feb 11, 2016

sorry for replying earlier @ivoaz

However, I am closing here in favour of #17760 which imo looks more like we should fix the issue and contains tests.

@xabbuh xabbuh closed this Feb 11, 2016
@ivoaz
Copy link
Author

ivoaz commented Feb 11, 2016

No problem, I agree with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants