Synopsis:
Drupal currently has a serious logic flaw in the way administrative textual input is handled in several of its modules.

Description:
The incorrect use of the t() function causes problems when the following preconditions are met:

  • using locales.module
  • with (at least) english and another language installed
  • the latter being selected for the admin account.

The problem lies in Form API's #default values incorrectly being passed through the t() function, causing already translated text to be stored in the database as locales_source, without the possibility to translate it back into English.

Places identified so far are:

  • code matching regex: variable_get\('\w+', ?t\(
    ie. common.inc, aggregator.module, contact.module, system.module plus another two third party modules (buddylist, volounteer)
  • _user_mail_text() in users.module

Steps to reproduce:

  1. Start with a fresh Drupal installation (english language installed, no variables stored): open Administer > Settings > Site Maintenance. The off-line message is displayed in english. Do NOT hit Save for now, so that no variables are created.
  2. Enable the locale module and import a language file. Leave english enabled as secondary language.
  3. Switch to My Account and set the newly installed language as your preferred language.
  4. Open Site Maintenance again, the following has changed now: the message text didn't exist as variable, but the default text is translated on-the-fly, as one can see from the sources:
    variable_get('site_offline_message', t('%site is currently under maintenance. We should be back shortly. Thank you for your patience.', array('%site' => ...))
  5. Hit Save. What happens? The localized text is now stored in the corresponding variable. This faces us with a problem, as we now have no (regular) possibility to translate the localized text back to English.

As a rule of thumb, no Form API's #default value should be passed through the t() function. Let t() handle localization when finally outputting it instead.
The admin manual should be updated to reflect these changes, ie. translation has to happen exclusively in the locale.module, not within the admin area.

CommentFileSizeAuthor
#11 localization.patch12.75 KBsmk-ka
#8 localization_patch.zip3.59 KBsmk-ka
#7 true_localization_patch.zip0 bytessmk-ka

Comments

killes@www.drop.org’s picture

killes@www.drop.org commented
Version: 4.7.0-rc3 » x.y.z

I can reproduce the issue.

killes@www.drop.org’s picture

killes@www.drop.org commented

I think the real problem is that our approach to letting users change strings is inconsistent.

We provide localisation which facilitates changing of strings like "my account" and we also supply a way to change some messages (like the site_offline message or the user mail messages) without using localisation at all.

While this is annoying I don't consider this a showstopper for 4.7. I suppose modules dealing with multiple languages on one site should simply replace the extra forms and only use the localisation interface for such messages.

killes@www.drop.org’s picture

killes@www.drop.org commented

Hmm, I've now done the following:

1) switched German off again. Went to admin /settings. The offline message was displayed in English. This was because the message wasn't saved to the database.

2) saved admin/settings. The message gets saved in the database.

3) switched on German again.

4) went to admin/settings: The message is still in English.

This behaviour is maybe a bit confusing, but IMHO not a bug, but rather a sign for the inconsistency mentioned above.

smk-ka’s picture

smk-ka commented

we also supply a way to change some messages (like the site_offline message or the user mail messages) without using localisation at all.

I'm not sure if I got you right, but I think these messages are localized as well on output, the real problem is that they are translated twice: once on the admin settings pages and another time when outputting them.

4) went to admin/settings: The message is still in English.

Yes, but that's the only case when everything is allright: as long as the textfields on the admin pages are in English, they can (and they will be) translated when outputting them. You'll only get in trouble if you'd save the german translation, because then, there is no way back to English (ie. all users will see the german message).

Anyway, I've just seen that there is less code to patch than written before, it affects only contact.module (first match), system.module and users.module.

I could put together a patch with the corrected usage of t() (as I see it) if it's worth the time, should I?

smk-ka’s picture

smk-ka commented

Aw, I see, I was wrong about your last point: variables are in fact not translated before they're output. So the problem lies a bit deeper and the patch grows bigger :/

chx’s picture

chx commented
Status: Active » Postponed

let's get back to this when 4.7 is out. the behaviour is the same as it was in 4.6.

smk-ka’s picture

smk-ka commented
StatusFileSize
new true_localization_patch.zip0 bytes

m'kay... anyway, just for the reference: this patch should fix it. the problem about the variables wasn't really one: we just need to move the t()ranslation from the admin pages to the output of the variables. since this only affects a specific number of text inputs, the patch was smaller than first thought...

cheers

smk-ka’s picture

smk-ka commented
StatusFileSize
new localization_patch.zip3.59 KB

amen

smk-ka’s picture

smk-ka commented
Status: Postponed » Needs review

May I bring this issue back into view, since 4.7 is finally out, and the patch is already developed?

Remeber that it currently can't be recommended to use Drupal for a multilanguage site, as all site users will receive German (or whatever language is the default one) account confirmation emails (for example). This might be of no interest for an average non-profit site, but could easily result in loss of customers in a more commercial environment.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

Please don't post patches in archives. cvs diff can take multiple aruments and make a single patchfile for multiple changed files.

smk-ka’s picture

smk-ka commented
Status: Needs work » Needs review
StatusFileSize
new localization.patch12.75 KB

Agreed.

dries’s picture

dries commented

Not sure I like this solution. Some of the proposed change seem to have nothing to do with the forms API?

killes@www.drop.org’s picture

killes@www.drop.org commented

What his patch does is to remove t() calls from default values for variable_get calls.

This has rather marginal advantages for people who use multiple langauges on one site _and_ don't bother to save and modify the variable. Since there is however only one value of a variable for all languages, they need to modify the text anyway to include all languages they support and shoult not run into the problem in the first place.

It is an major inconvenience for people who only use one language (!= English) as they need to translated the whole text and cannot use a default provided by the translator.

That being said: -1

smk-ka’s picture

smk-ka commented
Status: Needs review » Needs work

they need to translated the whole text and cannot use a default provided by the translator

True, and not desirable.

Well, then, the hard way will be making the Forms API and the variables system truly l10n-aware.

  1. Forms API: Add a special property #localize for textfield and textarea input types. The renderer would then replicate the input field for every active language (turning the input field into an array).
  2. Variables system: Enhance the data table variable by one field locale. variable_get/set() is modified to take the locale into account, falling back on en.
  3. hook_settings() will be enhanced to support both the new property, and store the variables accordingly.

That would allow us to transparently keep the current behavior, while fully supporting l10n. It would further solve the problem of the hard-coded date formats and move them into the language files (can't find the issue I have in mind right now - it was the one where killes [I think] came up with a date.module that hacks the settings page).

Comments? Concerns?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Localization issue » Variables are not localized properly
Status: Needs work » Active

The issue of having language dependant variables is a real one, and is dealt with in the contrib i18n module. Jose started a push to get better i18n functionality in core with this DEP There is already a working solution in the i18n module core in contrib.

Either mark this duplicated if the i18n functonality is fine with you, or recategorize this for the i18n module, if you think this needs fixing there. Marking active, setting up a better title.

smk-ka’s picture

smk-ka commented
Status: Active » Closed (duplicate)