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.
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>
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>
I never knew that you could search by the content of the notice before. That's really useful. This works perfectly. Thanks!
(In reply to Brendan Lawlor from comment #2) > Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Great. Thx
*** Bug 41140 has been marked as a duplicate of this bug. ***
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
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.
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>
Nice work everyone! Pushed to main for 25.11
Nice work everyone! Pushed to 25.05.x
Depends on Bug 39410 not in 24.11.x