Bug 21418 - Incorrectly filtered markup in staff client lists
Summary: Incorrectly filtered markup in staff client lists
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Owen Leonard
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 13618
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-26 17:50 UTC by Owen Leonard
Modified: 2019-10-14 19:58 UTC (History)
23 users (show)

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


Attachments
Bug 21418: Incorrectly filtered markup in staff client lists (4.37 KB, patch)
2018-09-26 17:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21418: Incorrectly filtered markup in staff client lists (4.45 KB, patch)
2018-09-29 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21418: Html escape the 2 variables (1.44 KB, patch)
2018-09-29 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21418: Remove shelfoff which is not used (1.55 KB, patch)
2018-09-29 14:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21418: Html escape the 2 variables (1.48 KB, patch)
2018-10-01 12:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21418: Remove shelfoff which is not used (1.60 KB, patch)
2018-10-01 12:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21418: Incorrectly filtered markup in staff client lists (4.50 KB, patch)
2018-10-02 16:42 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21418: Html escape the 2 variables (1.54 KB, patch)
2018-10-02 16:42 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21418: Remove shelfoff which is not used (1.65 KB, patch)
2018-10-02 16:42 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2018-09-26 17:50:26 UTC
The AJAX DataTable of existing lists is broken because the action buttons are incorrectly rendered. The "| html" filter should be removed from  the action_block declarations.
Comment 1 Owen Leonard 2018-09-26 17:58:49 UTC
Created attachment 79442 [details] [review]
Bug 21418: Incorrectly filtered markup in staff client lists

This patch removes the "|html" filter from some variable declarations in
the template used to display the list of lists in the staff client.

To test you should have at least one list. Apply the patch and go to
Lists.

In the table of lists, the "Edit" and "Delete" buttons should look
correct and work correctly.
Comment 2 Chris Cormack 2018-09-26 22:11:26 UTC
Hey Owen

I'm pretty sure we need to filter everything or the QA scripts will complain, so we should use raw when we don't want html

https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML9:_filter_all_the_variables

That way its easy to find unfiltered variables (by mistake) vs ones specifically chosen not to be filtered. If that makes sense?
Comment 3 Owen Leonard 2018-09-27 11:40:18 UTC
But isn't it getting filtered later at line 60?

[%~ To.json(action_block) | $raw ~%]
Comment 4 Jonathan Druart 2018-09-27 12:34:59 UTC
If the TT block contains '=' the script/test will not complain, so these changes should be good.
Comment 5 Jonathan Druart 2018-09-29 14:27:49 UTC
Created attachment 79610 [details] [review]
Bug 21418: Incorrectly filtered markup in staff client lists

This patch removes the "|html" filter from some variable declarations in
the template used to display the list of lists in the staff client.

To test you should have at least one list. Apply the patch and go to
Lists.

In the table of lists, the "Edit" and "Delete" buttons should look
correct and work correctly.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2018-09-29 14:27:55 UTC
Created attachment 79611 [details] [review]
Bug 21418: Html escape the 2 variables

We should be on the safe side without this patch because shelfnumber and
type comes from the DB and are integer or varchar. It may be better to
show good examples to start, and escape everything anyway.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2018-09-29 14:28:00 UTC
Created attachment 79612 [details] [review]
Bug 21418: Remove shelfoff which is not used

No need to add a new line to escape this variable that is never used,
better to remove it

`git grep shelfoff` will prove that it is not used anywhere else.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Owen Leonard 2018-10-01 12:00:52 UTC
Created attachment 79706 [details] [review]
Bug 21418: Html escape the 2 variables

We should be on the safe side without this patch because shelfnumber and
type comes from the DB and are integer or varchar. It may be better to
show good examples to start, and escape everything anyway.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Owen Leonard 2018-10-01 12:00:57 UTC
Created attachment 79707 [details] [review]
Bug 21418: Remove shelfoff which is not used

No need to add a new line to escape this variable that is never used,
better to remove it

`git grep shelfoff` will prove that it is not used anywhere else.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Josef Moravec 2018-10-02 16:42:41 UTC
Created attachment 79830 [details] [review]
Bug 21418: Incorrectly filtered markup in staff client lists

This patch removes the "|html" filter from some variable declarations in
the template used to display the list of lists in the staff client.

To test you should have at least one list. Apply the patch and go to
Lists.

In the table of lists, the "Edit" and "Delete" buttons should look
correct and work correctly.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Josef Moravec 2018-10-02 16:42:49 UTC
Created attachment 79831 [details] [review]
Bug 21418: Html escape the 2 variables

We should be on the safe side without this patch because shelfnumber and
type comes from the DB and are integer or varchar. It may be better to
show good examples to start, and escape everything anyway.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2018-10-02 16:42:55 UTC
Created attachment 79832 [details] [review]
Bug 21418: Remove shelfoff which is not used

No need to add a new line to escape this variable that is never used,
better to remove it

`git grep shelfoff` will prove that it is not used anywhere else.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Nick Clemens (kidclamp) 2018-10-05 16:26:30 UTC
Awesome work all!

Pushed to master for 18.11
Comment 14 Martin Renvoize (ashimema) 2018-10-08 08:43:31 UTC
Causing bug not in 18.05.x series.