Bug 12076 - xt/tt_valid.t can miss an invalid construct
Summary: xt/tt_valid.t can miss an invalid construct
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-11 18:48 UTC by Galen Charlton
Modified: 2015-06-04 23:30 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12076: better detect an untranslatable template construct (1.54 KB, patch)
2014-04-11 18:57 UTC, Galen Charlton
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12076: better detect an untranslatable template construct (1.67 KB, patch)
2014-04-11 21:23 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 12076: better detect an untranslatable template construct (1.77 KB, patch)
2014-04-12 16:04 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2014-04-11 18:48:34 UTC
Per bug 6458, template constructs of the form

<li [% IF (foo) %]selected="selected"[% END %]...

as that can cause problems with translated templates.  However, the test currently doesn't catch the variation where '-' is used to suppress extra whitespace:

<li [% IF (foo) %]selected="selected"[% END %]...
Comment 1 Galen Charlton 2014-04-11 18:50:56 UTC
(In reply to Galen Charlton from comment #0)
> Per bug 6458, template constructs of the form
> 
> <li [% IF (foo) %]selected="selected"[% END %]...

are forbidden, to complete the thought.
Comment 2 Galen Charlton 2014-04-11 18:56:30 UTC
(In reply to Galen Charlton from comment #0)
> <li [% IF (foo) %]selected="selected"[% END %]...

*sigh*.  I meant

<li [%- IF (foo) -%]selected="selected"[%- END -%]...
Comment 3 Galen Charlton 2014-04-11 18:57:27 UTC Comment hidden (obsolete)
Comment 4 Bernardo Gonzalez Kriegel 2014-04-11 21:23:19 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2014-04-12 16:04:43 UTC
Created attachment 27053 [details] [review]
[PASSED QA] Bug 12076: better detect an untranslatable template construct

Per bug 6458, template constructs of the form

<li [% IF (foo) %]selected="selected"[% END %]...

are forbidden as they can cause problems with translated templates.
However, the tt_valid.t test currently doesn't catch the variation
where '-' is used to suppress extra whitespace:

<li [%- IF (foo) -%]selected="selected"[%- END -%]...

This patch corrects the issue.

To test:

[1] Temporarily add the following line to a template file:

<li [%- IF a -%]a="a"[%- END -%] />

[2] Run prove -v xt/tt_valid.t.  Note that no error is reported.
[3] Apply the patch, and rerun the tt_valid.t test.  This time,
    an error should be reported.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Works well, detects the forbidden pattern
No koha-qa errors.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes all tests and QA script.
Comment 6 Galen Charlton 2014-04-12 17:59:09 UTC
Pushed to master.
Comment 7 Fridolin SOMERS 2014-08-06 09:00:42 UTC
Pushed to 3.14.x, will be in 3.14.10
Comment 8 Kyle M Hall 2014-08-13 14:01:34 UTC
Picked for 3.12.15!