Boolean field "On label" and "Off label" are not translatable

Marking as needs work as it needs tests.
I couldn't find a test for the current behavior for "Configuration Translation wants to display the title of each display instead of just printing Views display for each one".
We should probably add one for that so we are sure we don't break that feature.

+1 nice work. thanks @tstoeckler

1. +++ b/core/modules/config_translation/src/FormElement/ListElement.php
   @@ -115,22 +120,23 @@ public function setConfig(Config $base_config, LanguageConfigOverride $config_tr
   +        $prefix = strip_tags($group_build[$key]['source']['#markup']);
   

   Are we sure to strip tags here?

2. +++ b/core/modules/views/config/schema/views.schema.yml
   @@ -99,6 +99,7 @@ views.view.*:
   +        label_key: 'display_title'
   

   Do we need this here? Looks like not in scope of this issue.

@vijaycs85: re 2, looks like we need to do that change so we can test it (or at least we need to add it to a test schema to test it there, but then we need to fix it in views too as per above).

@tstoeckler: I think the API addition implemented here is entirely backwards compatible, so it should not be considered an API change. API additions should be possible in 8.1, not earlier though AFAIK. We may want to get the schema fix for string => label in ASAP, even if the UI is a bit confusing, at least the translatability would be there and the API addition for 8.1 AFAIS.

I just stumbled across this issue in a customer site - this time when adding a boolean to a contact form (using Core contact module). After applying the patch I was able to translate the boolean value labels, export the configuration, move it to a dev server with the patch - and the translation came along. Happy times!

Thanks for making this patch - now I'm hoping it can enter in a future 8.0.x so that also people who don't go hunting in the issue queue for patches can translate their labels..

A colleague tipped me about this issue:
https://www.drupal.org/node/1061438

Are these duplicate of one another?

Despite predating this issue by four and a half years, I'm closing #1061438: "On" and "Off" values for boolean fields are not translatable as a dup of this because the patch here has tests and the patch there wasn't approaching the fix correctly.

@mikeker: well actual tests for the actual fix are still missing. Can you help provide them? :) I think that is mostly what separates us from this being comittable.

Thanks for the patch. I noticed one typo:

+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -580,6 +580,13 @@ public function testViewsTranslationUI() {
+    // Make sure that the details element for each display has a proper (unqiue)

Nitpick (typo): uniqiue => unique.

Thanks! I looked at the new patch and have one question:

+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -734,6 +735,42 @@ public function testFieldConfigTranslation() {
+    $this->assertText(Html::escape($on_label) . ' Boolean settings');
+    $this->assertEscaped($on_label);
+    $this->assertEscaped($off_label);

Is there supposed to be an assertText for $off_label too?

Oh... I see, it's the fieldset so I guess not. That is a bit confusing.

+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -734,6 +735,42 @@ public function testFieldConfigTranslation() {
+    $this->assertText(Html::escape($on_label) . ' Boolean settings');
+    $this->assertEscaped($on_label);
+    $this->assertEscaped($off_label);

I think it would be useful to add some comments to explain what is being checked, particularly for the assertText part. Thanks.

I have tested the patch and it is working as expected.

For better UX though, I'm wondering if it would be make sense to get rid of the fieldset altogether?

@Kristen: for config translation, the fieldset is merely there because the two labels are contained within a parent element. This is automated and cannot be overridden in core at least. I think this is already a big enough improvement either way?

Looking at the summary, it seems to be updated according to current patch.

@Gábor Hojtsy - Ok, no worries. Yes, this is a great improvement! :) RTBC++

Committed 5830c2f and pushed to 8.0.x, 8.1.x and 8.2.x. Thanks!

This patch caused a random fail... https://www.drupal.org/pift-ci-job/214169

I don't think there is any reason to use ->randomString() here... how about just hardcoding values?

And here... https://www.drupal.org/pift-ci-job/214193

Here's 20 runs of the test... the first string is the on label and the second is the off label.

string(8) "3&):>&sX"
string(8) "*mkY>&W\"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "*y]P>&@B"
string(8) "|0.y>& _"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "R0]A>&pR"
string(8) ">3==>&&="
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "^3,$>&x_"
string(8) "T5Gb>&._"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "BL[U>&z8"
string(8) "HS_V>&NU"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "_RpN>&<F"
string(8) "BSX >&Rg"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  19 passes   1 fails
- Found database prefix 'simpletest745070' for test ID 19.
string(8) "`T(N>&]~"
string(8) ">z4W>&`p"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "T&og>&:w"
string(8) "Pk%U>&F&"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "z;xd>&]?"
string(8) "0},X>&+H"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) ")w]^>&\`"
string(8) "7X;A>&:a"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "Dg"l>&r\"
string(8) "Sky/>&o@"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "~mDR>&|2"
string(8) "XLhv>&5>"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "0PY\>&:\"
string(8) "q#>L>&cN"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "|HcM>&K8"
string(8) "D:$O>&N)"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "7-j`>&>J"
string(8) "0yR_>&Gx"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "i?{,>&}3"
string(8) "H$iX>&)Y"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "MwXX>&Tb"
string(8) "^=it>&Sd"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "Gmv%>&!7"
string(8) "DbF+>&8/"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) "pG_L>&$8"
string(8) "M35)>&>U"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes
string(8) ".PL6>&co"
string(8) "JNoq>&s@"
Drupal\config_translation\Tests\ConfigTranslationUiTest::tes  20 passes

So the problem seems to be related to spaces...

Changed the on/off labels to be fixed strings instead of randomString(). I still can't figure out what about those strings was causing the failure, though. Also added two comments to better describe what's being tested, as per #22.

Thanks for the update. I think it's useful that you added more comments.

+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -734,6 +735,45 @@ public function testFieldConfigTranslation() {
+    // Checks the details label.

Nitpick: For this one, maybe something like the following would be more clear?

// Checks the fieldset text.

The other ones look good to me.

@tstoeckler: Sure, that's a good idea. I've added some basic HTML and an ampersand to the labels. Doing that does raise an interesting point: in \Drupal\config_translation\FormElement\ListElement::getGroupTitle we strip_tags() the title, which is probably a bit heavy handed. (Summary elements can contain a fair bit of HTML.) Thus, we strip_tags() the expected value in the test.

But that's a different issue for a different day...

From #42:

// Checks the fieldset text.

Except that these are no longer fieldset but details elements. Regardless, I've updated the wording -- let me know if that clarifies things.

Thanks!

Committed 891e57e and pushed to 8.0.x, 8.1.x and 8.2.x. Thanks!

Superb, thanks!

Would be a string change in 8.0.6 then I guess.