Location of imported translations are insertes with a preceding wite space in locales_source. This makes the translation brake for taxonomy (and probably other cases) when using localized terms.

Patch needed!

Files: 
CommentFileSizeAuthor
#34 drupal.locale-import.384794-34.patch318 bytesjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#33 drupal.locale-import.384794-32.patch2.14 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#22 drupal-locale-import-384794-D6.patch2.19 KBjaydublu
#16 384794-16.patch318 bytesmvc
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 384794-16.patch.
[ View ]
#11 drupal-locale_import.patch477 bytesintuited
Passed on all environments.
[ View ]
#9 locale_trim.patch231 bytesalduya
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch locale_trim.patch.
[ View ]
#5 384794.patch316 bytesmvc
Failed: Failed to apply patch.
[ View ]

Comments

sign’s picture

deverman’s picture

I have had the same problem. Not clear how people are doing translation now this seems like a major bug. Posting here to follow.

deverman’s picture

it seems others aren't having this problem so what is the procedure I should take to translating? I tried to export and reimport the translations but this must not be correct. What is the best way to do translation? The hand books don't explain well.

Otherwise please give some direction on how to fix this issue and I will have my programmer look into it.

strellman’s picture

Project:Drupal core» Internationalization
Version:6.10» 6.x-1.0-beta6
Component:locale.module» Code
Status:Needs review» Active

If you are only dealing with menus or taxonomy, Get http://drupal.org/project/translation_table and put in your translations online. It sounded like there was a plan for translation table to delete things from the table where the translation is null, but that didn’t happen in the dev version.

To be sure it wasn’t fixed yet I got http://ftp.drupal.org/files/projects/i18n-6.x-1.x-dev.tar.gz as of 5/25. Notice that the official release is January 25th, about 4 months ago. This dev version still seems to duplicate on import of taxonomy. At least with translation_table you can see what is happening. Moving this to critical because it is impossible to use the translation export/import system as is. (I am assuming that menus have the same problem.)

mvc’s picture

Project:Internationalization» Drupal core
Version:6.x-1.0-beta6» 6.10
Component:Code» locale.module
Priority:Normal» Critical
Status:Active» Needs review
StatusFileSize
new316 bytes
Failed: Failed to apply patch.
[ View ]

Problem description: It is currently not possible to export a .po file from Drupal, edit it in an external editor, and re-import the updated file. Whitespace in the .po comment line is preserved, so that entries in locales_source such as 'term:1:name' are imported as ' term:1:name'. This is particularly problematic because Drupal inserts a space before the contents of locales_source.location in the comment line when exporting a .po file, thus a string with a location such as 'term:1:name' would be exported with the comment '#: term:1:name'. As a result, the uploaded translation for taxonomy term id 1 is stored as a new row in locales_source with the extra space character, and is never found by i18n_taxonomy, which understandably believes that no translation exists for that term. Other i18n_* modules behave similarly.

Proposed solution: Instead of changing how Drupal exports .po files, I think the import process should be more lenient, and trim extra whitespace from the string identifier in locales.inc. Trivial patch attached.

I will post suggestions for cleaning up the locales_* tables for users who have already encountered this bug while importing .po files in the forums at http://drupal.org/node/429822 since that's beyond the scope of this patch.

alduya’s picture

Project:Internationalization» Drupal core
Version:6.x-1.0-beta6» 6.10
Component:Code» locale.module
Status:Active» Reviewed & tested by the community

I had the same problem.
I exported .po files for translation. D6 imported these translations, but created new entries instead of updating the existing variables. This also gave double entries when searching for translation in the 'Interface translation' pages, which had an almost unnoticeable difference in their location string.
I tested the patch and it works great.

Gábor Hojtsy’s picture

Version:6.10» 7.x-dev

Fixes are committed to Drupal 7 first to ensure that we do not introduce regressions. I agree this patch seems like the right solution.

Status:Reviewed & tested by the community» Needs work

The last submitted patch failed testing.

alduya’s picture

Status:Needs work» Needs review
StatusFileSize
new231 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch locale_trim.patch.
[ View ]

I remade the patch so that it works in D7, also ran SimpleTests and they also work.

Status:Needs review» Needs work

The last submitted patch failed testing.

intuited’s picture

Status:Needs work» Needs review
StatusFileSize
new477 bytes
Passed on all environments.
[ View ]

Looks like that last patch came in a bit too late, the line number is off by one.

Here's another go.

brianV’s picture

Status:Needs review» Reviewed & tested by the community

Tested, fixes this issue. RTBC.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Commit to CVS HEAD. Thanks.

Status:Fixed» Closed (fixed)

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

maijs’s picture

When is this patch coming to Drupal 6.x core?

mvc’s picture

Version:7.x-dev» 6.x-dev
Status:Closed (fixed)» Needs review
StatusFileSize
new318 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 384794-16.patch.
[ View ]

now that this has been committed to 7.x, i've rerolled my patch from #5 for 6.16.

brianV’s picture

Status:Needs review» Reviewed & tested by the community
intuited’s picture

How come the 6.x patch is still queued for testing?

heliotrope’s picture

Same issue Here.
I notied that teh group setting was not applied when importing .po (for instance cck).
Is that the expected behaviour ?

:)

Gabriel Radic’s picture

The patch in #16 does NOT fix the issues for Drupal 6.x. It actually modifies the wrong function. Strings are being re-created with the extra space prefix in the location column.

What you need is to add the following line just after function _locale_import_one_string_db(...

$location = trim($location);

PS: Don't ask me to roll a patch.

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Needs review

@Gabriel: If it was good for D7, how is it not good for D6? Was it not good for D7 either? How can you still reproduce the issue with the patch applied?

jaydublu’s picture

StatusFileSize
new2.19 KB

When Drupal creates an export file, the location of each source string is passed in the .po file as a 'reference' entity which has the format '#: reference...' i.e. they 'look' like a comment with the space preceeded by a colon.

The problem here is that _locale_import_one_string() uses a relatively crude function _locale_import_shorten_comments() to extract this reference to use as the 'location' of the translation to be passed to _locale_import_one_string_db(). It would seem that the original purpose of _locale_import_shorten_comments() was to combine comments into one long string, and it does not properly handle references, nor would it handle multiple comments in this context.

In the attached patch (for D6) _locale_import_one_string() calls a new function _locale_import_get_reference() which iterates through an array of comments (i.e. entities that are prefixed '#') looking for a reference which it extracts and passes back.

In addition to fixing the immediate issue identified, the advantage of this approach over the 'trim()' solution suggested above is that it is tolerant of any other comments that might be present in the .po file and these do not break the correct import of the string.

Gabriel Radic’s picture

Status:Needs review» Fixed

From my tests with the current stable versions of Drupal and i18n, this issue is fixed.

Status:Fixed» Closed (fixed)

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

catch’s picture

Status:Closed (fixed)» Needs review

Re-opening this, there's no explanation as to how the patch is fine for D7 and not for D6, there's not much variation between versions for this either.

Status:Needs review» Needs work

The last submitted patch, 384794-16.patch, failed testing.

dude4linux’s picture

I encountered this problem while trying to implement a multilingual forum website. After getting the site working on a development server, I tried to export the translated strings and import them on the production server to avoid having to re-enter all the translations. The result was a broken translation. Applying the patch in #22 and re-importing the .po files repaired the damage.

Is there anyway we can get some attention to this problem in 6.xx? I appreciate that the problem has been fixed in 7.x, but I don't plan to upgrade to 7.x for some time. I don't like to have to keep patching core.

TIA - Dude

catch’s picture

Status:Needs work» Needs review

Not sure why the bot marked this needs work, setting back to CNR.

subho007’s picture

#9: locale_trim.patch queued for re-testing.

catch’s picture

Please don't re-send this for testing, it's against Drupal 6 which does not have any automated tests.

jhedstrom’s picture

The patch in #22 resolves the issue in my limited testing. I'm not sure how anybody is able to import functional i18n translations without this patch.

jhedstrom’s picture

Status:Needs review» Needs work

Patch in #22 has some code style issues. I'll reroll a new one shortly.

jhedstrom’s picture

Status:Needs work» Needs review
StatusFileSize
new2.14 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Here's the same patch as #22, with code style issues resolved.

jhedstrom’s picture

StatusFileSize
new318 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Reading through this more carefully, the patch in #22 (rerolled in #33), is way too complicated. The original backport from 7.x from #16 resolves the issue. Re-attached here to cleanly apply.

mvc’s picture

Status:Needs review» Reviewed & tested by the community

#34 resolves this for me. i see that the approach in #22 would be even more flexible. but since the original patch in #16 was backported from 7.x, adopting a new approach here would mean introducing a regression unless the change in #22 was first committed to 7.x. i suggest applying this patch to solve the whitespace problem, and opening another issue for 7.x to allow other comments in .po files if needed. since this patch is needed to allow drupal to import its own exported .po files, i respectfully suggest it not wait until that separate issue is dealt with.

Sweetchuck’s picture

The problem is more complex than the leading whitespace. Therefore the #34 patch is not enough.
Other problem:

<?php
// @@ -1390,7 +1407,7 @@ function _locale_import_one_string_db(&$report, $langcode, $source, $translation
db_query("INSERT INTO {locales_source} (location, source, textgroup) VALUES ('%s', '%s', '%s')", $location, $source, $textgroup);
$lid = db_result(db_query("SELECT lid FROM {locales_source} WHERE source = '%s' AND textgroup = '%s'", $source, $textgroup));
?>

When retrieve the $lid, the location is ignored.
The location is unique ID for strings in textgroups (taxonomy, cck, etc…)
If you have two menu item with same title, always get the ID of first one.

I had created an issue, which is duplication of this issue,
because I was search in "language system" component, not in the "locale.module".
This patches does not affects for the locale module. (But this is not important)
There is a patch, which is very similar to #33 patch
#1143368: PO export/import ignore the text group location

This bug is exists in D7 too.

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Fixed

Ok, I've committed this one to make sure any new discussions will not stop the basic patch from landing. I understand there are more bugs around here, so please either open up this one again if you think the issue is highly related, or reopen the other one, if you think not. Thanks for your contributions everyone! Keep it up.

Status:Fixed» Closed (fixed)

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