[Bug 41846] New: Notes field of routing list displays html characters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Bug ID: 41846 Summary: Notes field of routing list displays html characters Initiative type: --- Sponsorship --- status: Product: Koha Version: 25.05 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Serials Assignee: koha-bugs@lists.koha-community.org Reporter: esther.melander@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Created attachment 193148 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193148&action=edit routing list example with html To replicate: 1. Be sure the RoutingSerials system preference is enabled. 2. Find a serial with a subscription. Click on Edit Routing List. 3. Add some recipients and add some notes. Be sure the notes are formatted something like this line breaks: Testing out notes 1-26-26 more notes 2-5-26 = more notes, 3 copies 4. Save. Notice the text of the note appears in one line with the html notation: Notes:Testing out notes <br />1-26-26 more notes <br />2-5-26 = more notes, 3 copies 5. Click on Save and preview routing slip. Notice the text with the html notation carries through to the print preview display. I would expect the html notation to not display. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au, | |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #1 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- CCing David here. I think this is behaving the way we want in that HTML should not be rendered? If it is to be rendered it should be scrubbed first? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #2 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- I had forgotten about Bug 9750, perhaps that makes this one invalid? https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9750 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This is an individual note you can add on the routing list per issue. It would not be affected by the move to a notice template but should still be an option that we can insert into it on the fly. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- So there's an easy explanation for this one. serials/routing-preview.pl converts newlines to <br />: 137 my $routingnotes = $serials[0]->{'routingnotes'}; 138 $routingnotes =~ s/\n/\<br \/\>/g; I have an idea for this one which might work. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Created attachment 193371 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193371&action=edit Bug 41846: Fix nl2br to happen after html escaping This patch removes the newline to <br/> (nl2br) handling in the .pl script, and instead moves it after the Template::Toolkit "html" filter. In this way, we get the security of the "html" filter while also converting new lines into safe <br/> elements. We also fix a little style problem. Hurray! Test plan: 0. Apply the patch and koha-plack --restart kohadev 1. Ensure the RoutingSerials system preference is enabled 2. Create a "Subscription" for a known biblionumber (e.g. 29) with whatever details 3. On the subscription-detail.pl page for that subscription, click on "Create routing list" 4. Add whatever recipients you want and put the following into Notes: Testing out notes 1-26-26 more notes 2-5-26 = more notes, 3 copies <p>Awesome... still safe</p> 5. Click "Save" 6. Observe that your notes are output exactly the same way you put them in. The newlines appear to be newlines to the human eye, while that pesky <p></p> markup is escaped. 7. Observe that the notes are also neatly lined up with the rest of the routing list display for an attractive user experience 8. Celebrate again. Hurray! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | --- Comment #6 from David Cook <dcook@prosentient.com.au> --- So I fixed this problem without even looking at bug 9750... but bug 9750 would make it irrelevant. I'll take a look at bug 9750 now... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=9750 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #6)
So I fixed this problem without even looking at bug 9750... but bug 9750 would make it irrelevant.
I'll take a look at bug 9750 now...
The patches on Bug 9750 actually break this even further hehe. So I think we can proceed with this patch. As Katrin said, bug 9750 shouldn't actually overlap with this one anyway after all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- In terms of the change, basically we stop the controller script doing the newline to <br/> and instead use a Template::Toolkit filter to do it after we apply the "html" filter. Since newline to <br/> is safe and doesn't use HTML in the source before the "html" filter, we're all good. Chalk one more up for the good guys :D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #9 from David Nind <david@davidnind.com> ---
From testing:
1. Before the patch, the notes are shown in a continuous line without the line breaks. 2. After the patch: displays as expected, with new lines shown, the HTML <p> tag is also shown (owning and closing). 3. After you save the routing list: - "Save and preview routing slip": back to being on a continuous line, without the line breaks - Same when you print the routing slip Questions: 1. Should the HTML markup be shown or stripped out? 2. Should the preview routing slip and printed version be fixed up as well, or is that a separate bug? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- (In reply to David Nind from comment #9)
From testing:
1. Before the patch, the notes are shown in a continuous line without the line breaks.
2. After the patch: displays as expected, with new lines shown, the HTML <p> tag is also shown (owning and closing).
3. After you save the routing list: - "Save and preview routing slip": back to being on a continuous line, without the line breaks - Same when you print the routing slip
Questions:
1. Should the HTML markup be shown or stripped out?
Most of Koha just escapes it so the intention was to show it.
2. Should the preview routing slip and printed version be fixed up as well, or is that a separate bug?
Yeah no I think you're right. I'll fix that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- Created attachment 193668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193668&action=edit Bug 41846: Fix routing-preview-slip.tt too -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- Thanks again, David. That should be better now. Good catch! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #193371|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #193668|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 193679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193679&action=edit Bug 41846: Fix nl2br to happen after html escaping This patch removes the newline to <br/> (nl2br) handling in the .pl script, and instead moves it after the Template::Toolkit "html" filter. In this way, we get the security of the "html" filter while also converting new lines into safe <br/> elements. We also fix a little style problem. Hurray! Test plan: 0. Apply the patch and koha-plack --restart kohadev 1. Ensure the RoutingSerials system preference is enabled 2. Create a "Subscription" for a known biblionumber (e.g. 29) with whatever details 3. On the subscription-detail.pl page for that subscription, click on "Create routing list" 4. Add whatever recipients you want and put the following into Notes: Testing out notes 1-26-26 more notes 2-5-26 = more notes, 3 copies <p>Awesome... still safe</p> 5. Click "Save" 6. Observe that your notes are output exactly the same way you put them in. The newlines appear to be newlines to the human eye, while that pesky <p></p> markup is escaped. 7. Observe that the notes are also neatly lined up with the rest of the routing list display for an attractive user experience 8. Celebrate again. Hurray! Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #14 from David Nind <david@davidnind.com> --- Created attachment 193680 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193680&action=edit Bug 41846: Fix routing-preview-slip.tt too Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This change fixes the release notes| |routing list note so that | |new lines converted to <br> | |don't get escaped by the | |HTML filter. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- <div class="d-inline-block"> Why adding this class btw ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #193679|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #193680|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 198131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198131&action=edit Bug 41846: Fix nl2br to happen after html escaping This patch removes the newline to <br/> (nl2br) handling in the .pl script, and instead moves it after the Template::Toolkit "html" filter. In this way, we get the security of the "html" filter while also converting new lines into safe <br/> elements. We also fix a little style problem. Hurray! Test plan: 0. Apply the patch and koha-plack --restart kohadev 1. Ensure the RoutingSerials system preference is enabled 2. Create a "Subscription" for a known biblionumber (e.g. 29) with whatever details 3. On the subscription-detail.pl page for that subscription, click on "Create routing list" 4. Add whatever recipients you want and put the following into Notes: Testing out notes 1-26-26 more notes 2-5-26 = more notes, 3 copies <p>Awesome... still safe</p> 5. Click "Save" 6. Observe that your notes are output exactly the same way you put them in. The newlines appear to be newlines to the human eye, while that pesky <p></p> markup is escaped. 7. Observe that the notes are also neatly lined up with the rest of the routing list display for an attractive user experience 8. Celebrate again. Hurray! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 198132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198132&action=edit Bug 41846: Fix routing-preview-slip.tt too Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Notes field of routing list |Notes field of routing list |displays html characters |displays HTML characters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #18 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41846 --- Comment #19 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org