Problem/Motivation

Config translation form source labels are in English even if the site is installed in French and translating to Spanish. The target label should also be translated - I'm not sure whether this should be to French or Spanish.

For example here is translating the System information config:

Proposed resolution

tbd

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#17 interdiff.txt1.74 KBGábor Hojtsy
#17 2697291-17.patch5.2 KBGábor Hojtsy
#15 interdiff-13-15.txt801 bytesmaxocub
#15 2697291-15.patch5.2 KBmaxocub
#13 interdiff-8-13.txt2.04 KBmaxocub
#13 2697291-13.patch4.97 KBmaxocub
#8 2697291-8.patch4.43 KBmaxocub
#8 2697291-8-test-only.patch1.75 KBmaxocub
#5 2697291.patch2.68 KBGábor Hojtsy
Screen Shot 2016-03-31 at 00.33.09.png32.28 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler as a volunteer commented

I'm not sure whether this should be to French or Spanish.

Heh, that's a good question. Some thinking out loud:

  • I am anything but a usability expert but I think to avoid confusion we should translate both labels the same. Note that on narrow screens the source and target are actually displayed below each other and the label is only shown once, so having different translations of the label would provide a different experience on mobile vs. desktop which I also don't think is a good idea.
  • For Con*tent* Translation - where we only show a single form, not the source and translation next to each other - for a very long time you could only translate in the interface language of the target language. That is still the default (although it is now configurable). Following that pattern one might come to the conclusion that we should switch the interface language to the target language. Note that I have personally always found that behavior of Content Translation particularly unintuitive and would personally be against it, but it would be following an existing pattern. (To make this clear: What Content Translation is doing is switching the *entire* interface language which IMO is not something people expect to happen without their actively using a language switcher or the like.)
  • Further going along the example of Content Translation we now actually have a separate "Content translation language" language type, so one could conclude that we should also add that for config and make it configurable in the language negotiation. However, for the config itself we don't actually have a separate language type in the first place (in contrast to the separate "Content language" language type we have for content) - we just always use the interface language for everything in config land. So I think that would be confusing.

So having written that my personal stance would be that both labels should be in the current interface language (i.e. French in your example). I'm guessing that was probably the intended behavior - maybe even the previous behavior that was never tested and broken at some point due to refactoring of the language override functionality (that's just a guess though, not actually blaming anyone (but for myself for not writing tests for this ;-))). The only other possible solution I personally see is to also display both labels in the current interface language but link to the translation form in the interface language of the target language (i.e. Spanish in your example; concretely: make the link point to /es/admin/.../translations/add/es instead of /admin/.../translations/add/es) to be consistent with Content Translation, but - again - I personally don't think that's a pattern we should be spreading.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

I think all labels on this form (both "columns") should be in the interface language for the page. If they are not currently, that is hopefully due to them not being translated yet on your site? The labels from config schema are certainly exported in potx for translation to my best knowledge.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

The labels are translated :( so we have a bug here. I think the roots of which are in how the config translation form elements are built.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Active » Needs review
Issue tags: +D8MI, +language-config, +language-ui, +sprint, +Needs tests
FileSize
2697291.patch2.68 KB

So the labels are only translated for plural variant elements apparently. Don't know how this happened :D

Untested patch attached, but based on the PluralVariant.php code, it should work. Definitely needs tests. Can someone help with that? :)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Title: Config translation source labels are in English not the language of the site » Config translation form labels are not translated
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

At leas that did not fail anything. Anybody can help with testing?

maxocub’s picture

maxocub CreditAttribution: maxocub commented
Issue tags: -Needs tests
FileSize
2697291-8-test-only.patch1.75 KB
2697291-8.patch4.43 KB

Adds a test to check that the labels are translated.

The last submitted patch, 8: 2697291-8-test-only.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs review » Needs work 
+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -189,6 +190,26 @@ public function testSiteInformationTranslationUi() {
+    $this->assertText($fr_site_name_label, "The label '$fr_site_name_label' was not found");

The assertion message here is wrong - you;re checking that the test is found - plus it is unnecessary the default message is better.
I also think we should assert that the english label is not present.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

I also think we need to test a case were the interface and target languages are different.

maxocub’s picture

maxocub CreditAttribution: maxocub commented

@alexpott: Thanks for the review, I'll make the corrections.

maxocub’s picture

maxocub CreditAttribution: maxocub commented
Status: Needs work » Needs review
FileSize
2697291-13.patch4.97 KB
interdiff-8-13.txt2.04 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Needs review » Needs work 
+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -206,10 +207,16 @@ public function testSiteInformationTranslationUi() {
+    // Ensure that the label is in French (and not in English)
     $this->drupalGet("fr/$translation_base_url/fr/edit");
-    $this->assertText($fr_site_name_label, "The label '$fr_site_name_label' was not found");
+    $this->assertText($fr_site_name_label);
+    $this->assertNoText($site_name_label);
+
+    // Ensure that the label is also in French (and not in English)
+    // when editing another language with the interface in French
+    $this->drupalGet("fr/$translation_base_url/ta/edit");
+    $this->assertText($fr_site_name_label);
+    $this->assertNoText($site_name_label);

We need one more test like this...

$this->drupalGet("$translation_base_url/fr/edit");
$this->assertNoText($fr_site_name_label);
$this->assertText($site_name_label);

Just to check the form in the default language. Unless this check is done elsewhere in the test.

maxocub’s picture

maxocub CreditAttribution: maxocub commented
Status: Needs work » Needs review
FileSize
2697291-15.patch5.2 KB
interdiff-13-15.txt801 bytes

@alexpott: No, it wasn't done anywhere else in the test, so here it is.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs review » Needs work 
+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -189,6 +191,37 @@ public function testSiteInformationTranslationUi() {
+    // Translate 'Site name' label in French
...
+    // Ensure that the label is in French (and not in English)
...
+    // Ensure that the label is also in French (and not in English)
+    // when editing another language with the interface in French
...
+    // Ensure that the label is not translated when the interface is in English

Amazing, looks great except all the sentences in the tests missing a period at their end. :)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs work » Reviewed & tested by the community
FileSize
2697291-17.patch5.2 KB
interdiff.txt1.74 KB

Here it is, so I don't just complain :D RTBC since adding a few periods is very minor.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 3c88f61 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed d466fb2 on 8.2.x 
    Issue #2697291 by maxocub, Gábor Hojtsy, alexpott: Config translation...

  • alexpott committed 3c88f61 on 8.1.x 
    Issue #2697291 by maxocub, Gábor Hojtsy, alexpott: Config translation...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Issue tags: -sprint

Yay, thanks!

maxocub’s picture

maxocub CreditAttribution: maxocub as a volunteer and commented