Bug 21669 - TT assignment statements must not be html filtered
Summary: TT assignment statements must not be html filtered
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 13618 21663
Blocks:
  Show dependency treegraph
 
Reported: 2018-10-25 16:06 UTC by Jonathan Druart
Modified: 2019-10-14 19:56 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 21669: Do not html filter TT assignement statements (53.89 KB, patch)
2018-10-25 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21669: Do not html filter TT assignement statements (52.86 KB, patch)
2018-10-25 17:52 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 21669: Do not html filter TT assignement statements (53.13 KB, patch)
2018-10-26 13:36 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 Jonathan Druart 2018-10-25 16:06:11 UTC

    
Comment 1 Jonathan Druart 2018-10-25 16:20:43 UTC
Created attachment 81184 [details] [review]
Bug 21669: Do not html filter TT assignement statements

Why? Because we must filter the variables when we display them.
If we escape them on assignement, they will be double escaped:
  [% XXX = "<span>pouet</span>" | html %]
  [% XXX | html %]
  => &lt;span&gt;pouet&lt;/span&gt;

Also it will bring trouble if we are assigning a structure (see bug
21663 for instance).
Comment 2 Jonathan Druart 2018-10-25 16:21:50 UTC
No test plan here, too many changes.

- Eyeball the patch
- Are the changes make sense?
Comment 3 Owen Leonard 2018-10-25 17:52:41 UTC
Created attachment 81191 [details] [review]
Bug 21669: Do not html filter TT assignement statements

Why? Because we must filter the variables when we display them.
If we escape them on assignement, they will be double escaped:
  [% XXX = "<span>pouet</span>" | html %]
  [% XXX | html %]
  => &lt;span&gt;pouet&lt;/span&gt;

Also it will bring trouble if we are assigning a structure (see bug
21663 for instance).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Marcel de Rooy 2018-10-26 09:31:01 UTC
fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt).
Repository lacks necessary blobs to fall back on 3-way merge.
Comment 5 Jonathan Druart 2018-10-26 12:12:30 UTC
Please apply first:
Bug 13618: Remove html filter for LAST
Bug 21663: Incorrect filter prevents predefined notes from being added to patron acccounts


I thought they would have been pushed quickly.
Comment 6 Katrin Fischer 2018-10-26 13:28:03 UTC
Even with the bugs mentioned I can't get past:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 21669: Do not html filter TT assignement statements
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt).
error: could not build fake ancestor
Comment 7 Katrin Fischer 2018-10-26 13:36:41 UTC
Created attachment 81289 [details] [review]
[PASSED QA] Bug 21669: Do not html filter TT assignement statements

Why? Because we must filter the variables when we display them.
If we escape them on assignement, they will be double escaped:
  [% XXX = "<span>pouet</span>" | html %]
  [% XXX | html %]
  => &lt;span&gt;pouet&lt;/span&gt;

Also it will bring trouble if we are assigning a structure (see bug
21663 for instance).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Nick Clemens (kidclamp) 2018-10-26 17:25:19 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize (ashimema) 2018-11-06 13:24:57 UTC
Caused by bug 13618 not in 18.05.x series.