Bug 21156 adds 2 js files (Gettext.js, i18n.js) + 1 generated js file (locale_data.js). On master, their URL are broken and the files are not loaded. Steps to reproduce: 1. Checkout master 2. Open Firefox/Chrome devtools, network tab, and go to any page in the staff interface (except the main page) 3. You should see 3 404 errors for the files mentioned above.
Created attachment 98913 [details] [review] Bug 24661: Fix inclusion of locale-related javascript files Bug 21156 adds 2 js files (Gettext.js, i18n.js) + 1 generated js file (locale_data.js). Their URL are broken and the files are not loaded. Use Template::Toolkit plugin Asset to include these files, and exclude locale_data.js when language is english, because obviously translations are not needed in this case. Test plan: 1. Just make sure the files mentioned above are loaded correctly when in English and in other languages as well
Created attachment 98930 [details] [review] Bug 24661: Fix inclusion of locale-related javascript files 2 js files (Gettext.js, i18n.js) + 1 generated js file (locale_data.js) were added by bug 21156. Their URL are broken and the files are not loaded. Use Template::Toolkit plugin Asset to include these files, and exclude locale_data.js when language is english, because obviously translations are not needed in this case. Test plan: 1. Just make sure the files mentioned above are loaded correctly when in English and in other languages as well Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks for the quick fix Julian.. works as expected.. going for a fast QA on this one!
Nice work everyone! Pushed to master for 20.05
(In reply to Martin Renvoize from comment #4) > Nice work everyone! > > Pushed to master for 20.05 Too fast! I was about to sign this :) Anyway, I found the following, not related with this bug, that pops doing a language update (also create and install) to do the testing: koha-tmpl/intranet-tmpl/prog/js/checkouts.js:569: warning: unterminated string koha-tmpl/intranet-tmpl/prog/js/checkouts.js:858: warning: unterminated string koha-tmpl/intranet-tmpl/prog/js/checkouts.js:904: warning: unterminated string koha-tmpl/intranet-tmpl/prog/js/checkouts.js:911: warning: unterminated string koha-tmpl/intranet-tmpl/prog/js/checkouts.js:1095: warning: RegExp literal terminated too early
It looks like xgettext does not like ES6 template literals. https://savannah.gnu.org/bugs/?50920 From what I understand, support for template literals was added in gettext 0.20 (still not packaged in debian) but is still buggy in latest released version 0.20.1 and a fix is present in master.
(In reply to Julian Maurice from comment #6) > It looks like xgettext does not like ES6 template literals. > > https://savannah.gnu.org/bugs/?50920 > > From what I understand, support for template literals was added in gettext > 0.20 (still not packaged in debian) but is still buggy in latest released > version 0.20.1 and a fix is present in master. Then we have a little problem with that. Current (19.10/18.04) ubuntu has 0.19.8.1
(In reply to Bernardo Gonzalez Kriegel from comment #7) > Then we have a little problem with that. Current (19.10/18.04) ubuntu has > 0.19.8.1 Yes, same version in Debian unstable, so we're stuck with this version for some time. One possible workaround is to declare translatable strings as soon as possible in the file (before the first template literal), since xgettext parse these strings correctly.
Warning from qa tools: File not found : /js/locale_data.js at /usr/share/koha/Koha/Template/Plugin/Asset.pm line 84. koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc: [% Asset.js(lang _ '/js/locale_data.js') | $raw %] Cannot find this file. Not sure if this construction works in template toolkit btw.
(In reply to Marcel de Rooy from comment #9) > Warning from qa tools: > File not found : /js/locale_data.js at > /usr/share/koha/Koha/Template/Plugin/Asset.pm line 84. > > koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc: [% > Asset.js(lang _ '/js/locale_data.js') | $raw %] > Cannot find this file. Not sure if this construction works in template > toolkit btw. The construction is fine btw. Not sure why qa tools doesnt like it..
> The construction is fine btw. Not sure why qa tools doesnt like it.. Probably because lang is not defined so it looks for absolute path /js/locale_data.js Not sure why qa tools has to process templates.
(In reply to Julian Maurice from comment #11) > > The construction is fine btw. Not sure why qa tools doesnt like it.. > > Probably because lang is not defined so it looks for absolute path > /js/locale_data.js > Not sure why qa tools has to process templates. It's coming from xt/tt_valid.t (or similar code from the qa script)
Jenkins has a lot of those lines, coming from xt/author/translatable-templates.t, I do not get the warnings when run locally however, which is a bit weird. https://jenkins.koha-community.org/job/Koha_Master_D9/1172/consoleFull What about the following changes? diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc index 1ad6197288..94f86ad2f6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-close.inc @@ -26,6 +26,7 @@ var Koha = {}; </script> +[% SET lang = 'en' UNLESS lang.defined %] [% IF lang != 'en' %] [% Asset.js(lang _ '/js/locale_data.js') | $raw %] [% END %]
Or even [% IF lang.defined AND lang != 'en' %]
(In reply to Jonathan Druart from comment #14) > Or even > > [% IF lang.defined AND lang != 'en' %] I never used the .defined VMethod of Template::Toolkit, but it should be ok. I'm not sure it is even needed. I think you can simply write [% IF lang && lang != 'en' %] Since no language code can evaluate to false
(In reply to Julian Maurice from comment #15) > (In reply to Jonathan Druart from comment #14) > > Or even > > > > [% IF lang.defined AND lang != 'en' %] > > I never used the .defined VMethod of Template::Toolkit, but it should be ok. > I'm not sure it is even needed. I think you can simply write > > [% IF lang && lang != 'en' %] > > Since no language code can evaluate to false Looks good to me, nice and terse.
(In reply to Julian Maurice from comment #6) > It looks like xgettext does not like ES6 template literals. > > https://savannah.gnu.org/bugs/?50920 > > From what I understand, support for template literals was added in gettext > 0.20 (still not packaged in debian) but is still buggy in latest released > version 0.20.1 and a fix is present in master. Hmm.. do we need to discuss this bit a bit further... are you suggesting a coding guideline fix or am I misunderstanding?
(In reply to Martin Renvoize from comment #17) > (In reply to Julian Maurice from comment #6) > > It looks like xgettext does not like ES6 template literals. > > > > https://savannah.gnu.org/bugs/?50920 > > > > From what I understand, support for template literals was added in gettext > > 0.20 (still not packaged in debian) but is still buggy in latest released > > version 0.20.1 and a fix is present in master. > > Hmm.. do we need to discuss this bit a bit further... are you suggesting a > coding guideline fix or am I misunderstanding? See bug 24725.
(In reply to Jonathan Druart from comment #18) > (In reply to Martin Renvoize from comment #17) > > (In reply to Julian Maurice from comment #6) > > > It looks like xgettext does not like ES6 template literals. > > > > > > https://savannah.gnu.org/bugs/?50920 > > > > > > From what I understand, support for template literals was added in gettext > > > 0.20 (still not packaged in debian) but is still buggy in latest released > > > version 0.20.1 and a fix is present in master. > > > > Hmm.. do we need to discuss this bit a bit further... are you suggesting a > > coding guideline fix or am I misunderstanding? > > See bug 24725. Awesome, thanks Jonathan :)
Created attachment 99586 [details] [review] Bug 24661: (RM follow-up) Check lang is defined This adds a check that the lang variable is defined before testing against it. In normal operation this is likely a no-op, but it is required for compiling templates during QA. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
not pushed to 19.11.x due to dependencies
Pushed to 19.11.x for 19.11.04
There is a variable name collision with "lang" in some scripts: It's passed to all templates by C4::Templates to pass the language of the UI 84 $self->lang($lang); And, for instance, in controller like members/memberentry.pl 790 $template->param(%data); I know what we do in memberentry.pl is ugly, but fixing it will require some deep change it seems. Maybe the easiest would be to rename the "global" $lang with something like $ui_lang? The error in the log is: File not found : default/js/locale_data.js at /kohadevbox/koha/Koha/Template/Plugin/Asset.pm line 84. because borrowers.lang is "default" by default.