We have the following coding guidelines: ``` HTML1: Template Toolkit markup inside HTML Avoid [% IF %] inside tags because the translation tools try to read the templates as straight HTML and most TT comments inside tags probably confuse/break them. The most common example of an incorrect construction: <option value="1" [% IF (selected) %] selected="selected" [% END %]>Yes</option> This must be written this way: [% IF (selected) %] <option value="1" selected="selected">Yes</option> [% ELSE %] <option value="1">Yes</option> [% END %] ``` Everybody (should) know about it, and we are all aware it must not be done like that. But... I will need it for bug 38664 and I have decided to have a look at fixing it. However I have not managed to break anything! I did introduce TT tags within HTML tags, but the strings are correctly picked and the translated templates is perfectly generated. What am I missing here? Did we finally fix this long-standing problem without knowing it?
> What am I missing here? Did we finally fix this long-standing problem without knowing it? Either that or it was only valid with HTML::Template Because that's how old that guideline is: https://web.archive.org/web/20060902045435/Http://www.kohadocs.org/codingguidelines.html https://wiki.koha-community.org/w/index.php?title=Coding_Guidelines&oldid=473 It was updated for the switch to TT https://wiki.koha-community.org/w/index.php?title=Coding_Guidelines&type=revision&diff=6700&oldid=6210 So maybe it was still a valid issue that got fixed years later with a fix for something else in the translation script. Or it wasn't actually valid anymore but it was assumed to be. ---- Anyway, I guess you can use it safely for bug 38664 if you see that it doesn't create broken strings. If after your work there and your failed attempts to break things here, HTML1 could safely be obsoleted if nothing comes up.
Found one problem so far: <li [% IF loop.count > 5 && !facet.active %]class="collapsible-facet" style="display:none"[% END %]> It breaks because of the '>' in the TT condition.
Created attachment 177062 [details] [review] Bug 38720: HTML1 no longer valid - remove xt/tt_valid.t
(In reply to Jonathan Druart from comment #2) > Found one problem so far: > > <li [% IF loop.count > 5 && !facet.active %]class="collapsible-facet" > style="display:none"[% END %]> > > It breaks because of the '>' in the TT condition. We might need to reintroduce the test with an adjustment to catch that, but it's for later.
Test plan: Test bug 38714, if there is nothing wrong found there, this can go.
> gulp po:update --lang fr-FR Error: Command failed: misc/translator/xgettext.pl --charset=UTF-8 -F -o /tmp/koha-LJMkbx/Koha-opac-bootstrap.pot -f /tmp/koha-LJMkbx/files Incorrect structure found at ./koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc:80: '<li ' at misc/translator/xgettext.pl line 124, <$INPUT> line 121. The line: <li [% IF loop.count > 5 && !facet.active %]class="collapsible-facet" style="display:none"[% END %]>
(In reply to Owen Leonard from comment #6) > > gulp po:update --lang fr-FR > > Error: Command failed: misc/translator/xgettext.pl --charset=UTF-8 -F -o > /tmp/koha-LJMkbx/Koha-opac-bootstrap.pot -f /tmp/koha-LJMkbx/files > Incorrect structure found at > ./koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc:80: '<li ' at > misc/translator/xgettext.pl line 124, <$INPUT> line 121. > > The line: > > <li [% IF loop.count > 5 && !facet.active %]class="collapsible-facet" > style="display:none"[% END %]> See the previous comment, it's the only problem I encountered. It's fixed on "Bug 38664: opac-facets.inc - Fix translation script"
I'm happy with the discussions here and also can't find any other breaking cases.. I reckon we can safely go with this.
Created attachment 177688 [details] [review] Bug 38720: HTML1 no longer valid - remove xt/tt_valid.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Pushed for 25.05! Well done everyone, thank you!