[Bug 12123] New: HTML notices can break the notice viewer
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Bug ID: 12123 Summary: HTML notices can break the notice viewer Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27402 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27402&action=edit Bug 12123 - HTML notices can break the notice viewer -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- I do not consider this patch to be an optimal solution, but it -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27402|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 27403 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=27403&action=edit Bug 12123 - HTML notices can break the notice viewer -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 jdemuth@roseville.ca.us <jdemuth@roseville.ca.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jdemuth@roseville.ca.us -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- Please provide a proper commit message with a test plan. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #27403|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 33863 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33863&action=edit Bug 12123 - HTML notices can break the notice viewer 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. Test Plan: 1) Set your CHECKOUT notice to the following ( with HTML Message checked ) : <p>The following items have been checked out:</p> ---- <blockquote> <<biblio.title>> </blockquote> ---- <p>Thank you for visiting the <<branches.branchname>> of HMCPL.</p> 2) Check out some items to a patron 3) View the patrn's notices 4) Note the notice viewer is broken ( message is not collapsed, and con't be collapsed ). 5) Apply this patch 6) Reload the page 7) Note the notice viewer is no longer broken -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33863|0 |1 is obsolete| | --- Comment #6 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 33876 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33876&action=edit [SIGNED-OFF] Bug 12123 - HTML notices can break the notice viewer 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. Test Plan: 1) Set your CHECKOUT notice to the following ( with HTML Message checked ) : <p>The following items have been checked out:</p> ---- <blockquote> <<biblio.title>> </blockquote> ---- <p>Thank you for visiting the <<branches.branchname>> of HMCPL.</p> 2) Check out some items to a patron 3) View the patrn's notices 4) Note the notice viewer is broken ( message is not collapsed, and con't be collapsed ). 5) Apply this patch 6) Reload the page 7) Note the notice viewer is no longer broken Signed-off-by: Owen Leonard <oleonard@myacpl.org> I followed the test plan successfully. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33882 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33882&action=edit Screenshot I am not sure about this - it seems to work, but should it look like on the screenshot? Do we really need an iframe here? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33876|0 |1 is obsolete| | --- Comment #8 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 33885 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33885&action=edit Bug 12123 - [Alternative patch] HTML notices can break the notice viewer 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. Also changes the layout of the table a bit to be a little more user friendly. Test Plan: 1) Set your CHECKOUT notice to the following ( with HTML Message checked ) : <p>The following items have been checked out:</p> ---- <blockquote> <<biblio.title>> </blockquote> ---- <p>Thank you for visiting the <<branches.branchname>> of HMCPL.</p> 2) Check out some items to a patron 3) View the patrn's notices 4) Note the notice viewer is broken ( message is not collapsed, and con't be collapsed ). 5) Apply this patch 6) Reload the page 7) Note the notice viewer is no longer broken -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33885|0 |1 is obsolete| | --- Comment #9 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 33888 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33888&action=edit Bug 12123 - [Alternative patch] HTML notices can break the notice viewer 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. Test Plan: 1) Set your CHECKOUT notice to the following ( with HTML Message checked ) : <p>The following items have been checked out:</p> ---- <blockquote> <<biblio.title>> </blockquote> ---- <p>Thank you for visiting the <<branches.branchname>> of HMCPL.</p> 2) Check out some items to a patron 3) View the patrn's notices 4) Note the notice viewer is broken ( message is not collapsed, and con't be collapsed ). 5) Apply this patch 6) Reload the page 7) Note the notice viewer is no longer broken -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33885|1 |0 is obsolete| | --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 33885 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33885 Bug 12123 - [Alternative patch] HTML notices can break the notice viewer Review of attachment 33885: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=12123&attachment=33885) ----------------------------------------------------------------- I definitely prefer Liz's patch over Kyle's. I don't think iframes are needed at all. It looks like the problem is that we were trying to stuff HTML into a "p" element which couldn't take them as children. The only thing stopping my sign off would be the inline CSS. I find the CSS isn't necessary when I test it, and I suppose if it is necessary for other browsers, I would just prefer it to be in a stylesheet somewhere else so we can control our styles better. ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt @@ +41,4 @@
<h1>Sent notices for [% INCLUDE 'patron-title.inc' %]</h1>
[% IF ( QUEUED_MESSAGES ) %] + <table id="noticestable" style="table-layout: fixed">
Using Chrome on Windows, I don't find this CSS does anything. If anything, it makes the presentation slightly worse. However, if we do include it, my only request would be to not use inline CSS and to put this in a separate stylesheet somewhere like staff-global.css or something like that. @@ +55,4 @@
<tr> <td> <a class="notice-title" href="#">[% QUEUED_MESSAGE.subject %]</a> + <div class="notice">
Switching to a div fixes this problem nicely. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33885|0 |1 is obsolete| | Attachment #33888|0 |1 is obsolete| | --- Comment #11 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 33950 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33950&action=edit [SIGNED-OFF] Bug 12123 - [Alternative patch] HTML notices can break the notice viewer 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. Test Plan: 1) Set your CHECKOUT notice to the following ( with HTML Message checked ) : <p>The following items have been checked out:</p> ---- <blockquote> <<biblio.title>> </blockquote> ---- <p>Thank you for visiting the <<branches.branchname>> of HMCPL.</p> 2) Check out some items to a patron 3) View the patrn's notices 4) Note the notice viewer is broken ( message is not collapsed, and con't be collapsed ). 5) Apply this patch 6) Reload the page 7) Note the notice viewer is no longer broken Signed-off-by: Owen Leonard <oleonard@myacpl.org> This patch appears to fix the problem with the minimum required change. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #12 from David Cook <dcook@prosentient.com.au> --- Thinking again... I can see the merit of using an iframe instead of a div. After all, the div only works because "p" and "blockquote" can be children of a "div". If a person put "html" and "body" tags into the message, those the HTML would be invalid. *does a test* Mmm, instead of busting the notice viewer, Chrome removes the "html" and "body" tags. (So the viewer continues to work fine, although you'll still notice the "html" and "body" tags in the message_queue.) I've noticed libraries put "html" and "body" tags in their notices before, which is why this comes into my mind. Chrome is pretty rad at cleaning up tag soup, but I'm not sure about Firefox or Internet Explorer... Microsoft Outlook seems capable of handling a HTML email with "html" and "body" tags inside of a "body" tag. I suppose most email clients are pretty flexible when it comes to invalid HTML markup. In any case, I think Liz's solution works. If any problems arise in the future, I suppose we can address them then. (Or possibly strip "body" and "html" tags when saving notice templates.) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33950|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 33976 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33976&action=edit [PASSED QA] Bug 12123 - [Alternative patch] HTML notices can break the notice viewer 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. Test Plan: 1) Set your CHECKOUT notice to the following ( with HTML Message checked ) : <p>The following items have been checked out:</p> ---- <blockquote> <<biblio.title>> </blockquote> ---- <p>Thank you for visiting the <<branches.branchname>> of HMCPL.</p> 2) Check out some items to a patron 3) View the patrn's notices 4) Note the notice viewer is broken ( message is not collapsed, and con't be collapsed ). 5) Apply this patch 6) Reload the page 7) Note the notice viewer is no longer broken Signed-off-by: Owen Leonard <oleonard@myacpl.org> This patch appears to fix the problem with the minimum required change. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works nicely, no problems found. Passes tests and QA script. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Kyle and Liz! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |chris@bigballofwax.co.nz --- Comment #15 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.18.x will be in 3.18.01 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12123 Tom Misilo <misilot@fit.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misilot@fit.edu -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org