[Bug 16536] New: Warn about Template Toolkit directives inside HTML tags
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 Bug ID: 16536 Summary: Warn about Template Toolkit directives inside HTML tags Change sponsored?: --- Product: Project Infrastructure Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: QA tools Assignee: jonathan.druart@bugs.koha-community.org Reporter: oleonard@myacpl.org I was testing a patch (on Bug 16525) which contained this markup: <a href="[% IF ( budget_period_id) %]/cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% budget_period_id %][% ELSE %]/cgi-bin/koha/admin/aqbudgetperiods.pl[% END %]" class="cancel">Cancel</a> I expected it to trigger a warning by the QA tools. My system is a gitified package install, so it seems possible that this is a problem with my configuration. Running "prove xt" doesn't catch it either. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- xt/tt_valid.t and the QA tools do not caught this invalid syntax because the TT tags ([% %]) are inside the value of the attribute (surrounded by quotes). It could be possible to add a test for the 'selected' attributes, but it would not be exhaustive. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Owen, note that the changes made by bug 16525 do not introduce translatable issues. With a quick hack to xt/tt_valid.t, I get: # intranet-tmpl/prog/en/includes/doc-head-close-receipt.inc: 2 # intranet-tmpl/prog/en/includes/doc-head-close.inc: 19 # intranet-tmpl/prog/en/includes/facets.inc: 6, 6, 45 # intranet-tmpl/prog/en/includes/help-top.inc: 4 # intranet-tmpl/prog/en/includes/page-numbers.inc: 3, 4, 6 # intranet-tmpl/prog/en/modules/acqui/basket.tt: 554, 555, 556, 557, 558, 559, 560, 562 # intranet-tmpl/prog/en/modules/acqui/parcels.tt: 132, 138, 142 # intranet-tmpl/prog/en/modules/acqui/transferorder.tt: 125 # intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt: 383 # intranet-tmpl/prog/en/modules/admin/categories.tt: 92 # intranet-tmpl/prog/en/modules/admin/preferences.tt: 121 # intranet-tmpl/prog/en/modules/authorities/authorities.tt: 321 # intranet-tmpl/prog/en/modules/basket/basket.tt: 6 # intranet-tmpl/prog/en/modules/catalogue/subject.tt: 31 # intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: 437, 629 # intranet-tmpl/prog/en/modules/circ/printslip.tt: 14 # intranet-tmpl/prog/en/modules/circ/waitingreserves.tt: 139 # intranet-tmpl/prog/en/modules/members/moremember-print.tt: 8 # intranet-tmpl/prog/en/modules/members/moremember-receipt.tt: 6 # intranet-tmpl/prog/en/modules/members/printfeercpt.tt: 7 # intranet-tmpl/prog/en/modules/members/printinvoice.tt: 5 # intranet-tmpl/prog/en/modules/reports/stats_screen.tt: 23, 25 # intranet-tmpl/prog/en/modules/reserve/request.tt: 847 # intranet-tmpl/prog/en/modules/suggestion/suggestion.tt: 458, 459 # intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt: 415, 423, 432 # opac-tmpl/bootstrap/en/includes/doc-head-close.inc: 4 # opac-tmpl/bootstrap/en/includes/opac-facets.inc: 9, 11, 54 # opac-tmpl/bootstrap/en/includes/page-numbers.inc: 5, 11, 15 # opac-tmpl/bootstrap/en/includes/shelfbrowser.inc: 39, 41 # opac-tmpl/bootstrap/en/modules/opac-advsearch.tt: 138 # opac-tmpl/bootstrap/en/modules/opac-detail.tt: 427, 429, 437, 439, 448, 450, 459, 461, 965, 967, 1072, 1081 # opac-tmpl/bootstrap/en/modules/opac-readingrecord.tt: 48, 52 # opac-tmpl/bootstrap/en/modules/opac-review.tt: 41 # opac-tmpl/bootstrap/en/modules/opac-showreviews.tt: 8, 82 # opac-tmpl/bootstrap/en/modules/sco/help.tt: 6 # opac-tmpl/bootstrap/en/modules/sco/printslip.tt: 4 # opac-tmpl/bootstrap/en/modules/sco/sco-main.tt: 7 I had a quick look at them and have not seen any problems with them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The "quick hack" is diff --git a/xt/tt_valid.t b/xt/tt_valid.t index 7285019..c335bb9 100755 --- a/xt/tt_valid.t +++ b/xt/tt_valid.t @@ -46,9 +46,12 @@ my $checkers = [ description => 'TT syntax: not using TT directive within HTML tag', check => sub { my ($self, $name, $token) = @_; + push @{$self->{errors}->{$name}}, $token->{_lc} if exists $token->{_string} and $token->{_string} =~ m|\[%\s*IF|; my $attr = $token->{_attr}; next unless $attr; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- So our coding guideline, "HTML1: Template Toolkit markup inside HTML" is too broad? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Owen Leonard from comment #4)
So our coding guideline, "HTML1: Template Toolkit markup inside HTML" is too broad?
To avoid translatability issues, yes. But if you look at the different places where it is used, it does not ease the readability. So I think it's good to keep it like that (unless specific cases where it could be useful) even if we do not have a check to catch them. Or we could fix all the occurrences and add a new check :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 Agnes Rivers-Moore <arm@hanover.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arm@hanover.ca -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #6 from David Cook <dcook@prosentient.com.au> --- Not sure what's going on here, but is this one still valid? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16536 Agnes Rivers-Moore <arm@hanover.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|arm@hanover.ca | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org