Skip to content

[Form] fixed false value being converted to an empty string in ChoiceArrayList #17372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

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