Bug 37303 - Fuzzy translations displayed on the UI
Summary: Fuzzy translations displayed on the UI
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 37472
  Show dependency treegraph
 
Reported: 2024-07-10 10:21 UTC by Jonathan Druart
Modified: 2024-09-16 13:54 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.04
Circulation function:


Attachments
Bug 37303: Replace po2json with a JS version (9.78 KB, patch)
2024-07-10 10:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37303: Replace po2json with a JS version (10.63 KB, patch)
2024-07-10 12:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37303: Replace po2json with a JS version (10.68 KB, patch)
2024-07-25 04:19 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 37303: Replace po2json with a JS version (10.81 KB, patch)
2024-07-26 13:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 37303: Update yarn.lock after adding new dependency to packages.json (6.73 KB, patch)
2024-08-02 01:57 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-07-10 10:21:59 UTC
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.
Comment 1 Jonathan Druart 2024-07-10 10:36:27 UTC
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.
Comment 2 Jonathan Druart 2024-07-10 10:36:55 UTC
This seems bad, how didn't we catch that before?
Comment 3 Aude Charillon 2024-07-10 11:09:02 UTC
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!
Comment 4 Julian Maurice 2024-07-10 11:49:54 UTC
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 ...]
};
Comment 5 Julian Maurice 2024-07-10 11:55:02 UTC
Can't we add a "next if $po->fuzzy" in misc/translator/po2json ? It looks easier
Comment 6 Jonathan Druart 2024-07-10 12:04:27 UTC
(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.
Comment 7 Jonathan Druart 2024-07-10 12:13:51 UTC
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.
Comment 8 Jonathan Druart 2024-07-10 12:14:16 UTC
There is a deprecation warning that should be removed.
Comment 9 Julian Maurice 2024-07-10 12:31:25 UTC
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
Comment 10 Julian Maurice 2024-07-10 12:33:48 UTC
(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 :)
Comment 11 Victor Grousset/tuxayo 2024-07-25 04:19:42 UTC
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>
Comment 12 Victor Grousset/tuxayo 2024-07-25 04:56:57 UTC
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.
Comment 13 Martin Renvoize 2024-07-26 13:40:32 UTC
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>
Comment 14 Martin Renvoize 2024-07-26 13:40:54 UTC
Thanks for the work here guys.. all looking good from my side.  Passing QA
Comment 15 Martin Renvoize 2024-07-26 13:51:06 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 16 David Nind 2024-07-31 20:35:37 UTC
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.
Comment 17 Mason James 2024-08-02 01:57:15 UTC
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
Comment 18 Katrin Fischer 2024-08-05 13:33:22 UTC
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?
Comment 19 Lucas Gass 2024-08-29 16:53:39 UTC
Backported to 24.05.x for upcoming 24.05.04
Comment 20 Fridolin Somers 2024-09-16 13:54:25 UTC
Conflicts on backport to 23.11.x
Please provide rebased patches if possible.