On our site we have a complex menu system with duplicate menu items, e.g. "Shop" used as title and "Shop" also used as an item in the same menu. We exported the .po files to translate the strings. After translating and importing, only one of the "Shop" strings was correctly shown translated although all items had been translated in the .po file .

I tracked down the problem to the _locale_export_get_strings() function in locale.inc. There the source item to be translated is selected by its content, i.e. only using the 'source' and 'textgroup' columns. Its 'location' string is not used to disambiguate between identical source items. So, all translations of the same source string update the same row in the database. The other duplicate items remain without translation and show up incorrectly.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Christian Stümpel’s picture

The name of the affected function is _locale_import_one_string_db in locale.inc. Sorry for that.

mattyoung’s picture

.

Christian Stümpel’s picture

FileSize
1.03 KB

I wrote a fix which seems to do the job for me. I am not absolute sure how to treat static string, if their location value is important or not, so in my patch I use location to select the source item for all textgroups other than 'default'.

Stefan Freudenberg’s picture

subscribe

Stefan Freudenberg’s picture

Status: Active » Needs review
Stefan Freudenberg’s picture

FileSize
1.13 KB

I made the patch comply with the Drupal Coding Standards and did a first review. Looks good. Will do some more testing.

James Andres’s picture

Status: Needs review » Needs work

+1 subscribing.

The 611480_2.patch resolves the issue for me, and yes there is still an issue! However, I get quite a few PHP warnings about INSERT's into locales_target hitting duplicate keys.

To reproduce the issue, use the i18n module's "Export" tab to export and import any menu, nodetype, block, etc. that contains duplicate strings. For example, a duplicate taxonomy term will always cause the issue.

Stefan Freudenberg’s picture

This is a problematic issue. The locale module's approach to internationalization follows GNU Gettext, i.e. a msgid is unique within a textgroup (domain). Apparently on purpose i18n_strings does allow different translations for the same msgid based on the location. In order for i18n_strings() to translate an occurrence of a msgid the location must also match. I assume that's a replacement for the missing Gettext context feature which made it to D7.

Another downside: It gives all users of the Gettext tools a hard time when working with strings from the i18n module. PO-Editor and msggrep simply refuse to work with files that have duplicate msgid entries.

Gábor Hojtsy’s picture

Version: 6.14 » 8.x-dev
Priority: Critical » Major

I've been looking at http://drupalcode.org/project/drupal.git/blame/refs/heads/6.x:/includes/... and http://drupalcode.org/project/drupal.git/blame/refs/heads/7.x:/includes/... but the initial lookup does indeed not include the location.

FYI I'd like to get through the removal of textgroups in Drupal 8. They are a flawed concept for object translation. See #1188430: Rip out textgroup support from locale module. That would move this issue down to Drupal 7. Until that happens though, this is considered a bugfix applicable to Drupal 8 and should be applied there first :|

Gábor Hojtsy’s picture

Title: Importing .po file does not correctly import all user-defined strings (e.g. menu items) » _locale_import_one_string_db() should look at location for non-default textgroups

Retitling to make it clean.

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Version: 8.x-dev » 7.x-dev

Drupal 8 does not have textgroups anymore, so this should go to Drupal 7 straight.

Looking at the patch from Stefan in #6, the last change is IMHO not needed anymore since it was introduced with other fixes in the meantime I think (please verify). The rest of the changes look conceptually good but in no way comply with Drupal coding styles. Whitespace and lack of uppercase in SQL, lack of {} wrapping in control structures, etc.

@Stefan: if you look at the D7 i18n module, it actually uses context AND location (with same value, location primarily for backwards compatibility I think) to maintain the translations for these textgroups. That should allow custom translations for the same strings at different places very well.

Gábor Hojtsy’s picture

Version: 7.x-dev » 6.x-dev
Status: Needs work » Patch (to be ported)

A bit closer investigation shows that contrib textgroups use no location in D7, context is used for their identification exclusively. See http://drupalcode.org/project/i18n.git/blob/refs/heads/7.x-1.x:/i18n_str.... So I don"t think that this applies to Drupal 7 either. Still applies to Drupal 6 though.

kndr’s picture

kndr’s picture

Status: Needs work » Needs review
Issue tags: +locale .po import user-defined
FileSize
1.02 KB

#3 works for me. Duplicated terms are properly imported and I didn't notice any problems so far. I attach the same patch but with fixed coding style. Gábor Hojtsy is right. Change in #6 isn't required since it was commited to the core earlier.

Status: Patch (to be ported) » Needs work
Issue tags: -locale .po import user-defined

The last submitted patch, 611480_2.patch, failed testing.

Status: Needs review » Needs work
titouille’s picture

Issue summary: View changes

Hi,

It seems the patch in #15 isn't completed correctly... The same tests must be applied to the query on line 1393.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.