Project:Content Construction Kit (CCK)
Version:6.x-2.x-dev
Component:General
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

Hi,

CivicActions is reviewing and upgrading multiple modules for use on client sites. Part of this work is a coding standards review of internationalization handling of strings. Attached you will find a patch based on a review with the coder module and a careful examination of the code. Thanks!

The changes are all related to the handling of links in strings passed to t(). Currently you are using the format:

t('some text !link ', array('!link' => l('my link text', 'http://www.drupal.org'))),

However this doesn't give any context to the translator and the following format should be used instead:

t('some text <a href="@link">my link text</a> ', array('@link' => url('http://www.drupal.org'))),

Cheers,
Stella

AttachmentSize
cck_i18n.patch4.87 KB

Comments

#1

Very minor, but I'm not sure about:

+        '#description' => t("Advanced usage only: PHP code that returns a default value. Should not include &lt;?php ?&gt; delimiters. If this field is filled out, the value returned by this code will override any value specified above. Expected format: <pre>!sample</pre>Using <a href=\"@link_devel\">devel.module's</a> 'devel load' tab on a %type content page might help you figure out the expected format.", array(

Core tries to avoid backslashes within t() for clarity where possible. I think we can do this without damaging the wording too much, or even slightly improving it i.e.:

+        '#description' => t('Advanced usage only: PHP code that returns a default value. Should not include &lt;?php ?&gt; delimiters. If this field is filled out, the value returned by this code will override any value specified above. Expected format: <pre>!sample</pre>To figure out the expected format, you can use the <em>devel load</em> tab provided by <a href="@link_devel">devel module</a> on a %type content page.', array(

#2

I've attached another version of the patch - the only difference is the slight adjustment to the wording as proposed by catch.

Cheers,
Stella

AttachmentSize
cck_i18n.patch 4.88 KB

#3

Status:needs review» reviewed & tested by the community

Thanks Stella. Either of the two patches should be RTBC depending on the wording you want to use.

#4

Status:reviewed & tested by the community» fixed

Committed. Thanks!

#5

Status:fixed» needs work

Does this url() really makes sense? url('http://www.drupal.org/projects/schema') I would remove it... aside <b> should be changed into STRONG. And i wouldn't write .</b> -> </b>. might be better.

#6

Status:needs work» needs review

Here's a patch to fix the url() bit. I agree <b> tags should be changed to strong but it's not really an internationalization issue, so I've omitted that from this patch.

Cheers,
Stella

AttachmentSize
cck_url.patch 4.37 KB

#7

I know STRONG is not an i18n issue, but getting the strings STABLE is a real issue for translators. So better we get them permanently stable than changing every few weeks :-(.

#8

Ok well here's a patch that just changes <b> to <strong>.

AttachmentSize
cck_strong.patch 3.33 KB

#9

The url('http://www.drupal.org/project/devel') bugfix is missing :-)

#10

hass: they're two separate patches, by design. Both will need to be applied.

#11

Status:needs review» fixed

Committed.

#12

One more thing which you already know, but just for completeness, running the "interface text translatability" coder review provided by the potx module highlights additional internationalization issues.

The problem for all of these are the same. The t() function is not intended for use on $variables, constants, strings that have already been passed through t() or strings which have variables concatenated onto them. Running t() on $variables also prevents these strings from being extracted by the potx module for translation. This is why the i18nstrings module provides the tt() module, which you should consider using instead. However, it's not an ideal solution, see http://groups.drupal.org/node/15177 for a discussion.

Cheers,
Stella

#13

@Stella: Not every t'ified string using a variable is wrong. I know potx complains, but "all" this strings are somewhere else defined correctly and I'm not aware about a string that is not extracted with potx... have you found one that is untranslatable?

--project followup subject--

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

#14

Status:fixed» closed (fixed)

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