Bug 41053 - Make notice contents searchable on notices tab of patron details
Summary: Make notice contents searchable on notices tab of patron details
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
: 41140 (view as bug list)
Depends on: 39410
Blocks:
  Show dependency treegraph
 
Reported: 2025-10-20 08:58 UTC by Marcel de Rooy
Modified: 2025-12-02 10:56 UTC (History)
9 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.06
Circulation function:


Attachments
Bug 41053: Move notice contents to hidden div (2.60 KB, patch)
2025-10-20 09:23 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 41053: Move notice contents to hidden div (2.61 KB, patch)
2025-10-24 14:21 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 41053: Move notice contents to hidden div (2.68 KB, patch)
2025-11-13 09:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2025-10-20 08:58:43 UTC
In the past we could search. Changing the hidden iframe to a hidden div where the html filtered content goes from attribute to inner html.
Comment 1 Marcel de Rooy 2025-10-20 09:23:33 UTC
Created attachment 188143 [details] [review]
Bug 41053: Move notice contents to hidden div

With thanks to Jonathan for suggesting this direction.

Test plan:
Go to patron detail, notices tab for a patron with some notices.
Look for a string that only occurs in the notice text of some notice.
Without this patch, you should not find that one.
With this patch, you should find it.
Manipulate the text of a notice to check display of bad html. (Click title.)
* Like: update message_queue set content='<p>test</p><a href="' where message_id=X;

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Brendan Lawlor 2025-10-24 14:21:44 UTC
Created attachment 188404 [details] [review]
Bug 41053: Move notice contents to hidden div

With thanks to Jonathan for suggesting this direction.

Test plan:
Go to patron detail, notices tab for a patron with some notices.
Look for a string that only occurs in the notice text of some notice.
Without this patch, you should not find that one.
With this patch, you should find it.
Manipulate the text of a notice to check display of bad html. (Click title.)
* Like: update message_queue set content='<p>test</p><a href="' where message_id=X;

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 3 Brendan Lawlor 2025-10-24 14:24:23 UTC
I never knew that you could search by the content of the notice before. That's really  useful.

This works perfectly. Thanks!
Comment 4 Marcel de Rooy 2025-10-27 07:38:00 UTC
(In reply to Brendan Lawlor from comment #2)
> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Great. Thx
Comment 5 Jonathan Druart 2025-11-03 11:13:20 UTC
*** Bug 41140 has been marked as a duplicate of this bug. ***
Comment 6 Martin Renvoize (ashimema) 2025-11-13 09:18:44 UTC
Adding some history I found whilst digging into this.. I'm slightly concerned we're removing sandboxing with this patch.

---

Bug 12123 (2014) - Iframe introduced
  - Problem: HTML notices with malformed HTML (unclosed tags, invalid nesting like <blockquote> inside <p>) were breaking the notice viewer interface
  - Solution: Used <iframe srcdoc="..."> to sandbox the HTML content and prevent it from affecting the parent page structure
  - Key quote: "Depending on the content of an html notice, it can cause the notice to either not be collapsible, be uncollapsible, or to be permanently collapsed."

  Bug 39410 (March 2025) - Shadow DOM added
  - Problem: Notice previews in the staff interface showed Bootstrap styles that wouldn't actually appear in emails/prints sent to patrons
  - Solution: When displaying the notice modal, the content from the iframe's srcdoc is loaded into a Shadow DOM to isolate CSS
  - Result: More accurate preview showing what patrons actually see

  Bug 41053 (This bug) - Changed the iframe to a hidden div
  - Problem: Browser search (Ctrl+F) couldn't find text in the iframe's srcdoc attribute
  - Solution: Changed from iframe to <div style="display:none;"> with content as innerHTML
  - Benefit: Search now works again - users can search notice content
  - Note: Shadow DOM is still used when displaying the modal, maintaining CSS isolation

  Potential Disadvantages of the Current Approach

  1. Loss of HTML Isolation in the DOM
  The iframe provided complete sandboxing - malformed HTML couldn't escape and affect the page. With a div, even though the content is HTML-escaped ([% QUEUED_MESSAGE.html_content | html %]), there's theoretically more risk if:
  - The Template Toolkit escaping fails
  - There's a bug in the escaping mechanism
  - Extremely malformed HTML somehow breaks through

  2. Security Considerations
  - The iframe's srcdoc provided an additional layer of XSS protection
  - The current approach relies on Template Toolkit's | html filter being robust

All in all, I "think" the Change is Likely Safe, but i want to test with some really malformed notices before passing qa
Comment 7 Martin Renvoize (ashimema) 2025-11-13 09:25:40 UTC
OK.. I've tried throwing in a bunch of weird and wonderful html in the welcome notice and sent it to my test user.. so far I'm unable to break the display in chrome..

I reckon lets go with this.
Comment 8 Martin Renvoize (ashimema) 2025-11-13 09:25:46 UTC
Created attachment 189535 [details] [review]
Bug 41053: Move notice contents to hidden div

With thanks to Jonathan for suggesting this direction.

Test plan:
Go to patron detail, notices tab for a patron with some notices.
Look for a string that only occurs in the notice text of some notice.
Without this patch, you should not find that one.
With this patch, you should find it.
Manipulate the text of a notice to check display of bad html. (Click title.)
* Like: update message_queue set content='<p>test</p><a href="' where message_id=X;

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 9 Lucas Gass (lukeg) 2025-11-17 22:16:56 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 10 Paul Derscheid 2025-11-25 16:31:12 UTC
Nice work everyone!

Pushed to 25.05.x
Comment 11 Fridolin Somers 2025-12-02 10:56:34 UTC
Depends on Bug 39410 not in 24.11.x