We have our own "quote_po" sub that is stating that Locale::PO::quote is buggy because it does not deal with quoting new lines correctly. However it seems that it is fixed now. Ideally we could use Locale::PO::quote everywhere, but it does not escape tab characters: $string =~ s/\\(?!t)/\\\\/g; # \t is a tab This means the following: msgid "Tabulation (\\t)" msgstr "Tabulation (\\t)" become: -msgid "Tabulation (\t)" -msgstr "Tabulation (\t)" And we are seeing the following on Weblate: https://snipboard.io/BjQmDC.jpg Note that Locale::PO has not been updated since 2014...
Created attachment 156937 [details] [review] Bug 35043: Use Locale::PO->quote We have our own "quote_po" sub that is stating that Locale::PO::quote is buggy because it does not deal with quoting new lines correctly. However it seems that it is fixed now. Ideally we could use Locale::PO::quote everywhere, but it does not escape tab characters: $string =~ s/\\(?!t)/\\\\/g; # \t is a tab This means the following: msgid "Tabulation (\\t)" msgstr "Tabulation (\\t)" become: -msgid "Tabulation (\t)" -msgstr "Tabulation (\t)" And we are seeing the following on Weblate: https://snipboard.io/BjQmDC.jpg Note that Locale::PO has not been updated since 2014... The real problem behind this is that we have 2 methods to quote strings. At first glance it seems that Locale::PO::quote was not used before, but with the introduction of the koha-i18n project we will have scripts that will use Locale::PO->save_file_fromarray, which uses Locale::PO->quote => Those scripts will be used on the translation server for post processing (security reason, marking potential XSS strings as fuzzy). Test plan: 0. Do not apply the patch 1. gulp po:update --lang LANG # Replace LANG with your favorite language code 2. git commit -a -m"init PO files" 3. Apply this patch 4. Repeate 1. 5. git diff => The change is about the "Tabulation" and "New line" strings from tools/csv-profiles.tt 6. Translate them (replace the \t and \n with %s) and remove the fuzzy flag 7. install the template: cd misc/translator && perl translate install LANG 8. Enable the language, use it and go to the "Nouveau profil CSV" view => Notice that the \t and \n are correctly displayed.
Created attachment 156938 [details] [review] Bug 35043: Do not have \n or \t appear in PO files This is a trick, so that we won't have to deal with \t in PO files. \n is not a problem, but it seems consistent to use the same method for both.
Created attachment 156950 [details] [review] Bug 35043: Use Locale::PO->quote We have our own "quote_po" sub that is stating that Locale::PO::quote is buggy because it does not deal with quoting new lines correctly. However it seems that it is fixed now. Ideally we could use Locale::PO::quote everywhere, but it does not escape tab characters: $string =~ s/\\(?!t)/\\\\/g; # \t is a tab This means the following: msgid "Tabulation (\\t)" msgstr "Tabulation (\\t)" become: -msgid "Tabulation (\t)" -msgstr "Tabulation (\t)" And we are seeing the following on Weblate: https://snipboard.io/BjQmDC.jpg Note that Locale::PO has not been updated since 2014... The real problem behind this is that we have 2 methods to quote strings. At first glance it seems that Locale::PO::quote was not used before, but with the introduction of the koha-i18n project we will have scripts that will use Locale::PO->save_file_fromarray, which uses Locale::PO->quote => Those scripts will be used on the translation server for post processing (security reason, marking potential XSS strings as fuzzy). Test plan: 0. Do not apply the patch 1. gulp po:update --lang LANG # Replace LANG with your favorite language code 2. git commit -a -m"init PO files" 3. Apply this patch 4. Repeate 1. 5. git diff => The change is about the "Tabulation" and "New line" strings from tools/csv-profiles.tt 6. Translate them (replace the \t and \n with %s) and remove the fuzzy flag 7. install the template: cd misc/translator && perl translate install LANG 8. Enable the language, use it and go to the "Nouveau profil CSV" view => Notice that the \t and \n are correctly displayed. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 156951 [details] [review] Bug 35043: Do not have \n or \t appear in PO files This is a trick, so that we won't have to deal with \t in PO files. \n is not a problem, but it seems consistent to use the same method for both. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
(In reply to Jonathan Druart from comment #1) > However it seems > that > it is fixed now. Seems? My version on D11: our $VERSION = '0.27'; Do we know at which point this was solved?
Created attachment 157922 [details] [review] Bug 35043: Use Locale::PO->quote We have our own "quote_po" sub that is stating that Locale::PO::quote is buggy because it does not deal with quoting new lines correctly. However it seems that it is fixed now. Ideally we could use Locale::PO::quote everywhere, but it does not escape tab characters: $string =~ s/\\(?!t)/\\\\/g; # \t is a tab This means the following: msgid "Tabulation (\\t)" msgstr "Tabulation (\\t)" become: -msgid "Tabulation (\t)" -msgstr "Tabulation (\t)" And we are seeing the following on Weblate: https://snipboard.io/BjQmDC.jpg Note that Locale::PO has not been updated since 2014... The real problem behind this is that we have 2 methods to quote strings. At first glance it seems that Locale::PO::quote was not used before, but with the introduction of the koha-i18n project we will have scripts that will use Locale::PO->save_file_fromarray, which uses Locale::PO->quote => Those scripts will be used on the translation server for post processing (security reason, marking potential XSS strings as fuzzy). Test plan: 0. Do not apply the patch 1. gulp po:update --lang LANG # Replace LANG with your favorite language code 2. git commit -a -m"init PO files" 3. Apply this patch 4. Repeate 1. 5. git diff => The change is about the "Tabulation" and "New line" strings from tools/csv-profiles.tt 6. Translate them (replace the \t and \n with %s) and remove the fuzzy flag 7. install the template: cd misc/translator && perl translate install LANG 8. Enable the language, use it and go to the "Nouveau profil CSV" view => Notice that the \t and \n are correctly displayed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 157923 [details] [review] Bug 35043: Do not have \n or \t appear in PO files This is a trick, so that we won't have to deal with \t in PO files. \n is not a problem, but it seems consistent to use the same method for both. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #5) > (In reply to Jonathan Druart from comment #1) > > However it seems > > that > > it is fixed now. > > Seems? > My version on D11: > our $VERSION = '0.27'; > > Do we know at which point this was solved? Well, if this is 2014, lets assume we are safe.
Pushed to master for 23.11. Nice work everyone, thanks!
Depends on Bug 34959 not yet in 23.05.x