[Bug 22551] New: Stray "//" appears at bottom of opac-detail.tt
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Bug ID: 22551 Summary: Stray "//" appears at bottom of opac-detail.tt Change sponsored?: --- Product: Koha Version: 18.11 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: ephetteplace@cca.edu QA Contact: testopia@bugs.koha-community.org Target Milestone: --- Created attachment 86822 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86822&action=edit screenshot of stray characters In the "SocialNetworks" JavaScript area of the OPAC display, there's this code (currently lines 1390-1393 of opac-detail.tt): //<![CDATA[ {lang: '[% lang | html %]'} //]]> </script> It seems like an opening <script> tag is missing (the closing one is orphaned) which results in the "//" characters displaying on the page towards the very bottom (see screenshot). It's not 100% clear to me what this code was intended to do but I think it was meant to be part of the old Google+ (removed from Koha earlier this year) script tag like this: <script src="https://apis.google.com/js/plusone.js" type="text/javascript"> //<![CDATA[ {"lang":"en-US"} //]]> </script> However, right now it's only effect is the stray "//"—the JS code is commented out and even if it wasn't I don't think declaring {"lang": "en-US"} as an object literal unassigned to any variable has any effect. I can make a patch for this shortly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Eric Phetteplace <ephetteplace@cca.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |ephetteplace@cca.edu Patch complexity|--- |Trivial patch CC| |ephetteplace@cca.edu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 --- Comment #1 from Eric Phetteplace <ephetteplace@cca.edu> --- Created attachment 86824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86824&action=edit patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Eric Phetteplace <ephetteplace@cca.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #2 from Owen Leonard <oleonard@myacpl.org> --- I think you're right to identify that section as obsolete, but I don't think that's what's causing the problem. There is a stray "//]]>" further down in the template. A "//<![CDATA[" marker was removed without cleaning up the ending marker. Please also take a look at our guidelines for commit messages: https://wiki.koha-community.org/wiki/Commit_messages -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 --- Comment #3 from Eric Phetteplace <ephetteplace@cca.edu> --- I do see that other stray "//]]>" around line 1720 of opac-detail.tt but it is not what's causing the text to display—inspect an opac-detail page in your web browser, the segment I removed (just above an orphaned closing </script> tag and the Twitter script) falls outside any <script> tags which is why the browser renders it as a text node. That second stray "//]]>" is inside a very long <script> tag with several functions (OpacHighlightedWords, OPACShelfBrowser,IDreamBooksReviews, etc.) and thus not rendered as text. In any case, I can re-submit a patch that removes both since that seems to be the right thing to do. It's not actually clear to me what's wrong with my commit message...I can be more descriptive but I thought I stated the bug number/name in the subject and outlined the testing plan already. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Eric Phetteplace <ephetteplace@cca.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86824|0 |1 is obsolete| | --- Comment #4 from Eric Phetteplace <ephetteplace@cca.edu> --- Created attachment 87568 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=87568&action=edit patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Eric Phetteplace <ephetteplace@cca.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Hayley Mapley <hayleymapley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hayleymapley@catalyst.net.n | |z --- Comment #5 from Hayley Mapley <hayleymapley@catalyst.net.nz> --- (In reply to Eric Phetteplace from comment #3)
In any case, I can re-submit a patch that removes both since that seems to be the right thing to do. It's not actually clear to me what's wrong with my commit message...I can be more descriptive but I thought I stated the bug number/name in the subject and outlined the testing plan already.
It doesn't look like the bug number or description is in the first line of your patches, it should be. Also the patch should be descriptive, the commit you attached was: 'patch' It should be: 'Bug 22551: Removed stray characters from opac-detail.tt There are stray characters, for the reasons outlined in the bug report. This patch fixes that. Test plan: 1) Note the extra characters on the template 2) Apply the patch 3) Verify the characters have disappeared' or something similar. Not a dig, just trying to make what Owen said clearer :) I will test this patch now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 --- Comment #6 from Hayley Mapley <hayleymapley@catalyst.net.nz> --- (In reply to Hayley Mapley from comment #5)
(In reply to Eric Phetteplace from comment #3)
In any case, I can re-submit a patch that removes both since that seems to be the right thing to do. It's not actually clear to me what's wrong with my commit message...I can be more descriptive but I thought I stated the bug number/name in the subject and outlined the testing plan already.
It doesn't look like the bug number or description is in the first line of your patches, it should be. Also the patch should be descriptive, the commit you attached was:
'patch'
It should be: 'Bug 22551: Removed stray characters from opac-detail.tt
There are stray characters, for the reasons outlined in the bug report. This patch fixes that.
Test plan: 1) Note the extra characters on the template 2) Apply the patch 3) Verify the characters have disappeared'
or something similar.
Not a dig, just trying to make what Owen said clearer :) I will test this patch now (In reply to Eric Phetteplace from comment #3) I do see that other stray "//]]>" around line 1720 of opac-detail.tt but it is not what's causing the text to display—inspect an opac-detail page in your web browser, the segment I removed (just above an orphaned closing </script> tag and the Twitter script) falls outside any <script> tags which is why the browser renders it as a text node. That second stray "//]]>" is inside a very long <script> tag with several functions (OpacHighlightedWords, OPACShelfBrowser,IDreamBooksReviews, etc.) and thus not rendered as text.
In any case, I can re-submit a patch that removes both since that seems to be the right thing to do. It's not actually clear to me what's wrong with my commit message...I can be more descriptive but I thought I stated the bug number/name in the subject and outlined the testing plan already.
(In reply to Hayley Mapley from comment #5)
(In reply to Eric Phetteplace from comment #3)
In any case, I can re-submit a patch that removes both since that seems to be the right thing to do. It's not actually clear to me what's wrong with my commit message...I can be more descriptive but I thought I stated the bug number/name in the subject and outlined the testing plan already.
It doesn't look like the bug number or description is in the first line of your patches, it should be. Also the patch should be descriptive, the commit you attached was:
'patch'
It should be: 'Bug 22551: Removed stray characters from opac-detail.tt
There are stray characters, for the reasons outlined in the bug report. This patch fixes that.
Test plan: 1) Note the extra characters on the template 2) Apply the patch 3) Verify the characters have disappeared'
or something similar.
Not a dig, just trying to make what Owen said clearer :) I will test this patch now
Ahh I see. Did you remove the description when you attached the commit? I see it now that I am amending to your commit that your commit message is perfect. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551 Hayley Mapley <hayleymapley@catalyst.net.nz> 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=22551 --- Comment #7 from Hayley Mapley <hayleymapley@catalyst.net.nz> --- Created attachment 88041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88041&action=edit Bug 22551: Stray "//" appears at bottom of opac-detail.tt Removes a couple stray "//<
bugzilla-daemon＠bugs.koha-community.org