New source string is created if original source has ampersand
| Project: | Localization client |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | reviewed & tested by the community |
If a source string has an ampersand in it, then when the translated string is saved, it creates a new source string with the translation, and the original source string is left untranslated. See this video demonstration of the bug for more details.
It also occurs with double forward slash characters, such as in http://. The new source string has the ampersand replaced with a %20, and the second forward slash is replaced with a %2F.
I guess the l10n_client client-side javascript is url-encoding source language string when it sends it server-side, but the server is not url-un-encoding it. I'm surprised this results in a new set of language strings though.
The most obvious solution is to url-en-encode the source string server-side. However a better solution may be to use POST instead of GET to send the new translation server-side. That way this issue goes away and the client-server communication can scale to large strings with complicated content.
Thoughts?
#1
I think this is unrelated to #304494: Fix for copy source, other HTML & htmlentities in string related issues and #218516: Translatable entries not HTML encoded since it was an issue in DRUPAL-6--1-7 too, and they describe a different issue.
#2
#3
#4
This issues exists if the source string has a trailing new line character. I didn't test with new line characters at beginning or in middle, or other sorts of leading or trailing whitespace. It may or may not have been introduced by the changes in this patch.
#5
Hm, you see we already use POST. GET and POST have no difference in terms of the escaping required or performed. You also see, that by modifying client side encoding, you already fix the issue, so the problem is not on server side.
This same issue was submitted before at #298498: Drupal assumes mod_rewrite bugs which are not there and as you can see, it was turned to a Drupal core issue (of Drupal.encodeURIComponent()). We could maybe not use that function, but the reason that function exists is that Drupal deems it required for some reason, so your version of the code might break on certain Apache configs. See that issue.
(Tempted to marking as duplicate, but we do need a solution for this module if core does not solve it in itself).
#6
Oh goodness. I knew I shouldn't have opened this can of worms. Sigh!!
;)
#7
Ok #298498: Drupal assumes mod_rewrite bugs which are not there was marked a duplicate of #310139: drupal_query_string_encode() should not call drupal_urlencode(), which says we should not use Drupal.encodeURIComponent() for things in the URL other then the path, and that system.module has a bug that it does not conform to this rule. So all-in-all, your patch looks like going into the right direction. It cleans up code, and does the escaping via jQuery. Marking RTBC.
#8
Yaya! :)
#9
Bump. Can this get in?