Created attachment 104375 [details] on returns.pl page With the introduction of this bug: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23409. ( Show circulation note and OPAC note with line feeds ), the ability to add HTML was included. However, when a patron note with HTML is seen during circulation (check-in)- the HTML tags are visible.
This uses the $raw filter for the circulation note and HTML can be added since it is input from the staff side. As for the opacnote, I think this filter needs to remain HTML as OPAC users should not be able to add their own HTML.
Hi Lucas, but the patrons can't change/set these notes - only the library staff can. They are only shown in OPAC.
Cait, In that case it would be OK to change those filters from HTML to $raw?
I think we have done in other places like the OPAC item notes - so it might be ok.
Created attachment 148387 [details] [review] Bug 25379: Add raw filter to circulation note on checkin page This is a consistency patch: we already allow adding HTML tags to the patron circulation note and the contents display correctly in the patron account and in the patron search results in the staff interface. But the HTML tags are printed on the checkins page. To test: * Add a circulation note including HTML tags to a patron Example: <h1>This patron is great!</h1> * Check an item out to this patron * Verify that the note displays formatted in patron search results * Verify that the note displays formatted in the patron record * Return the item * Verify the note shows the printed tags * Apply patch * Check item out again and check it in again * Verify the note now displays correctly in list of checkins
Created attachment 148496 [details] [review] Bug 25379: Add raw filter to circulation note on checkin page This is a consistency patch: we already allow adding HTML tags to the patron circulation note and the contents display correctly in the patron account and in the patron search results in the staff interface. But the HTML tags are printed on the checkins page. To test: * Add a circulation note including HTML tags to a patron Example: <h1>This patron is great!</h1> * Check an item out to this patron * Verify that the note displays formatted in patron search results * Verify that the note displays formatted in the patron record * Return the item * Verify the note shows the printed tags * Apply patch * Check item out again and check it in again * Verify the note now displays correctly in list of checkins Signed-off-by: David Nind <david@davidnind.com>
Shouldn't we use html_linebreak as well? We do in moremember: [% patron.borrowernotes | $raw | html_line_break %] Also, why are we only dealing with the checkin page? There are other places where it's displayed escaped (`git grep 'borrowernotes | html'`)
Created attachment 150395 [details] [review] Bug 25379: Add raw filter to circulation note on checkin page This is a consistency patch: we already allow adding HTML tags to the patron circulation note and the contents display correctly in the patron account and in the patron search results in the staff interface. But the HTML tags are printed on the checkins page. To test: * Add a circulation note including HTML tags to a patron Example: <h1>This patron is great!</h1> * Check an item out to this patron * Verify that the note displays formatted in patron search results * Verify that the note displays formatted in the patron record * Return the item * Verify the note shows the printed tags * Apply patch * Check item out again and check it in again * Verify the note now displays correctly in list of checkins Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 150396 [details] [review] Bug 25379: (QA follow-up) Adjust other occurrences and add line break This updates the other occurrences and adds html_line_break to all except the edit page - it seems html or raw doesn't affect the value of the textarea, but moved to raw for consistency To test: 1 - Add an HTML note to a patron, make sure to add a line break: <h1>Testing</h1> <h2>Newline</h2> 2 - Checkout to patron and return 3 - Confirm note shows correctly 4 - Add patron to a list 5 - Confirm note displays correctly 6 - Edit patron, confirm note field is correct 7 - Batch modify borrower 8 - Confirm note displays correctly in batch list Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Minor followups, RM can veirfy, PQA
Thanks, Nick! I am not sure really about allowing HTML and then also meddling with line breaks. It creates a somewhat weird combination. html_line_break This filter replaces any newlines with <br> HTML tags, thus preserving the line breaks of the original text in the HTML output. So you can do now without <br>, but if you add any line breaks just for making the HTML more readable, they will also all create a line break. Should we not do either/or?
(In reply to Katrin Fischer from comment #11) >if you add any line breaks just for > making the HTML more readable, they will also all create a line break. > Should we not do either/or? We had calls that already did this - if we want either or, we need a flag to determine whether the field is HTML or not, otherwise we need to test with regex? I don't think the additional breaks for HTML cause a problem, but I'll follow your viewpoint. Maybe a new bug - so we can make a decision and be consistent on all the calls?
(In reply to Nick Clemens from comment #12) > (In reply to Katrin Fischer from comment #11) > >if you add any line breaks just for > > making the HTML more readable, they will also all create a line break. > > Should we not do either/or? > > We had calls that already did this - if we want either or, we need a flag to > determine whether the field is HTML or not, otherwise we need to test with > regex? > > I don't think the additional breaks for HTML cause a problem, but I'll > follow your viewpoint. Maybe a new bug - so we can make a decision and be > consistent on all the calls? I think mostly people will probably use bold, italic etc and not do whole paragraphs here, so we are probably OK with the line breaks. You are right, we should discuss separately when needed, but I feel we can wait and see if it actually turns out to be an issue.
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to stable for 22.11.x