Langauge configuration is one of the strangest beasts in Drupal. We tell everybody you should not use t() on dynamic data, but the language system freely uses t($language->name) as it wishes. Woah! To make that kind of work, it specifically asks for the English name of the language, to mimic how t()-ed strings work. (The basis for using t() is that *most* languages will use a predefined name so they in some way come from code after all even though they are user configurable). Also, even worse is that languages ask for a native name too, which is basically a "cache" of t($language->name, array(), array('langcode' => $language->language). So we ask for the language name in two specific languages on one form!

You see this pattern nowhere else in Drupal. If you go to edit a contact form or a view, the title for the contact form/view will be asked in *your* admin language (and then translated from there by i18n_string). There is absolutely no reason for the language list to be exceptional here. People are repeatedly puzzled about why we ask for the English name and a native name for a language. Many sites who use this functionality don't even have any English support on the site, but they need to specify the language in English anyway. Scandal!

To clean this up, we should stop pretending we have a configuration translation system. We don't have one. We should lead by example and stop t()-ing user editable data (language name), ask for the language name simply in the configuration language and stop asking the user about the native name, which is to be retrieved from the translation system itself anyway.

We have a couple of patches ongoing that I'd prefer to not break, so marking this postponed on them:

- #1260860: Rework language list admin user interface
- #1280530: Decouple domain/path negotiation setup from language configuration
- #1296566: Improve usability of add language screen

CommentFileSizeAuthor
#21 kill-fake-locale-config-translation-21.patch26.85 KBGábor Hojtsy
#18 kill-fake-locale-config-translation-18.patch26.93 KBGábor Hojtsy
#16 kill-fake-locale-config-translation-16.patch26.34 KBGábor Hojtsy
#13 kill-fake-locale-config-translation-13.patch26.12 KBGábor Hojtsy
#11 kill-fake-locale-config-translation-11.patch26.12 KBGábor Hojtsy
#8 kill-fake-locale-config-translation-8.patch25.68 KBGábor Hojtsy
#7 kill-fake-locale-config-translation-7.patch28.75 KBGábor Hojtsy
#6 kill-fake-locale-config-translation.patch29.71 KBGábor Hojtsy
#5 kill-fake-locale-config-translation.patch24.61 KBGábor Hojtsy
#1 kill-fake-locale-config-translation.patch23.72 KBGábor Hojtsy
#1 EditLanguage.png46.86 KBGábor Hojtsy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Active » Postponed
FileSize
EditLanguage.png46.86 KB
kill-fake-locale-config-translation.patch23.72 KB

Here is a quick patch to show how this "feels". No update function but one will need to be provided. Once this and #1280530: Decouple domain/path negotiation setup from language configuration lands, the language edit/add form is going to be *very* simple, like this:

EditLanguage.png

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Postponed » Needs review

Interested in test results.

Status: Needs review » Needs work

The last submitted patch, kill-fake-locale-config-translation.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review
FileSize
kill-fake-locale-config-translation.patch24.61 KB

Should fix the tests.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
FileSize
kill-fake-locale-config-translation.patch29.71 KB

Even more t()-ed language name instances found. I believe this is (almost) complete. Did a pretty extensive search/hunt. :)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: +Usability
FileSize
kill-fake-locale-config-translation-7.patch28.75 KB

#1280530: Decouple domain/path negotiation setup from language configuration did land and made most of this fail when applied. Here is a reroll.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
FileSize
kill-fake-locale-config-translation-8.patch25.68 KB

Rerolled for latest core changes.

tobiasb’s picture

tobiasb
Primary language German
Location Berlin
CreditAttribution: tobiasb commented

You mean that Deutsch is Deutsch not German in e.g. the english interface?

Gábor Hojtsy’s picture

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

@tobiasb: this patch attempts to solve two configuration translation related problems:

A: $language->name which is user input is used in t() to be translated. This is a bad practice we fight against extensively in contrib and elsewhere in core. User input should never be t()-ed. The argument for that was that language names are usually from a preset list. Well, that is not a good argument for this. We should not t() user editable data period :)

B: $language->native is just a "cached" value of $language->name translated to the language in question, and we do get that data via the translation system itself, so there is no reason to bother the user with entering it, so we both simplify the interface and the data structure. Whatever we can do on behalf of humans in our code, we should do instead of relying on humans do it :)

Did this help provide background?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
FileSize
kill-fake-locale-config-translation-11.patch26.12 KB

Rerolled for recent changes, no changes. Please, please review!

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs review » Needs work

The patch looks good and seems to work well from a cursory test. There are still a couple of things to be fixed:

+++ b/modules/locale/locale.install
@@ -255,3 +247,10 @@ function locale_schema() {
+function locale_update_7000() {

I guess this should be locale_update_8000().

Probabiy we need to remove native language names from the predefined list and fix _locale_prepare_predefined_list (see line 815).

-20 days to next Drupal core point release.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review
FileSize
kill-fake-locale-config-translation-13.patch26.12 KB

Fixed patch with 8000 number for the update function, you are right. On the native name removal from the built-in language codes, we are in fact doing the contrary. See #1295696: Sync predefined list of languages with localize.drupal.org and extend native language information, where I extend the list to have native names for all. Basically, we need to display native names for languages from where we don't yet have translations loaded (unlike the case for names here, where if we do need the translation, we should have it loaded, since we specifically have it configured). Any other concerns?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented 
locale_update_8000()

Looks like this needs an @ingroup. edit, or @addtogroup, I can never get these right.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Ok for the predefined list but:

<?php
function _locale_prepare_predefined_list() {
  include_once DRUPAL_ROOT . '/includes/standard.inc';
  $languages = language_list();
  $predefined = standard_language_list();
  foreach ($predefined as $key => $value) {
    if (isset($languages[$key])) {
      unset($predefined[$key]);
      continue;
    }
    $predefined[$key] = t($value[0]); // Shouldn't this be fixed somehow?
  }
  asort($predefined);
  return $predefined;
}
?>
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
FileSize
kill-fake-locale-config-translation-16.patch26.34 KB

@plach: the list of predefined languages comes from the source code and the source code parsed in potx() extracts those strings. Thus they should be available for translation like any other t()-ed string. Its not direct, its indirect like hook_menu() strings, but its there.

@catch: its defgroup is both block.install and system.install in D8, so I've added that here too.

Any other issues?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs review » Reviewed & tested by the community

Looks good!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Needs review
FileSize
kill-fake-locale-config-translation-18.patch26.93 KB

All right, @catch unearthed our phpdoc grouping doc at http://drupal.org/node/1354#groups. It says @defgroup updates-7.x-to-8.x should only be used once in the whole codebase in the most important place. I've verifier this with 7.x update files. So we should use addtogroup elsewhere. The attached patch uses that for locale.install (and fixes the only one other place, in block.install too). I expect this to land soon, so including that slightly related piece seemed like a good idea.

No other changes, so if passes tests (it should :), should be back on RTBC.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs work

Sorry to nitpick but the addtogroup isn't quite right, according to block.install D7, we only need:

/**
 * @addtogroup updates-7.x-to-8.x
 * @{
 */

since the description is already in the original defgroup.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Reviewed & tested by the community
FileSize
kill-fake-locale-config-translation-21.patch26.85 KB

There you go. (No other change).

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x, thanks!

Also
15 files changed, 50 insertions(+), 94 deletions(-)
mmm diffstat.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
CreditAttribution: podarok commented

nice.
tnx!

Gábor Hojtsy’s picture

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

Added change entry http://drupal.org/node/1323146 for this. Thanks!

Status: Fixed » Closed (fixed)

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

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue tags: +API clean-up

Cleaning up bogus/duplicate "API cleanup" term.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: +language-base

Tagging for base language system.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: +language-config

Tag for config translation too).