Summary: | xt/tt_valid.t is too strict | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Templates | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | RESOLVED LATER | QA Contact: | |
Severity: | critical | ||
Priority: | P5 - low | CC: | chris, claire.hernandez, f.demians, paul.poulain |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 6458 | ||
Bug Blocks: | 8385 | ||
Attachments: |
Bug 8407: make tt_valid less strict (for non translatable strings)
Bug 8407: make tt_valid less strict (for non translatable strings) Bug 8407: make tt_valid less strict (for non translatable strings) Bug 8407: make tt_valid less strict (for non translatable strings) |
Description
Jonathan Druart
2012-07-10 13:16:19 UTC
Created attachment 10744 [details] [review] Bug 8407: make tt_valid less strict (for non translatable strings) Proposed patch 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) 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. Hi Chris, I proposed a patch for Bug 8385. It fixes these 2 errors. Created attachment 10808 [details] [review] Bug 8407: make tt_valid less strict (for non translatable strings) Signed-off-by: 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 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? Created attachment 10870 [details] [review] 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 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. 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 Created attachment 11139 [details] [review] 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> 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> (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 I guess that proves that the problem still exists with TT :( So we shouldn't change the test and fix the templates. 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 ;-( |