The l10n_server_parse_po() function has diverged greatly from the latest version of _locale_import_read_po().

This patch is aiming at reconcile the function with the original one, fixing at the same time an issue with importing plurals from po files. The wrong checks were made:
if ($context == 'MSGSTR')
instead of
if (($context == 'MSGSTR') || ($context == 'MSGSTR_ARR'))

This should help resyncing it if needed in the future.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

laurentchardin’s picture

Assigned: Unassigned » laurentchardin
Status: Active » Needs review
StatusFileSize
new13.72 KB
laurentchardin’s picture

I also identified that the current l10n_server_parse_po() method is actually failing plurals *with context*.

I would like to add a test case update for that, but i found out that the current test cases are somewhat dependant from the potx ones. Since i need to modify the test module for strings, it would mean i need to update the potx test cases as well.

For example, i'd like to do the following:

Modify potx_test_7.module:

function potx_test_7_page() {
  t('This is a test string.');
  format_plural($count, '1 test string', '@count test strings');
  format_plural($count, '1 test string with context', '@count plural test strings with context', array(), array('context' => 'Plural context'));
  watchdog('test watchdog type', 'My watchdog message');

and then modify the sample po file from l10n_server and add:

msgctxt "Plural context"
msgid "1 test string with context"
msgid_plural "@count test strings with context"
msgstr[0] "1 test string with context - imported translation"
msgstr[1] "@count test strings with context - imported translation"

What do you think ?

I'm not very keen with having module *dependencies* regarding test cases. Should'nt we have a clean version of the test stuff for l10n_server only ?

laurentchardin’s picture

I've decided to do the other way around, and propose new tests to the potx module to track the context + plurals issue.

See here : http://drupal.org/node/1039552

As soon as i'm getting some feedback, i'll update the test coverage for l10n_server as well.

sebcorbin’s picture

gábor hojtsy’s picture

Does this apply to the Drupal 6 version?

gábor hojtsy’s picture

Version: 7.x-1.x-dev » 3.0.x-dev
Issue summary: View changes

3.0.x still has this bug unfortnately, in L10nPo::parse(). That is the parser of translation imports and of .po uploads. After the msgstr[0], msgstr[1] lines of a plural entry, the parser is in the MSGSTR_ARR state, but currently a msgid or msgctxt line ends the entry only after a msgstr line. Blank lines do not end entries. So when the entry after a plural entry has no comment line:

  • The plural entry is lost. A next entry without a context is read with the context of the plural entry.
  • If both entries have a context, the parse fails with "msgctxt" is unexpected.

The packaged .po files and the compact exports have no comment lines. So currently, a re-import of a downloaded package loses each plural entry that another entry follows.

I ported the suggested fix from the issue summary: a msgid or msgctxt line also ends the entry after msgstr[] lines. New kernel tests (PoParserTest) parse two files with entries after plural entries.

LLM disclosure

LLM was used to triage and diagnose this issue. With human review.

  • gábor hojtsy committed 9879f197 on 3.0.x
    fix: #1960638 [PATCH] l10n_server_parse_po() not working properly with...
gábor hojtsy’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.