Translatable entries not HTML encoded

If t() is not used in some module, then it is the problem of that module, not the problem of l10n_client. Also, there are strings showing up from your theme (like region names), which are not shown on the page. The Drupal 5 version also shows menu item names. I don't see a bug here, as you did not specify and specific buggy behavior of the module. Maybe you have specifics to say?

Could be an encoding issue what i found, too. This patches should fix this. The D5 patch is little bit different, while is sync's code with D6 and cleans up a small part.

Aside this patches are fixing XHTML validity bugs...

I have an error when trying to apply this patch l10n_client_encode_D5.patch

patching file `l10n_client.module'
Hunk #1 FAILED at 223.
Hunk #2 FAILED at 299.
2 out of 2 hunks FAILED -- saving rejects to l10n_client.module.rej

I'm not able to repro this hunks. The lines have changed a little bit, but this isn't any issue. Nevertheless, updated patch attached.

patch -p0 < l10n_client_encode_D5_2008080101.patch
patching file `l10n_client.module'
Hunk #1 FAILED at 204.
Hunk #2 FAILED at 307.
2 out of 2 hunks FAILED -- saving rejects to l10n_client.module.rej

Do you apply to CVS version?

No! I've applied it to version from a package l10n_client--5.x-1.0.tar.gz.

Patches are every time build against latest CVS version and not against an old release.

OK, now I saw new version.

The patch solved the issue: #281721: shift in the string list

but there is still problem with the patch:
after the patch, instead of

@count hét

I see

@count hét

can it be fixed somehow?

This is an UTF8 problem... that patch uses only htmlentities() and your "e" is not changed. Maybe your server config have issues... drupal is UTF8...

See http://www.php.net/manual/en/function.htmlspecialchars.php for the chars that this patch fixes.

Any good example page to test this with so I can reproduce the bug and see that this is indeed fixing it?

I am not sure, but the first string on admin/build/modules:

(<span class="admin-disabled">disabled</span>)

is one which is not encoded, and displayed differently after the patch.

2. using htmlentities($something, ENT_COMPAT, 'UTF-8') fixed my UTF8 problem

I've also tested the patch. A couple of issues:

1. UTF-8 problem as reported above. I did reproduce it. See after_2.jpg.
2. There is a strip_tags() in the "selection list" builder so that you can see and search actual strings, instead of useless HTML markup taking up space there. Now if you add an escaping code before it, that will obviously not remove the HTML bloat. See after_1.jpg.
3. The escaped tags appear in the source display as escaped and also get copied to the editor textarea escaped. This is clearly not desired.

Looks like this patch needs some work, please keep up the improvements!

Ups... this is really not desired.

After much testing, this patch fixes several issues I had. They all result in the same symptoms as above so i've rolled them all into a single patch. Maybe they should be broken into separate issues/patches, but since they seemed to all be causing the same problem, this is how i did it.

1. A php string from a views header got into the locale table, so I ended up with really ugly half-php code in the table. I fixed this in l10n_client_footer, by simply not including anything with <?php in it. This part is definitely up for debate. I am currently discussing the bigger issue involving views with merlinofchaos and nedjo, but this seems like a reasonable fail-safe for this module.

2. HTML strings were being injected as-is into the DOM lookup table. So if there was a mis-matched tag or something along those lines, the entire lookup goes bad. This is what was causing the mis-matched source and target panes. In my case, the offending string was < and <=, which I believe are translated in views. This is remedied by doing an htmlspecialchars() on the output, then using $().text() rather than $().html() in the javascript.

3. Along the same lines as #2, strip_tags() aggressively strips < and >. So in the above instance, the < string in the string selector pane was not displaying. There was also a string like < None > which also displayed as an empty selection. This along with the mis-matched panes caused quite an odd user-experience. The way I fixed this was to check for an empty string after doing the strip_tags() and in the case of an empty string, resort to doing an htmlspecialchars() on the original string. This way we get nice looking strings for everything except these edge cases.

4. This last fix also made brought to my attention that the ... for truncated strings was being added manually, instead of letting truncate_utf8() handle it for us.

1. I am not sure the PHP check belongs here. PHP code should not be added for t().

2. Using .text() would not get over the HTML tags from the source strings, so users will not translate things like links and other inline elements. Using htmlspecialchars() and combining it with decoding the .html() output sounds like a better approach, so that the HTML will contain things like < and the copied data will be <.

3. If you htmlspecialchars() before truncate_utf8(), you can end up with HTML entities cut in half, since as far as I see http://api.drupal.org/api/function/truncate_utf8/6 does not have any protection for HTML entities at all. So you can end up with a string like "foo bar baz &l", with the "t;" cut off.

4. Right, no objections :)

Thanks for your work. Let's fix these remaining problems and get the fixes in!

I should have noticed earlier that this patch has security implications. Anyway, it was reported independently as a security issue, so was unpublished for a while. The escaping part was fixed via http://drupal.org/node/434682 (SA-CONTRIB-2009-019).

Committed the special casing code for strings which are HTML-only. Expanded that to happen also in the JS and also the string limitation and tag stripping to work there as well. Committed that before the SA in http://drupal.org/cvs?commit=194594

The PHP code checking is left uncommitted, but I am totally not convinced that should happen in this module. I'd be more comfortable with moving to using drupal_alter() and letting other modules alter our strings with whatever criteria. I don't consider Views sane to export PHP code for translation, and that is not something we should work around in this module.

All this said, I consider this issue fixed. Anything else should have a new open issue.