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.
Comment | File | Size | Author |
---|---|---|---|
#15 | core-611480-1-6.22-D6.patch | 1.02 KB | kndr |
#6 | 611480_2.patch | 1.13 KB | Stefan Freudenberg |
#3 | 611480.patch | 1.03 KB | Christian Stümpel |
Comments
Comment #1
Christian Stümpel CreditAttribution: Christian Stümpel commentedThe name of the affected function is _locale_import_one_string_db in locale.inc. Sorry for that.
Comment #2
mattyoung CreditAttribution: mattyoung commented.
Comment #3
Christian Stümpel CreditAttribution: Christian Stümpel commentedI 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'.
Comment #4
Stefan Freudenberg CreditAttribution: Stefan Freudenberg commentedsubscribe
Comment #5
Stefan Freudenberg CreditAttribution: Stefan Freudenberg commentedComment #6
Stefan Freudenberg CreditAttribution: Stefan Freudenberg commentedI made the patch comply with the Drupal Coding Standards and did a first review. Looks good. Will do some more testing.
Comment #7
James Andres CreditAttribution: James Andres commented+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.
Comment #8
Stefan Freudenberg CreditAttribution: Stefan Freudenberg commentedThis 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.
Comment #9
Gábor HojtsyI'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 :|
Comment #10
Gábor HojtsyRetitling to make it clean.
Comment #11
Gábor HojtsyMarked #918344: _locale_import_one_string_db() makes assumptions on PO import as duplicate.
Comment #12
Gábor HojtsyDrupal 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.
Comment #13
Gábor HojtsyA 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.
Comment #14
kndrMarked #1261738: duplicate terms with different term id are not imported as duplicate.
Comment #15
kndr#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.
Comment #18
titouilleHi,
It seems the patch in #15 isn't completed correctly... The same tests must be applied to the query on line 1393.