Bug 36171 - Incorrect translation of Template Toolkit directive causes patron view error in ru-RU
Summary: Incorrect translation of Template Toolkit directive causes patron view error ...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Kevin Carnes
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-27 09:43 UTC by sergeinik
Modified: 2024-08-21 11:20 UTC (History)
7 users (show)

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


Attachments
Russian interface (31.66 KB, image/png)
2024-02-27 09:43 UTC, sergeinik
Details
Bug 36171: Use FILTER collapse instead of tilde in address includes (82.04 KB, patch)
2024-02-27 17:38 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 36171: Incorrect translation of Template Toolkit directive causes patron view error in ru-RU (5.02 KB, patch)
2024-04-25 09:30 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 36171: Fix template toolkit tags split by HTML parser (1.33 KB, patch)
2024-08-09 13:58 UTC, Kevin Carnes
Details | Diff | Splinter Review
Template toolkit tags repaired (7.56 KB, text/plain)
2024-08-09 13:58 UTC, Kevin Carnes
Details
Bug 36171: Fix template toolkit tags split by HTML parser (7.17 KB, text/plain)
2024-08-12 09:23 UTC, Kevin Carnes
Details
Bug 36171: Fix template toolkit tags split by HTML parser (7.04 KB, patch)
2024-08-13 11:48 UTC, Kevin Carnes
Details | Diff | Splinter Review
Bug 36171: Fix template toolkit tags split by HTML parser (5.40 KB, patch)
2024-08-15 09:10 UTC, Kevin Carnes
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description sergeinik 2024-02-27 09:43:49 UTC
Created attachment 162490 [details]
Russian interface

After the last update of the program, the display of user pages, as well as the interface for writing off and adding literature, disappeared in the Russian administrator interface.

Everything works in the English interface.
Comment 1 Owen Leonard 2024-02-27 12:22:50 UTC
Could you provide instructions for how to reproduce the problem?
Comment 2 sergeinik 2024-02-27 13:49:46 UTC
You need to switch the interface language to Russian and open the card of any user.
Comment 3 Owen Leonard 2024-02-27 16:32:59 UTC
Template process failed: file error - parse error - member-display-address-style.inc line 1: unexpected token (line_break)
  [% КОМПЛЕКТ line_break = "<br/>" %] at /kohadevbox/koha/C4/Templates.pm line 134

It looks like a template directive is being picked up for translation. This is the line from the English language template:

[%~ SET line_break = "<br/>" ~%]

A temporary fix would be to edit the affected file, member-display-address-style.inc, to replace "КОМПЛЕКТ" with "SET"

It looks like the same problem is in display-library-address.inc.

The real fix will require that we figure out why the translation script is picking that line up.
Comment 4 Owen Leonard 2024-02-27 16:48:43 UTC
(In reply to Owen Leonard from comment #3)
> It looks like the same problem is in display-library-address.inc.

I should have pointed out that this include is in the OPAC, not the staff interface.
Comment 5 Owen Leonard 2024-02-27 17:38:28 UTC Comment hidden (obsolete)
Comment 6 sergeinik 2024-02-28 06:30:13 UTC
The patch works. But, as soon as I update the language according to the last point, the problem returns. This happened after the last update.
Comment 7 sergeinik 2024-04-04 08:27:44 UTC
When will this error be fixed in the next update?
Comment 8 Owen Leonard 2024-04-04 15:30:46 UTC
I'm putting this back to "NEW" because I've been unable to find a template-level fix for it.
Comment 9 Katrin Fischer 2024-04-04 20:14:55 UTC
(In reply to sergeinik from comment #7)
> When will this error be fixed in the next update?

This bug was trying to find a solution so that these text no longer shows up in translations. A temporary fix is that someone with translation permissions for Russian fixes these:

https://translate.koha-community.org/search/koha/-/ru/?q=+source%3ASET+&sort_by=-priority%2Cposition&checksum=
Comment 10 Katrin Fischer 2024-04-06 13:28:53 UTC
I've fixed the broken translation with Jonathan's help.
Comment 11 Kevin Carnes 2024-04-25 09:30:49 UTC
Created attachment 165506 [details] [review]
Bug 36171: Incorrect translation of Template Toolkit directive causes patron view error in ru-RU

It appears that the template parser parses HTML tags first and then template
toolkit tags. This can cause a template toolkit tag that contains an HTML tag
to have its start and end markers separated. This causes it to be treated as
text that can be translated instead of a template toolkit tag.

The template toolkit tags should probably be parsed first, but as a workaround
the HTML tags in a template toolkit tag can be obfuscated. This patch uses
concatenation as an example.

To test:
1. You will need to edit the staff-prog.po for a language and modify the
   the translation for "SET line_break". You can change SET to FOO.
2. Install the translation for the language.
3. View the details of a patron in the staff interface with the language.
4. The page will have an error.
5. Apply the patch.
6. Install the translation for the language.
7. View the details of a patron in the staff interface with the language.
8. The page should now display correctly.

If you want to, you can run "gulp po:update --lang LANGUAGE-CODE" to make sure
that the template tag is no longer included in .po file.
Comment 12 Owen Leonard 2024-06-21 15:33:41 UTC
I think this is supposed to be "Needs signoff" ?
Comment 13 Kevin Carnes 2024-06-25 14:22:53 UTC
It is just a workaround, so I don't know if it is ready to be signed off. Is it enough or should the method for parsing be discussed?
Comment 14 Jonathan Druart 2024-07-11 13:04:25 UTC
This feels hacky. IMO we should fix it properly (ie. not pick those strings for translation).
Comment 15 David Nind 2024-07-12 19:50:33 UTC
A couple of questions:

1. Does this only affect ru-RU, or all languages?

2. Is the status still "Critical"?
Comment 16 Victor Grousset/tuxayo 2024-07-23 04:36:33 UTC
> 1. Does this only affect ru-RU, or all languages?

It should affect all languages where the problematic strings have been translated.

Here is what I've been able to find
https://translate.koha-community.org/translate/koha/23-11/opac-bootstrap/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3ASET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-11/opac-bootstrap/te/?checksum=28bf1f208fcb60c4&q=+source%3ASET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/main/opac-bootstrap/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3ASET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/main/opac-bootstrap/te/?checksum=28bf1f208fcb60c4&q=+source%3ASET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/main/staff-prog/te/?checksum=28bf1f208fcb60c4&q=+source%3ASET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/24-05/opac-bootstrap/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/opac-bootstrap/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/opac-bootstrap/te/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/22-11/opac-bootstrap/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/22-11/staff-prog/te/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/staff-prog/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/staff-prog/te/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-11/staff-prog/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-11/staff-prog/te/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/24-05/staff-prog/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/main/staff-prog/fa_ARAB/?checksum=28bf1f208fcb60c4&q=+source%3A%25+SET+&sort_by=-priority%2Cposition

https://translate.koha-community.org/translate/koha/22-11/staff-prog/ca/?checksum=47311df64b1b946c&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/22-11/staff-prog/fa_ARAB/?checksum=47311df64b1b946c&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/22-11/staff-prog/te/?checksum=47311df64b1b946c&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/staff-prog/ca/?checksum=47311df64b1b946c&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/staff-prog/fa_ARAB/?checksum=47311df64b1b946c&q=+source%3A%25+SET+&sort_by=-priority%2Cposition
https://translate.koha-community.org/translate/koha/23-05/staff-prog/te/?checksum=47311df64b1b946c&q=+source%3A%25+SET+&sort_by=-priority%2Cposition


> 2. Is the status still "Critical"?

It seems fair until someone with rights on these languages/all languages empties the translation or copy paste the source. Maybe the 2nd one is best?
That's the 1st group of strings. 28bf1f208fcb60c4

The 2nd group (47311df64b1b946c) needs only to have the variable name fixed.

And then for digging at the root cause could be major severity I guess.

----

(In reply to Jonathan Druart from comment #14)
> This feels hacky. IMO we should fix it properly (ie. not pick those strings
> for translation).

+1 to not pick or eliminate the uses of these weird conditions to insert line breaks and generate links

1)
If not picking those strings, it must be kept in mind that the second group of strings with breaking translations (47311df64b1b946c) is needed to be translated:
[%% SET countries = [ "Afghanistan", "Albania",

Thankfully it's not needed anymore since 23.11 thanks to Bug 35091 :D
So if someone manages to exclude all "SET" TT tags it could be applied to main, 24.05, 23.11 and either we do not bother with older releases (just fix the existing error in the links) or apply the current patch there.

2) If eliminating the structure are get picked up:
about display-library-address.inc and member-display-address-style.inc
Is that even needed? It might be dead code.
There are two [% PROCESS 'display-library-address' %]
And I don't see how the no_line_break variable would be set.

About pref_MARCOverlayRules_link it's from bug 29589 and likely pref_CurbsidePickup_link is modeled after it. bug 29589 was already to fix a translatability issue ^^"
Since then, is there know another trick known that could superseed the solution of bug 29589 ?

But maybe we are not seeing any translation error on pref_MARCOverlayRules_link and pref_CurbsidePickup_link because the final are scary enough that no one thinks it's really something to translate ^^
%s %s [%% SET pref_MARCOverlayRules_link = '
%s [%% SET pref_CurbsidePickup_link = '
So maybe we can get away with these link generation cases?
Comment 17 Victor Grousset/tuxayo 2024-08-01 20:46:28 UTC
(In reply to Victor Grousset/tuxayo from comment #16)
> > 2. Is the status still "Critical"?
> 
> It seems fair until someone with rights on these languages/all languages
> empties the translation or copy paste the source. Maybe the 2nd one is best?
> That's the 1st group of strings. 28bf1f208fcb60c4
> 
> The 2nd group (47311df64b1b946c) needs only to have the variable name fixed.

So I made suggestions to string found broken, they just need to be accepted by someone with rights on all languages or just rights on either Telugu, Persian and Catalan.


-----


> But maybe we are not seeing any translation error on
> pref_MARCOverlayRules_link and pref_CurbsidePickup_link because the final
> are scary enough that no one thinks it's really something to translate ^^

*the strings are scary enough
Comment 18 Katrin Fischer 2024-08-02 14:24:14 UTC
I've accepted all your suggestions.
Comment 19 Kevin Carnes 2024-08-09 13:58:11 UTC
Created attachment 170194 [details] [review]
Bug 36171: Fix template toolkit tags split by HTML parser
Comment 20 Kevin Carnes 2024-08-09 13:58:53 UTC
Created attachment 170195 [details]
Template toolkit tags repaired
Comment 21 Kevin Carnes 2024-08-09 14:11:12 UTC
Here is an attempt at repairing split template toolkit tags. If a text block begins with the end of a tag, it goes back through the tokens to try to find the beginning and combine them into a directive token.

I also included a list of the "repairs". It looks like the action block for the virtual shelves results would need to use I18N.t for translation and doc-head-open would need to use Dumper.dump_html instead of filtering (the '-->' is confusing the parser).

If this is not reasonable, I can limit it to SET tags.

Another option is to slurp the file, remove the tags, and then use $self->parse instead of $self->parse_file.

I would appreciate any feedback and testing since this was done on a Friday afternoon.
Comment 22 Kevin Carnes 2024-08-12 09:23:42 UTC
Created attachment 170227 [details]
Bug 36171: Fix template toolkit tags split by HTML parser

Bug 36171: Fix template toolkit tags split by HTML parser

It appears that the template parser parses HTML tags first and then template
toolkit tags. This can cause a template toolkit tag that contains an HTML tag
to have its start and end markers separated. This causes it to be treated as
text that can be translated instead of a template toolkit tag.

This patch searches for text sections that have a template toolkit end marker
without a start and then searches for the start in previous sections. It then
creates a new template toolkit section with the start and end markers.

It also changes the replacement of "-->" in doc-head-open.inc so it is not
interpreted as the end of the comment. It now replaces all "--" with "- -".

The action buttons in the staff Lists view have also been changed to allow
translation.

To test:
1. You will need to edit the staff-prog.po for a language and modify the
   the translation for "SET line_break". You can change SET to FOO.
2. Install the translation for the language.
3. View the details of a patron in the staff interface with the language.
4. The page will have an error.
5. Apply the patch.
6. Install the translation for the language.
7. View the details of a patron in the staff interface with the language.
8. The page should now display correctly.

If you want to, you can run "gulp po:update --lang LANGUAGE-CODE" to make sure
that the template tag is no longer included in .po file.

You can also add translations for "Edit", "Delete", and "Transfer" in
messages.po to make sure that the action buttons are translated in the Lists
view.
Comment 23 Kevin Carnes 2024-08-13 11:48:56 UTC
Created attachment 170262 [details] [review]
Bug 36171: Fix template toolkit tags split by HTML parser

It appears that the template parser parses HTML tags first and then template
toolkit tags. This can cause a template toolkit tag that contains an HTML tag
to have its start and end markers separated. This causes it to be treated as
text that can be translated instead of a template toolkit tag.

This patch searches for text sections that have a template toolkit end marker
without a start and then searches for the start in previous sections. It then
creates a new template toolkit section with the start and end markers.

It also changes the replacement of "-->" in doc-head-open.inc so it is not
interpreted as the end of the comment. It now replaces all "--" with "- -".

The action buttons in the staff Lists view have also been changed to allow
translation.

To test:
1. You will need to edit the staff-prog.po for a language and modify the
   the translation for "SET line_break". You can change SET to FOO.
2. Install the translation for the language.
3. View the details of a patron in the staff interface with the language.
4. The page will have an error.
5. Apply the patch.
6. Install the translation for the language.
7. View the details of a patron in the staff interface with the language.
8. The page should now display correctly.

If you want to, you can run "gulp po:update --lang LANGUAGE-CODE" to make sure
that the template tag is no longer included in .po file.

You can also add translations for "Edit", "Delete", and "Transfer" in
messages.po to make sure that the action buttons are translated in the Lists
view.
Comment 25 Katrin Fischer 2024-08-14 11:20:29 UTC
Thanks for the links, I had only been looking for the line_break one. Should be good now.
Comment 26 Jonathan Druart 2024-08-14 15:17:10 UTC
(In reply to Kevin Carnes from comment #23)
> It also changes the replacement of "-->" in doc-head-open.inc so it is not
> interpreted as the end of the comment. It now replaces all "--" with "- -".

Why all?
Comment 27 Kevin Carnes 2024-08-15 09:10:16 UTC
Created attachment 170342 [details] [review]
Bug 36171: Fix template toolkit tags split by HTML parser

It appears that the template parser parses HTML tags first and then template
toolkit tags. This can cause a template toolkit tag that contains an HTML tag
to have its start and end markers separated. This causes it to be treated as
text that can be translated instead of a template toolkit tag.

This patch searches for text sections that have a template toolkit end marker
without a start and then searches for the start in previous sections. It then
creates a new template toolkit section with the start and end markers.

The action buttons in the staff Lists view have been changed to allow
translation.

To test:
1. You will need to edit the staff-prog.po for a language and modify the
   the translation for "SET line_break". You can change SET to FOO.
2. Install the translation for the language.
3. View the details of a patron in the staff interface with the language.
4. The page will have an error.
5. Apply the patch.
6. Install the translation for the language.
7. View the details of a patron in the staff interface with the language.
8. The page should now display correctly.

If you want to, you can run "gulp po:update --lang LANGUAGE-CODE" to make sure
that the template tag is no longer included in .po file.

You can also add translations for "Edit", "Delete", and "Transfer" in
messages.po to make sure that the action buttons are translated in the Lists
view.
Comment 28 Kevin Carnes 2024-08-15 10:06:05 UTC
(In reply to Jonathan Druart from comment #26)
> (In reply to Kevin Carnes from comment #23)
> > It also changes the replacement of "-->" in doc-head-open.inc so it is not
> > interpreted as the end of the comment. It now replaces all "--" with "- -".
> 
> Why all?

Just in case there was a browser that was strict about an even number of "--" and also to avoid writing '-' _ '->'.

I removed the change since it looks like everything between a comment and an HTML tag is ignored anyways, so the change is not necessary.

In this case it doesn't cause a problem, but there is a problem if a template toolkit tag is outside a comment and contains a "<!".

There is also a problem if it is inside a comment and contains a "-->" followed by an HTML tag and then some text before the real end of the comment.

Finally, any text between an HTML tag and a comment is ignored (e.g. <a>before<!-- comment -->after</a>).