In the main ILL template file ill-requests.tt a few variable declarations that are not strings have the "html" filter applied to them, this breaks them. These filters should be removed.
Created attachment 79079 [details] [review] Bug 21377: Remove erroneous "html" filter use The "html" filter that is being applied to these variable declarations is inappropriate and has been removed. To test: - Before applying the patch, on master, view the "Manage request" page for an ILL request - TEST: Observe that, apart from the "Edit request" button, the various available action buttons do not display correctly - Apply the patch - TEST: Refresh the page and observe that the buttons now display correctly
Upping the priority of this as it impacts every ILL bug
...including master
If variables should not be escaped, you have to be explicit about it by using the $raw filter: https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML9:_filter_all_the_variables
Created attachment 79401 [details] [review] Bug 21377: (follow-up) Add raw filter Rather than just removing the html filter, it should be replaced with the raw filter
Thanks for the pointer Julian, I'd completely missed the purpose of the raw filter, thanks for flagging it up!
Actually, I've just noticed that adding the raw filter breaks it (stupid browser caching when I tested it). This suggests the raw filter doesn't just pass the variable straight through and somehow breaks variables that aren't just strings. I'll do some digging...
I'm not sure applying any filter is appropriate. I'm just assigning a variable based on another variable, there's no displaying going on. So, actually looking at what I'm doing, I'm not surprised it breaks it!
Created attachment 79402 [details] [review] Bug 21377: (follow-up) Re-remove raw filter This patch removes the raw filter that was erroneously applied, these are variable declarations, they don't need filtering. I've also simplified things by removing two of the extraneous variable declarations.
(In reply to Andrew Isherwood from comment #7) > Actually, I've just noticed that adding the raw filter breaks it (stupid > browser caching when I tested it). This suggests the raw filter doesn't just > pass the variable straight through and somehow breaks variables that aren't > just strings. I'll do some digging... That's odd. The raw filter should return exactly what it has been given. But as theses variables are not directly displayed, it should be ok without filters. Sorry for the extra work
No worries :) It actually ended up with me simplifying things a little bit, so was worthwhile!
Created attachment 79437 [details] [review] Bug 21377: Remove erroneous "html" filter use The "html" filter that is being applied to these variable declarations is inappropriate and has been removed. To test: - Before applying the patch, on master, view the "Manage request" page for an ILL request - TEST: Observe that, apart from the "Edit request" button, the various available action buttons do not display correctly - Apply the patch - TEST: Refresh the page and observe that the buttons now display correctly Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 79438 [details] [review] Bug 21377: (follow-up) Add raw filter Rather than just removing the html filter, it should be replaced with the raw filter Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 79439 [details] [review] Bug 21377: (follow-up) Re-remove raw filter This patch removes the raw filter that was erroneously applied, these are variable declarations, they don't need filtering. I've also simplified things by removing two of the extraneous variable declarations. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 79512 [details] [review] Bug 21377: Remove erroneous "html" filter use The "html" filter that is being applied to these variable declarations is inappropriate and has been removed. I've also simplified things by removing two of the extraneous variable declarations. To test: - Before applying the patch, on master, view the "Manage request" page for an ILL request - TEST: Observe that, apart from the "Edit request" button, the various available action buttons do not display correctly - Apply the patch - TEST: Refresh the page and observe that the buttons now display correctly Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Squashed all patches into one, to make it easier to read
Awesome work all! Pushed to master for 18.11
Dependencies/Cause not in 18.05.x series.