Internationalization support

Adding some tags...

Sounds like a much better way to translate labels and descriptions that what we currently do (t($field['widget']['label'], yuck).

Problems, however:
- how will this new approach play with the current t($field['widget']['label'] code ?
I'm not sure how we can avoid breaking existing CCK D6 sites.
That approach could be useful for D7, though.
- the _content_type_info is persistently cached, so the module in #531660: i18n + CCK will cache the translations in the language that triggers the cache refresh. See D7's #503550: Translated strings are cached in _info() hooks.
- I'm a bit skeptical about translating allowed values - sounds like this could have strange effects.
- I'm frankly puzzled by

Translating field values on display ? Sounds like a slippery slope... And why only within Views ?

The current code seems a little rough still.
Also - apart from these initial remarks, I most likely won't have the time to help there.

Thanks for your feedback, Yves. I put 4 days on digging into CCK, Views and i18n to get this done. I was concentrated mainly on Text+Options widget and Views integrations, so some parts might really passed me by. Anyways, here's the answers:

> how will this new approach play with the current t($field['widget']['label'] code
Gotcha, I didn't spotted these calls. They should be removed, if we want to use tt() approach.

> the _content_type_info is persistently cached
I hope I understood you right. My alterations are being triggered after the cache is written to DB, so there are no problems with it, I guess.

> I'm a bit skeptical about translating allowed values - sounds like this could have strange effects
Well, I don't think so, because I'm not touching keys, only values. Also, this alteration translates field values on node pages (at least for options field).

> Translating field values on display ? Sounds like a slippery slope... And why only within Views ?
In fact, not only in Views. It will be called before any content_format() call. For instance, take a look at content_handler_field.inc, ::render(). There are only content_format() which can be used fur such alterations.

> I'm not sure how we can avoid breaking existing CCK D6 sites.
I suppose, we can write some additional code to handle this (like "if (module_exists('i18ncck')) { ... tt() ... } else { ... t() ... }"). Many people do it in this way.

t($field['widget']['label']) is also used by many modules. If we give them translated, will break them. So I think this change, for D6, is big -1. It is too late, in my opinion.

If something can be translated, in the event that the method to store these translations is changed, then such a change should provide a method to migrate translations to the new storage, and this would have to be transparent a very well documented to minimize the impact. I'm not sure yet if this was going to be possible, but just in case... CCK for D6 is used by more than 100 thousand sites out there, and surely many of them had to translate widget labels.

In regards to allowed values... TBH, I'm not sure yet how these are actually translated. I haven't looked. If it is possible, then I would say the same as with widget labels. If it is not possible, then I think it CCK would have to do something about it. I haven't looked at this, so I cannot really tell.

This is not a CCK3 issue, so if we need to change anything here, it needs to be against CCK 2.x, then ported to 3.x.

Okay, I see. So, the problem is only in labels? If so, I can disable by default the new way of translation of labels in i18ncck. But I don't want to throw it away completelly as there a lot of places where default way of translation simply doesn't works. For example, in views, there's a option to show table headers as widget labels. They aren't being affected by t().

So, for people who will not get i18ncck everything will remains the same. People with i18ncck will get correct localization, but also they get 3-10 junk strings in the database (already translated strings will be passed to t()), which I think is not very critical, as we already have another junk data there (t() is not allowed for dynamical data).

It's compromise variant, as for me.

As for the versions, sure no problem, here's a patch for 6.x-2.x. Actually I worked with it initially, but I thought that approval in CCK just like in core — from bigger version to minor. But anyway, patch is in attach.

Attachment	Size
cck-drupal_alters-for-i18ncck-2.x-#531662.patch	889 bytes

FYI, we were talking about that here: http://drupal.org/node/293297

So guys, any decision?

Once again, this patch itself isn't doing anything wrong. As for the labels, here's an example:

1. We have field "Image" ($field['widget']['label'] == "Image").
2. We made a russian version of the site and traslated "Image" as "Картинка" throught the translation interface (t($field['widget']['label']) == 'Картинка')
3. We updated site to new CCK verion (with my patch). Absolutelly nothing happened, as nothing implements alter-hooks.
3. We decided to make translations via i18n and enable i18ncck for it. i18ncck imports the current field translations and start to translate labels (update function should be added to i18ncck tomorrow). Now $field['widget']['label'] == 'Картинка', so t($field['widget']['label']) == 'Картинка' too! So, nothing is broken! (Just like we need) And in future versions of CCK (or even in 7.x) we'll just get rid of t() at all to make everything just a little bit better, because localized strings in locale sources table is not what it suppose to contain (just like any other custom strings even in English).

P.S. Previous one redirects to the front page of drupal.org by some reason, here's another try.

Attachment	Size
cck-drupal_alters.patch	1.04 KB

To be honest, I don't feel myself able to evaluate i18n related changes to CCK because I haven't studied this subject much. I would prefer this is reviewed by yched and/or KarenS.

One thing I believe is not a very a good approach is these drupal_alter's. One can use them to change things that should not be possible to change (dangerous approach I would say). Also, I'm concerned about the fact that all modules will be scanned while probably only (say) i18ncck would be invoked (performance).

The drupal_alter() calls could be dangerous but I agree that might be a cleaner approach. Translating allowed values would work (I think) so long as the keys are never translated. The recommended method of creating allowed values identifies the key|value in the list, but some people do it with just a list of values that ends up using the same value for both the key and the value, and if you translated them everything would break.

But as yched mentions above, the biggest problem is caching. On a multi-language site you will be caching content_info and content data in whatever language was used at the time the cache was refreshed. I don't see how this patch will work with multiple languages. I assume you would need to find a way to create separate cache entries for each item for each language for this to work. But I don't know enough about the translation processing to know how/if that would work or what it would do to performance.

Hey, I'm _already_ using this patch and the i18ncck on the live site with enabled caching. Everything works fine, because I'm translating after cache get/set, recheck the patch, please. The key|value problem isn't exists, as even if user enters just a value, CCK sets key equal to value (but before I translate the data, so there are correct values).

This usage of t(), besides going against Drupal guidelines (see t() function api comments) makes it impossible for other modules do use some cleaner approach (like i18nstrings), see #531660.

And anyway, what is the point of translating field names if you cannot translate content type names? I think the alter hooks would be great so you can have views properly translated by another modules.

About backwards compatibility, you can create a small module implementing the alter hooks and just running strings through t(), while anyone else wanting a cleaner solution can just not enable it and use i18n modules.

Caching should be always per language, you just need to add a language code to the cache key.

Subscribe.

For the project I'm working on, we're starting to play with i18n right now, so I expect to have time to review this CCK patch and the i18n+CCK module, hopefully soon.

drupal_alter() against the whole content types structure could be dangerous, as it opens the door to other module to alter things that shouldn't be altered. I'll try to find a way to just expose a structure of data for i18n.

Here's a patch that's only focussed on the field label and description, the content_type_info stuff.

Changes:
- Content type info is cached per language.
- drupal_alter() exposes only field labels and descriptions, one field at a time.

This approach should simplify the code in i18n+CCK, as a new loop against all fields is not necessary. On the other hand, we're doing drupal_alter() per field. Overall, I think this is a safer approach that doing drupal_alter() against the whole content type info struct.

If this is validated, I think we could commit, and then see what can we do with allowed values, etc.

Attachment	Size
content_i18n-531662-14.patch	2.71 KB

Looks nice.

@neochief: maybe you could adapt your i18n+cck module at #531660 to match this new approach ?

If no one else has objections to the patch in #14, and no error is found, I think we can commit here.

I would proceed with this patch, and once committed, then focus on allowed values and content_format stuff. Those are things I'm not 100% sure, yet.

I've been thinking a bit more about 2) allowed values and 3) content format.

2) allowed values.

The function content_allowed_values() can generate allowed values from PHP code, and from a list in the form key|value (where value is optional in which case key is also value). Well, I think the former does not need drupal_alter() because the PHP code could provide translated values if that's what they want. So, drupal_alter() needs only to be invoked on the second case.

3) content format.

Like yched in #2, I don't see the benefit of translating raw field values in content format. Also, note the structure of $item may vary from field to field. Not all fields have 'value'. Second, the output of the fields depend on the formatter, and the final rendered form of the field may vary a lot. There may be formatters that do not even generate text, or they generate an IMG where the title attribute comes from tokens, for example.

So, I think we can deal with 2 of the 3 places where we can provide drupal_alter() for i18ncck.

Here's a patch that, besides the code in #14, adds a drupal_alter() for allowed values, but only when these come from a list, not when these come from PHP code.

Attachment	Size
content_i18n-531662-17.patch	3.05 KB

Minor: missing "to" (allow xx to yyy)

Other than that, this approach looks neat !

Committed the patch in #17, including "to" in inline code comments as suggested. Thanks, yched, for reviewing!

I'm marking this as "needs work" because we all forgot about field group labels. I'll look into that as soon as possible.

@neochief: Probably, your i18ncck module can now awake in the i18n queue. ;-)

OMG! ...We are now experiencing memory limit problems here at the office. The problem is static memory in _content_type_info(). :(

I think the solution to this is caching in database per language, but still keep one version of $info in static memory. I'm going to commit using this approach.

[DONE]
For the reference:
- patch 1: I have committed this one to rollbacks the patch in #17 and apply the change using the new approach, not caching per language in static memory.
- patch 2: This is the diff between before and after the patch in #17 and the result.

Patch was incomplete. $info['content_types'][$type]['fields'] included unaffected widgets, which was shown on node form. Also, content type information is needed by i18ncck, so I added it in widget's alter.

Attachment	Size
cck-content_types.patch	1.5 KB

"I think the solution to this is caching in database per language, but still keep one version of $info in static memory"
True, actually that's the approach taken in D7's #503550: Translated strings are cached in _info() hooks.

Neochief's #21 looks right too.

Sweet. Committed the patch in #21 to CCK2 and CCK3.

I'll look into the fieldgroup module now. The function to look at is fieldgroup_groups().

Here's a patch that deals with fieldgroups.

• A new drupal_alter('content_fieldgroup_strings') has been added to allow translations of group label, form description and display description. This means i18ncck needs to implement a new hook_content_fieldgroup_strings_alter() to deal with fieldgroup strings.
• drupal_alter('content_field_strings') is also invoked here because field labels and descriptions are also loaded in the 'fields' array of each group. Note this time 'label' and 'description' for fields is not nested in a 'widget' subarray. This is transparent from the POV of the drupal_alter() implementation.

Please, test and let me know how it looks. Let's get this stuff in, so we can easy the task of translators when dealing with CCK stuff. :)

Attachment	Size
content_i18n-531662-24.patch	6.51 KB

Much need functionality! Thank you all for working on this!

Also, subscribing :P

@all: If you have the chance, please test the patch in #24. Whether it breaks anything in CCK or not can be tested as-is, even without i18ncck ready for that. Thanks.

I'm trying to test this patch, but I need some directions, I have cck 2.5 installed unmodified... Should I start with the patch in #17 and then #21 for the content.module file and then apply #24 to the fieldgroup.module file?

What about the memory limit problems markus_petrux refers on #20? Do I need to change anything else?

Any pointers would be wellcome, thank you!

All patches in this issue are already committed to CVS, except the one in #24, which is related to fieldgroups. To try all this stuff, you would need to download the latest 6.x-2.x-dev snapshot of CCK. Then, apply the patch in #24.

The memory issues in _content_type_info() were fixed already in CVS.

I have adapted the i18ncck module posted in the i18n issue with support for fieldgroups. Also tested here and it seems to work like a charm. Reference: #531660: i18n + CCK

Committed the patch in #24 to CCK2 and CCK3.

Hello Markus, thanks for the pointers, I didn't realize it was commited to CVS, anyway...

I've tested the patch and it's working great here to! Field groups are also working correctly, I'm going to test the i18n submodule now...

Thank you and all that worked on this patch!

You're welcome. :)

I'm now working on a patch for translation tables module to add support for CCK strings as well. I'll probably post the patch here: #499668: Possibility to integrate cck fields?

Sorry, didn't meen to change the status...

Also posted a patch to add support for CCK to Translation table module, here: #499668: Possibility to integrate cck fields?.

Cheers

Great job, guys !

I opened an issue for D7 Field API: #549698: Prepare field label and description for DDT (translatable queries)

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

Great !!
patch is NOT in CCK 2.5 but in the dev version and works as expected

Hi,

I installed the dev version of cck for drupal 6.x but I still can't see my labels in other languages... (I translated it through the Translation table module)
I am a little bit confused concerning these patches, because I have found 3 different threads apparently working on the same thing :

http://drupal.org/node/293297
http://drupal.org/node/531660
and this one...

So what is best to do at the moment to have this working ??
(patch ? dev version ? tricks in the content-field.tpl.php ?)

Shouldn't we open a new clean thread ?

Thanks for your help !

I also installed the patch on top of the dev version, (This one: http://drupal.org/node/531660#comment-1933270)

It seemed to work (created the strings for CCK labels), but after translating them (Those from the CCK textgroup), the translated labels still don't show up...
I refreshed / updated translations, and ran cron...

Did I miss something ?

@fourmi4x: Please, check out the -dev snapshots of CCK and i18n, and read the file CHANGELOG.txt

If you run into any problem after that, then please open a separate issue in the proper queue. Thanks

Hi,

Thank you for your answer.

I don't know why it works now... Thanks again !