Bug 35701 - Cannot use i18n.inc from memberentrygen
Summary: Cannot use i18n.inc from memberentrygen
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 35329
  Show dependency treegraph
 
Reported: 2024-01-04 14:53 UTC by Jonathan Druart
Modified: 2024-03-19 09:29 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.03,23.05.09


Attachments
Bug 35701: DO NOT PUSH - to recreate the problem (1.21 KB, patch)
2024-01-04 14:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35701: Do not use t (2.60 KB, patch)
2024-01-09 13:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35701: Do not use t (2.65 KB, patch)
2024-01-09 16:16 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35701: (follow-up) Fix last instance of t variable (1.59 KB, patch)
2024-01-09 16:16 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35701: Do not use t (2.72 KB, patch)
2024-01-17 11:10 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35701: (follow-up) Fix last instance of t variable (1.66 KB, patch)
2024-01-17 11:10 UTC, Martin Renvoize
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-01-04 14:53:35 UTC
Not sure what's going on, but I am experiencing something weird.

Add

        [% SET xxx = t("foo") %]
        console.log("[% xxx %]");

inside a js include

[% MACRO jsinclude BLOCK %]


And you will see "Ms", instead of "foo".
Comment 1 Jonathan Druart 2024-01-04 14:56:03 UTC
Created attachment 160535 [details] [review]
Bug 35701: DO NOT PUSH - to recreate the problem

Hit http://localhost:8081/cgi-bin/koha/members/memberentry.pl
open the console
See "Ms", while we expect "foo"
Comment 2 Julian Maurice 2024-01-04 15:02:14 UTC
There's a [% FOREACH t ... %] earlier in the same file. It probably overrides the t function.
Comment 3 Jonathan Druart 2024-01-04 15:20:01 UTC
erk good catch, I searched for "t = "!
Comment 4 Jonathan Druart 2024-01-04 15:23:34 UTC
Thanks!
Comment 5 Jonathan Druart 2024-01-09 13:57:17 UTC
Created attachment 160693 [details] [review]
Bug 35701: Do not use t

To not interfer with the t from I18N

On bug 35329 we are using the I18N TT plugin that is providing the 't'
subroutine.

This loop is redefining the subroutine, therefore it is not longer
accessible.

Test plan:
Edit a patron and confirm the the "Title" dropdown still contain the
different values of syspref BorrowersTitles.
Comment 6 Julian Maurice 2024-01-09 15:10:55 UTC
There is one [% t | html %] left (under ELSE)
Comment 7 Owen Leonard 2024-01-09 16:16:56 UTC
Created attachment 160708 [details] [review]
Bug 35701: Do not use t

To not interfer with the t from I18N

On bug 35329 we are using the I18N TT plugin that is providing the 't'
subroutine.

This loop is redefining the subroutine, therefore it is not longer
accessible.

Test plan:
Edit a patron and confirm the the "Title" dropdown still contain the
different values of syspref BorrowersTitles.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Owen Leonard 2024-01-09 16:16:58 UTC
Created attachment 160709 [details] [review]
Bug 35701: (follow-up) Fix last instance of t variable

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 David Cook 2024-01-10 03:18:50 UTC
Variable scope in template toolkit definitely leaves a lot to be desired. I've bumped into similar issues with MACROs in email templates as well..
Comment 10 David Cook 2024-01-10 03:19:45 UTC
Should probably change this bug title although I don't have a good title immediately in mind...
Comment 11 Martin Renvoize 2024-01-17 11:10:40 UTC
Created attachment 161094 [details] [review]
Bug 35701: Do not use t

To not interfer with the t from I18N

On bug 35329 we are using the I18N TT plugin that is providing the 't'
subroutine.

This loop is redefining the subroutine, therefore it is not longer
accessible.

Test plan:
Edit a patron and confirm the the "Title" dropdown still contain the
different values of syspref BorrowersTitles.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2024-01-17 11:10:43 UTC
Created attachment 161095 [details] [review]
Bug 35701: (follow-up) Fix last instance of t variable

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2024-01-17 11:11:05 UTC
Scoping fix, all working now.

Passing QA
Comment 14 Katrin Fischer 2024-01-26 14:18:02 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 15 Fridolin Somers 2024-02-01 13:23:30 UTC
Pushed to 23.11.x for 23.11.03
Comment 16 Lucas Gass 2024-02-09 20:43:30 UTC
Backported to 23.05.x for upcoming 23.05.09