Addressing misuse of t()

We encourage users to post events happening in the community to the community events group on https://www.drupal.org.
nedjo's picture

An issue in Drupal 6+ contrib needing some thought is the current misuse of t().

Prior to D6 there was no clear way to provide translation for strings other than ones built into code. So many module authors started resorting to passing variables through t(), e.g.,

<?php
$output
.= t($variable['key']);
?>

where $variable['key'] might be some string entered by a site admin. A quick fix, but one with significant problems. Yes, site users now could enter translation through the locale UI, but it was essentially a misuse of the locale system. The data coming into t() have no context. So, e.g., when the data are updated, there's no way to invalidate or replace the previous version. Etc.

With D6 we have a flexible locale system that can accept multiple types (called 'groups') of data (as defined hook_locale()). The strings module included in i18n (i18nstrings) provides methods to facilitate working with custom groups. i18nstrings allows modules to uniquely identify strings that are being sent for translation and handles updating of existing strings. (Strings are identified by a colon-separated series of identifiers, in which the first item is the locale group and subsequent ones provide whatever level of specificity is needed. At the simplest, an identifier might look like: mygroup:mysetting).

But few if any contrib modules have introduced an i18nstrings dependency. And introducing a new locale group every time we have one or two new strings to translate would quickly clutter the locale interface.

Perhaps we need a new general purpose locale 'group' in i18nstrings, e.g, 'configuration'? Module authors could identify their strings as e.g.

configuration:mymodule:mykey

and, if they want to avoid a requirement for i18n, use conditional calls like:

<?php
if (module_exists('i18nstrings')) {
 
$variable = tt('configuration:mymodule:mykey', $variable);
}
?>

Or maybe we need a simpler version of i18nstrings, or one not part of i18n to avoid this large dependency?

Comments

Developers aren't aware

stella's picture

I think the problem is that there are a lot of module developers who are just not aware that they shouldn't be passing variables through t(). I can't find any documentation in the module developers guide or in the translation section of the handbook that says not to do this. Perhaps there is such a doc, but it's not easy to find. I think the first step to addressing the misuse of t() is education and to do this we need guidelines for developers.

As a developer, the only way I know of finding out if I've used t() correctly is to extract strings using the Potx module, or run the Potx module Coder review. It returns errors like:

+451: Invalid marker content watchdog('wordfilter','Updated filter for:'.$form_state['values']['words'])

While these are useful for identifying the problematic lines, it doesn't advise the developer on how to fix their code. More documentation could help here and could possibly reduce the misuse of t().

Cheers,
Stella

mostly agreed - but I think it goes further

greggles's picture

I will agree that a lot of devs just don't know how to properly use t(). But, as nedjo has pointed out and is trying to fix, some of the problem comes from the fact that there is no single well documented way to localize strings that come from the admin interface.

I inherited the comment_notify module and now have a bit of an issue where I want admins to be able to change the text in the message, but I also want that text to be sent through t() when it is used so that it can be localized.

I would love to know how to do this!

A set of best practices would also be good (maybe that exists, maybe not, I didn't look) but things like when to put HTML into the call to t() and when not to - how to handle spaces, etc.

--
Growing Venture Solutions | Drupal Dashboard | Learn more about Drupal - buy a Drupal Book

This is what I've been doing

mfb's picture

This is what I've been doing to allow strings to be customized via either the configuration page or translation. If the variable is empty then I use a translation:

<?php
  drupal_set_title
(($var = variable_get('google_cse_results_title', '')) ? check_plain($var) : t('Search'));
?>

While I'm here.. I would also like to note that it can be unclear what is encoding context of translated strings: HTML or plain text. For example, should translators use an ampersand, meaning the translation should run thru check_plain() when it's being rendered to page output, or an HTML entity-encoded ampersand? In my experience, translated strings are usually considered HTML-encoded, as they are not run thru check_plain(). So translators need to be careful not to use an invalid character such as ampersand (since it's unfiltered the HTML could actually be malicious in addition to invalid), and if the string needs to be used in a plain-text context (plain-text email) it will need to be HTML entity decoded.

I kind of wish Drupal would just get rid of strings.. All strings would be objects with various encodings (HTML, plain text etc.) and translations available.

yes, but

greggles's picture

How do you localize the google_cse_results_title? That's my problem.

Settings...

nedjo's picture

...as stored in the variables table and/or global $conf array have their own custom solution in i18n module, see http://drupal.org/node/67824. The approach involves declaring an array of variables to make translatable in the settings.php file and then translating them through the UI (switch to target language, visit configuration page, enter translated text).

It works but relies on a lot of manual configuration and obviously doesn't address translation needs outside the variables table or global $conf array.

So, if we assume any site that wants to translate variables can use this approach, the answer is, for now, you don't have to do anything.

Though, clearly, a way to inform i18n that your variables are translatable would be very useful. Do we need a hook here?

<?php
/**
* Implementation of hook_i18n_variables().
*
* Return variables handled by this module that should be translated.
*/
hook_i18n_variables() {
  return array(
'mymodule_myvariable1', 'mymodule_myvariable2');
}
?>

Similar link for D5:

stella's picture

Similar link for D5: http://drupal.org/node/134002 Haven't tried it in D6 yet.

Yes, I agree we need some

Jose Reyero's picture

Yes, I agree we need some better and broader approach here, that can be easily used by all contrib module authors.

Though the i18nstrings tt() solution is working at the moment, maybe having an additional dependency for all modules isn't such a good idea, so I think we may work on some alternate approach like this:

  1. Easy two lines adition for all the modules that want to use this feature

    function mymodule_tt($string_id, $default, $language = NULL) {
       return function_exists('tt') ? tt($tring_id, $default, $language) : $default;
    }
  2. You wrap all your module's strings with your 'mymodule_tt' function.

  3. One or more contrib modules that should implement this api, so they can make an easy drop-in replacement.

This way, if you have such translation modules enabled this will 'magically' translate strings, otherwise it does nothing, no new module dependencies introduced.

For now we just need some carefully defined API and consistent string ids. We can build on that later to make full objects translatable.

(About the string ids, I don't think they need to be module related, as they will be used mostly to translate once and again the same 'textgroups' like taxonomy, menu, etc across different modules. So maybe they should be just 'schema-related', including the table, the field, and the key value(s) for that table.... We just need to think a bit more about this to make it consistent and extendable enough)

@nedjo, btw, your schema-based translation module is extremely interesting, I'll follow up on that thread.
http://drupal.org/node/312038

t() get_t() and st()

catch's picture

Although it's somewhat different, there's also the issue of using t() st() or get_t() in .install files - as far as I can see, it should be get_t() apart from strings in the actual install system for core, but I was unable to find docs outside api.drupal.org, and get_t() and st() aren't mentioned on http://drupal.org/node/114774

get_t() and st()

Gábor Hojtsy's picture

Get_t() and st() are not at all new to Drupal 6: http://api.drupal.org/api/function/get_t/5

Great discussion. Someone

moshe weitzman's picture

Great discussion. Someone page Gabor!

Promoted to gdo home page.

Hah

Gábor Hojtsy's picture

I just found this out (I am not a frequent g.d.o homepage visitor, you see). Will reply in detail below.

there is no solution yet

Pasqualle's picture

there is no perfect solution to translate dynamic strings in Drupal. And even D6 core uses t() on some dynamic strings.
Can you find a t() used on node type description? There is a t() used on it.. you can check it with l10n_client, or search for the string here admin/build/translate/search..

we need a general function like tt() and push it into core..

ok, don't search it

Pasqualle's picture

the page (node type) description is used as a tooltip on the "create page" menu item, and tooltips are localized..

I see many other issues first...

hass's picture

Not sure how often there are untranslatable stings, but mostly this is a node title or something like this. Not every dynamic variable in a t() is wrong. This could also be translated somewhere else.

There are much more other issues that developers need to follow more carefully and that wasted my time in last months. Search for my bugs reported as "context sensitive" translation issues. I created some monster patches for i18n, cck, views, og, fckeditor, and many others and nearly every other module I open have the same issues. All this don't do it rules are documented in the API docs but nobody follows them! If the most important and top 10 modules are not following the rules - I think nobody follows them as there is no good example if they read code and learn from code in one of the top 10 modules.

I would mostly agree as well

tjholowaychuk's picture

I would mostly agree as well but some cases (menu module) it is needed. I think as far as hook implementations it is somewhat 'cleaner' to leave out the t(), however like everyone is saying that sort of makes its confusing for anyone else who wishes to use the data, they should not really have to be guessing if the string has been translated or not.


Tj Holowaychuk

Vision Media - Victoria BC Web Design
Victoria British Columbia Web Design School

avoiding dependency

catch's picture

It'd be nice for modules to be able to support i8nstrings without the dependency.

How about something like:

<?php
$t
= module_exists('i18nstrings') ? tt() : t();
?>

no

Pasqualle's picture

using t() instead of tt() is plainly wrong..

Wrapper functions are

moshe weitzman's picture

Wrapper functions are extremely common in Drupal. Are you opposed to all wrapper functions? What if t() became the wrapper? Would you still oppose it, or are you just attached to 't' as a function name? I find your comment to be plainly unhelpful and lacking evidence and argument. Your statement is not self evident.

explanation

Pasqualle's picture

Translating something with t() which should be translated with tt() is a big no no. Translation of dynamic strings should NOT be done with a t() function call..

and the alternative?

catch's picture

Is to have every single contrib module introduce a dependency for i18nstrings so they can access a single function. This solution allows for maintainers to support proper internationalisation without introducing that dependency.

alternative

Pasqualle's picture

there should be a function in core which is used for dynamic string translation..

for D6, yes, if a contrib module needs dynamic string translation, then introduce a dependency for i18nstrings.

dependency

catch's picture

I don't think it's reasonable to expect every module with dynamic strings to introduce a dependency on i18nstrings - that's a large dependency, for quite a lot of modules. Hence the pattern posted above, which would support i18nstrings rather than requiring it.

Smart hacks?

kbahey's picture

Here is a thought ...

How about something like this?

We define a stub tv() function like this in common.inc

<?php
if (!function_exists('i18nstrings')) {
  function
tv($blah) {
   
// Do something
   
return $blah;
  }
}
?>

So, a form of the function is available in core, but no dependency on i18nstrings.

Drupal performance tuning, development, customization and consulting: 2bits.com, Inc..
Personal blog: Baheyeldin.com.

Drupal performance tuning, development, customization and consulting: 2bits.com, Inc..
Personal blog: Baheyeldin.com.

the static text and the dynamic text

Gábor Hojtsy's picture

The static text should be translated with t() and st(). Translation template extractor, coder, etc is on noticing these calls and enforcing best practices. So dynamic data translation needs some other way (because of the structure storage requirement, coder, potx, etc). So relying on or falling back on t() is a totally bad idea.

As for the guide for developers I have one in the works, and I am sure you'll love it. I was just about to publish some early beginnings on the api.drupal.org topics section, but those HTML file formats used there just make me cry. I'd love to make it versioned and let everyone contribute there. So I write a mock PHP file instead to have better formatting?

started to publish my docs on drupal.org

Gábor Hojtsy's picture

I've taken on http://drupal.org/node/194962 quite some time ago and had some results not posted due to trying to target it to api.drupal.org. Instead of starting to struggle with the api.drupal.org infrastructure for some hours, converting my PHP examples from code to colored HTML soap, I decided to do it instead on the drupal.org handbooks in the APIs handbook.

The first fruits of my contributions are available at http://drupal.org/book/export/html/322729 to read :) This forms a new "Localization API" section in the Drupal APIs section of the documentation. Feedback is welcome. I have lots of more pages in the pipeline to be written. Next up is a page titled "Dynamic or static links and HTML in translatable strings". Lots of fun there.

Great work!

nedjo's picture

Gabor: this is just what was needed and provides a tremendous leg up to developers introducing localization support into their modules.

Patch on t() with some of the ideas from this discussion

nedjo's picture

I've put some of the comments and suggestions arising from this discussion into a patch on the documentation for t(): http://drupal.org/node/336115. Comments welcome.

This will enhance api.g.o. as well

markus_petrux's picture

And having api.d.o. in mind, maybe it could also be applied to D6 branch?

really blank with code

drupalsmutant's picture

how to make this code to be easy

Internationalization

Group organizers

Group categories

Group notifications

This group offers an RSS feed. Or subscribe to these personalized, sitewide feeds: