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

Files: 
CommentFileSizeAuthor
#21 kill-fake-locale-config-translation-21.patch26.85 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,639 pass(es). View
#18 kill-fake-locale-config-translation-18.patch26.93 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,634 pass(es). View
#16 kill-fake-locale-config-translation-16.patch26.34 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,632 pass(es). View
#13 kill-fake-locale-config-translation-13.patch26.12 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,648 pass(es). View
#11 kill-fake-locale-config-translation-11.patch26.12 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,619 pass(es). View
#8 kill-fake-locale-config-translation-8.patch25.68 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,330 pass(es). View
#7 kill-fake-locale-config-translation-7.patch28.75 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,300 pass(es). View
#6 kill-fake-locale-config-translation.patch29.71 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,290 pass(es). View
#5 kill-fake-locale-config-translation.patch24.61 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [MySQL] 33,290 pass(es). View
#1 kill-fake-locale-config-translation.patch23.72 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [MySQL] 33,301 pass(es), 10 fail(s), and 0 exception(es). View
#1 EditLanguage.png46.86 KBGábor Hojtsy

Comments

Gábor Hojtsy’s picture

Status: Active » Postponed
FileSize
46.86 KB
23.72 KB
FAILED: [[SimpleTest]]: [MySQL] 33,301 pass(es), 10 fail(s), and 0 exception(es). View

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

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

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
24.61 KB
PASSED: [[SimpleTest]]: [MySQL] 33,290 pass(es). View

Should fix the tests.

Gábor Hojtsy’s picture

FileSize
29.71 KB
PASSED: [[SimpleTest]]: [MySQL] 33,290 pass(es). View

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

Issue tags: +Usability
FileSize
28.75 KB
PASSED: [[SimpleTest]]: [MySQL] 33,300 pass(es). View

#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

FileSize
25.68 KB
PASSED: [[SimpleTest]]: [MySQL] 33,330 pass(es). View

Rerolled for latest core changes.

tobiasb’s picture

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

Gábor Hojtsy’s picture

@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

FileSize
26.12 KB
PASSED: [[SimpleTest]]: [MySQL] 33,619 pass(es). View

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

plach’s picture

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

Status: Needs work » Needs review
FileSize
26.12 KB
PASSED: [[SimpleTest]]: [MySQL] 33,648 pass(es). View

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

locale_update_8000()

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

plach’s picture

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

FileSize
26.34 KB
PASSED: [[SimpleTest]]: [MySQL] 33,632 pass(es). View

@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

Status: Needs review » Reviewed & tested by the community

Looks good!

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
26.93 KB
PASSED: [[SimpleTest]]: [MySQL] 33,634 pass(es). View

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

Status: Needs review » Reviewed & tested by the community
catch’s picture

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

Status: Needs work » Reviewed & tested by the community
FileSize
26.85 KB
PASSED: [[SimpleTest]]: [MySQL] 33,639 pass(es). View

There you go. (No other change).

catch’s picture

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

nice.
tnx!

Gábor Hojtsy’s picture

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

Issue tags: +API clean-up

Cleaning up bogus/duplicate "API cleanup" term.

Gábor Hojtsy’s picture

Issue tags: +language-base

Tagging for base language system.

Gábor Hojtsy’s picture

Issue tags: +language-config

Tag for config translation too).