[Bug 8407] New: xt/tt_valid.t is too strict
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Priority: P5 - low Change sponsored?: --- Bug ID: 8407 Assignee: oleonard@myacpl.org Summary: xt/tt_valid.t is too strict Severity: enhancement Classification: Unclassified OS: All Reporter: jonathan.druart@biblibre.com Hardware: All Status: NEW Version: unspecified Component: Templates Product: Koha A new bug (Bug 8385) highlights a problem already known : In Koha, we forbid the following syntax in tt files: <option value="foo" [% IF ( option.selected ) %] selected="selected" [% END %]>bar</option> We prefer : [% IF ( option.selected ) %] <option value="foo" selected="selected">bar</option> [% ELSE %] <option value="foo">bar</option> [% END %] For Bug 8385, if I want to make tt_valid.t happy, I should replace 8 lines (with the same structure as previous) with 40 lines. These new lines get nowhere. Less readability (discutable), less concise, a lot of useless lines, etc. Why ? I don't know exactly. The translator tool crashes if this kind of syntax occurs. In fact, after few test, it seems the problem existed with HTML::Template, but I don't get through with Template::Toolkit. It would seem that we can introduce a IF statement to add attributes like selected, checked or disabled (for example). Given that the string (between IF and END) does not contain translatable strings. I will propose a patch (for discussion and massive test). It would be great if several testers could check that this patch does not valid a broken template or a template containing non translatable strings. Like as Frédéric Demians said me, it is a time bomb and we have to be sure the templates do not passed as valid if they are not. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion Assignee|oleonard@myacpl.org |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #1 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 10744 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10744&action=edit Bug 8407: make tt_valid less strict (for non translatable strings) Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8385 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr Depends on| |6458 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 claire.hernandez@biblibre.com <claire.hernandez@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |claire.hernandez@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff CC| |chris@bigballofwax.co.nz, | |paul.poulain@biblibre.com --- Comment #2 from Paul Poulain <paul.poulain@biblibre.com> --- Chris, could you give your opinion here ? That did not work at all with HTML::Template::Pro, but it seems it work well with T::T (jonathan, I've switched to need signoff) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #3 from Chris Cormack <chris@bigballofwax.co.nz> --- These seems to work ok, the template still passes the translated test. Before the patch perl xt/author/translatable-templates.t 1..2 ok 1 - opac templates are translatable ok 2 - intranet templates are translatable xt/tt_valid.t 2012-07-13 08:04:57 christopher pts/2 1..2 not ok 1 - TT syntax: not using TT directive within HTML tag # Failed test 'TT syntax: not using TT directive within HTML tag' # at xt/tt_valid.t line 55. # Files list: # intranet-tmpl/prog/en/modules/admin/searchengine/solr/indexes.tt: 132, 133, 134, 135, 136, 137, 141, 144 ok 2 - Token received same as original put on stack # Looks like you failed 1 test of 2. After the patch perl xt/tt_valid.t 2012-07-13 08:09:14 christopher pts/2 1..2 not ok 1 - TT syntax: not using TT directive within HTML tag # Failed test 'TT syntax: not using TT directive within HTML tag' # at xt/tt_valid.t line 60. # Files list: # intranet-tmpl/prog/en/modules/admin/searchengine/solr/indexes.tt: 141, 144 ok 2 - Token received same as original put on stack # Looks like you failed 1 test of 2. Dropped the errors in the tt down from 8 to 2. Looking at that file It appears to still be the checked="checked" lines it doesn't like. If we fix that up, Ill do the first signoff. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #4 from Jonathan Druart <jonathan.druart@biblibre.com> --- Hi Chris, I proposed a patch for Bug 8385. It fixes these 2 errors. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10744|0 |1 is obsolete| | --- Comment #5 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 10808 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10808&action=edit Bug 8407: make tt_valid less strict (for non translatable strings) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #6 from Chris Cormack <chris@bigballofwax.co.nz> --- I have signed off, but will leave it needs signoff, I would like to get it a bit more testing. It seems safe, but If someone could test creating and updating the .po files for another language as well as installing them. I only did english-nz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |katrin.fischer@bsz-bw.de --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I tested breaking the templates and checking the output of the test. I think maybe the test needs should be modified to ignore the keywords disabled, checked and selected in a TT directive. Example: <th [% UNLESS ( selected ) %]title="TESTING"[% END %]>Date</th> This will pass the test. I am not sure, if it really breaks translations. In my tests "TESTING" didn't show up in the updated po files. I am not sure, if this is reason to fail this an if it really breaks something. Frederic, maybe you can take a look and give your opinion? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10808|0 |1 is obsolete| | --- Comment #8 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 10870 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10870&action=edit Bug 8407: make tt_valid less strict (for non translatable strings) Hi Katrin, Good catch :) This one fails for the following lines: <th [% UNLESS ( selected ) %] title="TESTING" [% END %]>Date</th> <th [% UNLESS ( selected ) %]title="TESTING"[% END %]>Date</th> <th [% UNLESS ( selected ) %] selected="selected" [% END %]>Date</th> and passes: <th [% UNLESS ( selected ) %]selected="selected"[% END %]>Date</th> <th [% UNLESS ( selected ) %]disabled="disabled"[% END %]>Date</th> <th [% UNLESS ( selected ) %]checked="checked"[% END %]>Date</th> Switch back to needs signedoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |critical --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- I raise the priority for this bug. Indeed, Jenkins is not happy currently. Either this patch corrects and does not introduce regression, or I will submit a patch for Bug 8385 (which will replace 6 lines with 6*5 [useless] lines). Please test and report any eventual problems. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #10 from Chris Cormack <chris@bigballofwax.co.nz> --- Have signed off again, I am happy with it. It fixes the problems katrin found. Works with Maori and English-NZ Drops the fails to 1, which bug 8385 fixes -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10870|0 |1 is obsolete| | --- Comment #11 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 11139 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11139&action=edit Bug 8407: make tt_valid less strict (for non translatable strings) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #12 from Paul Poulain <paul.poulain@biblibre.com> --- chris, could you describe the exact tests you made. I did the following: in memberentrygen.tt, replaced [% IF ( categoryloo.categorycodeselected ) %] <option value="[% categoryloo.categorycode %]" selected="selected" data-typename="[% typeloo.typename %]">[% categoryloo.categoryname %]</option> [% ELSE %] <option value="[% categoryloo.categorycode %]" data-typename="[% typeloo.typename %]">[% categoryloo.categoryname %]</option> by <option value="[% categoryloo.categorycode %]" [% IF ( categoryloo.categorycodeselected ) %]selected="selected"[% END %] data-typename="[% typeloo.typename %]" title="blablabla">[% categoryloo.categoryname %]</option> in english, no problems. then ./translate update fr-FR translate blablabla into bliblibli then ./translate install fr-FR switching to english, I get a very nasty: Template process failed: file error - parse error - /home/paul/koha.dev/koha-community/koha-tmpl/intranet-tmpl/prog/fr-FR/modules/members/memberentrygen.tt line 520: unexpected token (%) [% "[%">[% categoryloo.categoryname %] at /home/paul/koha.dev/koha-community/C4/Templates.pm line 127. at /home/paul/koha.dev/koha-community/members/memberentry.pl line 842 Checking the french templates, I see: [% FOREACH typeloo IN typeloop %] [% FOREACH categoryloo IN typeloo.categoryloop %] [% IF ( loop.first ) %] [% IF ( typeloo.typename_C ) %]<optgroup value="C" label="Enfant">[% END %] [% IF ( typeloo.typename_A ) %]<optgroup value="A" label="Adulte">[% END %] [% IF ( typeloo.typename_S ) %]<optgroup label="Staff" value="S">[% END %] [% IF ( typeloo.typename_I ) %]<optgroup value="I" label="Collectivité">[% END %] [% IF ( typeloo.typename_P ) %]<optgroup value="P" label="Professionnel">[% END %] [% IF ( typeloo.typename_X ) %]<optgroup value="X" label="Statistiques">[% END %] [% END %] <option data-typename="[% typeloo.typename %]" value="[% categoryloo.categorycode %]" categoryloo.categorycodeselected="categoryloo.categorycodeselected" %]="%]" if="IF" end="END" )=")" title="bliblibli" %]selected="selected" (="(" [%="[%">[% categoryloo.categoryname %]</option> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #13 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #12)
switching to english, I get a very nasty:
One must read "switching to french, I get a very nasty:...3 In english, everything is OK -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I guess that proves that the problem still exists with TT :( So we shouldn't change the test and fix the templates. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- Problem is in routine text_replace_tag (tmpl_process3.pl). I don't know how we can fix it :-/ I abandon this bug (for now...) and will submit a patch for Bug 8385 ;-( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8407 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |LATER -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org