Textgroups bug: cannot import from/into a newly created textgroup [#192406]

I have tried to create a separate textgroup for a group of modules. I implemented hook_locale (groups operation), and everything seems to work according to the interface. Importing a po file into a textgroup seems to work (strings appear translated in the interface). However when I try to export a po file from the new textgroup, it is empty. If I look into the table locale_source, the textgroup of the new strings is still 'default'. I would have expected the textgroup to be the newly created textgroup.

Comment	File	Size	Author
#10	install-textgroup-import.patch	681 bytes	JirkaRybka

#2	locale-groups-import.patch	1.67 KB	Gábor Hojtsy

Comments

Comment #1

JirkaRybka CreditAttribution: JirkaRybka commented 15 November 2007 at 11:28

I came across textgroups a while ago, and it seems to be a bit unfinished feature as it stands now. I'm not sure about import/export - this part needs to be examined more, if relevant here. But otherwise, although admin UI supports textgroups nicely, t() and friends currently only support the 'default' textgroup. So as far as I understand it, if anyone wants to translate using any other textgroup, he need to do it independently on db level.

Comment #2

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 15 November 2007 at 14:31

Status:

Active

» Needs review

File	Size
locale-groups-import.patch	1.67 KB

Well, as far as I see, the only problem on import is that _locale_import_read_po() gets the group properly, but does not pass it through to _locale_import_one_string(), which could accept it nicely. So all imports default to the 'default' group. Although I did not test this, I looked through the code deeply, and this seems to be the only problem from the looks. Please apply the attached patch and test.

About the usefulness of textgroups, we discussed this to great lengths in Barcelona, and Jose said he fully intends to use them in the Drupal 6 version of textgroups. And he does just that already: http://www.flickr.com/photos/reyero/1818917139/ (Although I suggested textgroups originally, I was a bit dissatistfied that we did not find a core use case for them, and did not flash out the bugs like these, but Jose uses this in contrib already, and seems like others also caught up with it, so great :).

Comment #3

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 19 November 2007 at 15:16

Guys, I think this is very easy to test. Why not do it? :) It would be great to release Drupal 6 with this fixed.

Comment #4

karna CreditAttribution: karna commented 20 November 2007 at 21:36

I'll test it tomorrow night.

Thanks for the patch...

Comment #5

karna CreditAttribution: karna commented 21 November 2007 at 21:49

Status:

Needs review

» Needs work

I reviewed the patch and the patch fixes the problem reported in the bug. I can now import strings in a specific textgroup and they are stored correctly in locales_source and locales_target. I can export these strings from the textgroup and get them back.

However, it introduces a lingering problem. The strings are now sitting in the new textgroup, and not in default. The result is that the translated strings are not shown. I haven't been able to pinpoint or fix it, but it seems that in the locale() function that actually translates the string, the textgroup in the query is hardcoded to 'default'. As the string is not there, the original English string is returned. E.g.

$result = db_query("SELECT s.source, t.translation, t.language FROM {locales_source} s LEFT JOIN {locales_target} t ON s.lid = t.lid AND t.language = '%s' WHERE s.textgroup = 'default' AND s.version = '%s' AND LENGTH(s.source) < 75", $langcode, VERSION);

A solution may be to delete the "s.textgroup = 'default'" where clause from the queries (at least it seems logical to look for translations in any textgroup as long as none is specified). I've updated the queries in locale() but that does not seem to do it.

Comment #6

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 21 November 2007 at 22:07

Status:

Needs work

» Fixed

Thanks for testing, I just committed this patch.

Your other problem is actually "by design". The default textgroup is supposed to be used by the locale system itself. For other textgroups, the locale system provides storage, a translation user interface and export functionality. Modules such as i18n use textgroups to translate menus, taxonomy terms, user profile categories. These should and will not overwhelm the locale() function. So textgroups are a feature to let other modules store their translatable data here and use the gettext functionality, and is not a feature to segment usual interface translation strings.

Comment #7

karna CreditAttribution: karna commented 22 November 2007 at 10:17

Thanks for fixing this bug and explaining the textgroup functionality. I was (planning on) using the textgroup functionality to be able to have all strings from a set of custom modules grouped together, making it more easy to maintain translations for them without getting overflowed with untranslated strings from other modules. Now importing and exporting works correctly, I can still do that, only I need to import the translations to the default textgroup as well to get them displayed.

Yesterday, I have been reading the discussion about textgroups on drupal.org and I think it will make sense to have the default t() and locale() functions look in all textgroups if no textgroup is provided (or first in default, and if no translation is found, look into the other textgroups). This way you have a good default behaviour. Custom modules can then make their on translation implementation if they need it.

Comment #8

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 22 November 2007 at 10:26

l10n_server and/or l10n_client will be your friend if you'd like to have closer control on translations of a set of modules. The l10n_server UI rework is in the works to make it more usable and there are discussions into making l10n_server usable onsite (ie. translating modules installed), or have an l10n_server-like interface on top of l10n_client instead. See http://hojtsy.hu/blog/2007-oct-10/comparison-your-localization-options-d...

Comment #9

karna CreditAttribution: karna commented 22 November 2007 at 12:23

I'll look into the l10n_server. I've seen it in action during the D6 dutch translation sprint during drupaljam 2007, and it certainly looks as if it is very usable for my problem.

Thanks, F.J.

Comment #10

JirkaRybka CreditAttribution: JirkaRybka commented 22 November 2007 at 23:06

Version:	6.0-beta2	» 6.x-dev
Priority:	Normal	» Critical
Status:	Fixed	» Needs review

File	Size
install-textgroup-import.patch	681 bytes

Re-opening as critical, because the current 6.x-dev fails to import translations in install.php. All translations are now imported with textgroup column empty, and so nothing shows up for 'default' - no strings translated on the newly installed site.

The bug was introduced with the commit earlier in this issue: _locale_import_read_po() needs a sensible default for the $group argument. Although _locale_import_one_string() have the default right, it seems that NULL got through (sorry, no time to check php docs now; I'm on php 5.1.6).

Attached patch fixes the problem for me.