When using __() (ie. Gettext.js) we are seeing the translations that are marked as fuzzy. This is definitely not the expected behaviour. It happens because (our version of) po2json are old and no longer maintained, and just embed them.
Created attachment 168712 [details] [review] Bug 37303: Replace po2json with a JS version When using __() (ie. Gettext.js) we are seeing the translations that are marked as fuzzy. This is definitely not the expected behaviour. It happens because (our version of) po2json are old and no longer maintained, and just embed them. It seems that the bin we have has been upgraded to a JS version (different authors). Test plan: (replace LANG with your language code) 0. Do not apply this patch Edit misc/translator/po/LANG-messages-js.po Mark a string as fuzzy Edit ./intranet-main.tt and add the following lines inside $(document).ready console.log(_("Your string")); console.log(__("Your string")); Replace "Your string" with the string you are actually testing. Update the templates: `koha-translate --update LANG --dev kohadev && restart_all` Go to the Koha home page, open the console. => Notice that the second log in the console is displaying the fuzzy string. 1. Apply this patch Install the new version of po2json using `yarn install` Repeat the previous steps. => With this patch applied both logs show the English version of the string. Remove fuzzy, update the templates and try again.
This seems bad, how didn't we catch that before?
I guess it only affects languages where not everything is translated - either due to lack of translators or because it's English to English (so no translation needed, in most cases). Speaking for the en-GB translation crew, we simply assumed it was our fault; that we'd messed up our file at some point in the past!
Patch does not work for me. po2json expects 2 arguments (<input> <output>) and we only give one. So the resulting locale_data.json looks like this: var json_locale_data = {"Koha":yarn run v1.22.20 >>$ /home/koha/koha/node_modules/.bin/po2json /home/koha/koha/misc/translator/po/fr-FR-messages-js.po >>output argument is required >>Usage: po2json <input> <output> [options] [... full of usage ...] };
Can't we add a "next if $po->fuzzy" in misc/translator/po2json ? It looks easier
(In reply to Julian Maurice from comment #5) > Can't we add a "next if $po->fuzzy" in misc/translator/po2json ? It looks > easier Maybe, but I thought it was better to rely on a maintained library and remove the binary we embedded.
Created attachment 168713 [details] [review] Bug 37303: Replace po2json with a JS version When using __() (ie. Gettext.js) we are seeing the translations that are marked as fuzzy. This is definitely not the expected behaviour. It happens because (our version of) po2json are old and no longer maintained, and just embed them. It seems that the bin we have has been upgraded to a JS version (different authors). Test plan: (replace LANG with your language code) 0. Do not apply this patch Edit misc/translator/po/LANG-messages-js.po Mark a string as fuzzy Edit ./intranet-main.tt and add the following lines inside $(document).ready console.log(_("Your string")); console.log(__("Your string")); Replace "Your string" with the string you are actually testing. Update the templates: `koha-translate --update LANG --dev kohadev && restart_all` Go to the Koha home page, open the console. => Notice that the second log in the console is displaying the fuzzy string. 1. Apply this patch Install the new version of po2json using `yarn install` Repeat the previous steps. => With this patch applied both logs show the English version of the string. Remove fuzzy, update the templates and try again.
There is a deprecation warning that should be removed.
I compared locale_data.js before and after the patch: with the patch: * keys are not sorted (does not matter) * header keys are lowercase (should not matter but needs to be tested) * header values are trimmed (good) Other than that they are identical
(In reply to Julian Maurice from comment #9) > Other than that they are identical And other than the fact that fuzzy messages are not included, obviously :)
Created attachment 169536 [details] [review] Bug 37303: Replace po2json with a JS version When using __() (ie. Gettext.js) we are seeing the translations that are marked as fuzzy. This is definitely not the expected behaviour. It happens because (our version of) po2json are old and no longer maintained, and just embed them. It seems that the bin we have has been upgraded to a JS version (different authors). Test plan: (replace LANG with your language code) 0. Do not apply this patch Edit misc/translator/po/LANG-messages-js.po Mark a string as fuzzy Edit ./intranet-main.tt and add the following lines inside $(document).ready console.log(_("Your string")); console.log(__("Your string")); Replace "Your string" with the string you are actually testing. Update the templates: `koha-translate --update LANG --dev kohadev && restart_all` Go to the Koha home page, open the console. => Notice that the second log in the console is displaying the fuzzy string. 1. Apply this patch Install the new version of po2json using `yarn install` Repeat the previous steps. => With this patch applied both logs show the English version of the string. Remove fuzzy, update the templates and try again. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! :) ---- commit: > It happens because (our version of) po2json are old and no longer maintained, > and just embed them. (In reply to Jonathan Druart from comment #6) > (In reply to Julian Maurice from comment #5) > > Can't we add a "next if $po->fuzzy" in misc/translator/po2json ? It looks > > easier > > Maybe, but I thought it was better to rely on a maintained library and > remove the binary we embedded. About that... The latest version is from 2016 ^^" https://github.com/mikeedwards/po2json/tags And the dates of the alpha version suggest the efforts for the next version have stalled :( That causes worries about the dependencies of po2json https://github.com/mikeedwards/po2json/issues/101 That nomnom dependency was last published 10 years ago and is officially abandoned: https://www.npmjs.com/package/nomnom (still 424 180 Weekly Downloads [U+1F480]) And it depends on underscore 1.6.0 which has the security vulnerability mentioned in the po2json ticket. But nomnom seems to be just to parse command line arguments for po2json so it should be okay. Though it shows the need to look at the rest of the dependency tree. That would be bad if malicious translations could be used to remotely execute code at the packaging step. Or on devs machines. Though that's still a worry with the perl po2json. It uses a 2014 version of Locale::PO to read the .po files and a 2011 version of JSON to generate JSON.
Created attachment 169718 [details] [review] Bug 37303: Replace po2json with a JS version When using __() (ie. Gettext.js) we are seeing the translations that are marked as fuzzy. This is definitely not the expected behaviour. It happens because (our version of) po2json are old and no longer maintained, and just embed them. It seems that the bin we have has been upgraded to a JS version (different authors). Test plan: (replace LANG with your language code) 0. Do not apply this patch Edit misc/translator/po/LANG-messages-js.po Mark a string as fuzzy Edit ./intranet-main.tt and add the following lines inside $(document).ready console.log(_("Your string")); console.log(__("Your string")); Replace "Your string" with the string you are actually testing. Update the templates: `koha-translate --update LANG --dev kohadev && restart_all` Go to the Koha home page, open the console. => Notice that the second log in the console is displaying the fuzzy string. 1. Apply this patch Install the new version of po2json using `yarn install` Repeat the previous steps. => With this patch applied both logs show the English version of the string. Remove fuzzy, update the templates and try again. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks for the work here guys.. all looking good from my side. Passing QA
Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant
I'm not sure if this error message is related to this bug when trying to install a language in KTD: koha-translate --install de-DE --dev kohadev error Command "po2json" not found. read_file '/tmp/de-DE-messages.po' - open: No such file or directory at /kohadevbox/koha/misc/translator/LangInstaller.pm line 461. (Assuming that this is still the correct way to just install a language for testing, not update or test any other language changes.) This is on the current main, with KTD all up to date (git pull and ktd pull), and po on main and up to date as well.
Created attachment 169975 [details] [review] Bug 37303: Update yarn.lock after adding new dependency to packages.json use ./xt/verify-yarnlock.t test in BZ 37490 to confirm
I have pushed Mason's follow-up to main. I think there was some discussion about using a different module/dependency maybe, but felt that would fix things for now while we dig deeper - maybe on a separate bug?
Backported to 24.05.x for upcoming 24.05.04
Conflicts on backport to 23.11.x Please provide rebased patches if possible.