I think this should be merged into 3.4 as a bugfix.

Can we add a test to prevent future regressions?

I think this should be merged into 3.4 as a bugfix.

I started my branch from the 3.4. I just took the wrong one for the PR. It's fixed :)

Can we add a test to prevent future regressions?

Done @xabbuh

Thank you @VincentLanglet.

@nicolas-grekas @VincentLanglet @xabbuh

This change caused a regression in https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig#L304

Our radio button labels are not translated anymore unless I add 'choice_translation_domain' => 'messages' to the form type.

Symfony 4.4.10

Symfony 4.4.11

Prior to this change, we did not had to configure the translation domain because "messages" was the default and labels were translated automatically.

@XWB Thanks for the report. I am going to look into it.

It seems to be expected.

In the DoctrineType, there is the following config:

choice_translation_domain => false

Prior to this fix, the choice_translation_domain was certainly overridden because the label_translation_domain was true.

Now the FormType correctly use the choice_translation_domain option. So

choice_translation_domain => true

is needed if you save translation key in your database.

@VincentLanglet I understand, but it was the default behavior for years, so now all our radio buttons are broken. In my opinion that's a BC break.

In my opinion that's a BC break.

I understand that the situation is annoying, but it's not because a bug is old that it's a BC break.
A bugfix should always be called a bugfix.

People shouldn't expect the choice to be translated when they use choice_translation_domain => false.
When using

->add('foo', ChoiceType::class, [
    'label'       => 'foo.label',
    'choices'  => [
        'no'  => false,
        'yes' => true,
    ],
    'choice_translation_domain' => false,
]);

The choice was translated. How can you not call this a bug ? That's not what the documentation explains.

And when using

->add('foo', ChoiceType::class, [
    'label'       => 'label',
    'translation_domain' => false,
    'choices'  => [
        'choice.no'  => false,
        'choice.yes' => true,
    ],
    'choice_translation_domain' => 'messages',
]);

The choices wasn't translated, this is also an issue.

I completely understand that this bug + the fact that the EntityType had the option to false by default should have mistaken a lot of user. I don't know if a message can/should be added to the Changelog to alert users about this. WDYT ?

@VincentLanglet

People shouldn't expect the choice to be translated when they use choice_translation_domain => false.

I understand, though that was not entirely my use case. Unlike your example, I never specified choice_translation_domain, and the form labels of EntityType have been translated by default (just like all other form types) as long as I can remember.

I completely understand that this bug + the fact that the EntityType had the option to false by default should have mistaken a lot of user. I don't know if a message can/should be added to the Changelog to alert users about this. WDYT ?

An entry at https://symfony.com/blog/ would have been nice. It took me almost 2 hours to figure out what was going on between Symfony 4.4.10 and 4.4.11.

I understand, though that was not entirely my use case. Unlike your example, I never specified choice_translation_domain, and the form labels of EntityType have been translated by default (just like all other form types) as long as I can remember.

Yes but the EntityType was using choice_translation_domain => false since the beginning.
The tricky part was that this was not working since the beginning.

The changelog should have been

[Form] Fix ChoiceType translation domain
[Form] EntityType does not translate choices, as configured. Please set `choice_translation_domain => true` to keep the previous behavior.

Or a PR could be make to set choice_translation_domain => true to the EntityType.

Yes but the EntityType was using choice_translation_domain => false since the beginning.

Unfortunately that was never really clear from the documentation, as no default value is specified in the docs: https://symfony.com/doc/current/reference/forms/types/entity.html#choice-translation-domain

The changelog should have been

I agree.

Yes but the EntityType was using choice_translation_domain => false since the beginning.

Unfortunately that was never really clear from the documentation, as no default value is specified in the docs: https://symfony.com/doc/current/reference/forms/types/entity.html#choice-translation-domain

This could worth a PR ! Do you want to make it @XWB ?

@VincentLanglet

It looks like a default template is included, any idea how I could address this change without altering the other form types?

I don't know :(

see symfony/symfony-docs#14220 for the documentation update

Thank you @xabbuh