[Bug 22551] New: Stray "//" appears at bottom of opac-detail.tt

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] New: Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
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: [hidden email]
          Reporter: [hidden email]
        QA Contact: [hidden email]
  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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Eric Phetteplace <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|[hidden email]         |[hidden email]
   Patch complexity|---                         |Trivial patch
                 CC|                            |[hidden email]

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

--- Comment #1 from Eric Phetteplace <[hidden email]> ---
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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Eric Phetteplace <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |Needs Signoff

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Katrin Fischer <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
           Severity|enhancement                 |normal

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Owen Leonard <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Needs Signoff               |Failed QA

--- Comment #2 from Owen Leonard <[hidden email]> ---
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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

--- Comment #3 from Eric Phetteplace <[hidden email]> ---
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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Eric Phetteplace <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #86824|0                           |1
        is obsolete|                            |

--- Comment #4 from Eric Phetteplace <[hidden email]> ---
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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Eric Phetteplace <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Needs Signoff

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Hayley Mapley <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
                   |                            |z

--- Comment #5 from Hayley Mapley <[hidden email]> ---
(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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

--- Comment #6 from Hayley Mapley <[hidden email]> ---
(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.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Hayley Mapley <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Needs Signoff               |Signed Off

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

--- Comment #7 from Hayley Mapley <[hidden email]> ---
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 "//<![CDATA[" like comments which are artifacts of other
code that since been removed. One of the comments causes two forward slashes to
appear at the bottom of each OPAC detail page.

Testing plan:

- enable SocialNetworks
- visit any OPAC detail page, observe "//" at bottom of page
- apply patch
- revisit page, slashes should be gone

Signed-off-by: Hayley Mapley <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

--- Comment #8 from Eric Phetteplace <[hidden email]> ---
Thank you, Hayley, that helps me understand what I did wrong. You're exactly
right—I didn't enter a description or "comment" when I selected "add an
attachment" in bugzilla because I thought the patch file itself was the only
thing that mattered; my patch looks the same as yours it's just I didn't fill
out the Bugzilla form correctly. I'll do that next time!

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |martin.renvoize@ptfs-europe
                   |                            |.com
  Attachment #87568|0                           |1
        is obsolete|                            |

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #88041|0                           |1
        is obsolete|                            |

--- Comment #9 from Martin Renvoize <[hidden email]> ---
Created attachment 88688
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=88688&action=edit
Bug 22551: Stray "//" appears at bottom of opac-detail.tt

Removes a couple stray "//<![CDATA[" like comments which are artifacts of other
code that since been removed. One of the comments causes two forward slashes to
appear at the bottom of each OPAC detail page.

Testing plan:

- enable SocialNetworks
- visit any OPAC detail page, observe "//" at bottom of page
- apply patch
- revisit page, slashes should be gone

Signed-off-by: Hayley Mapley <[hidden email]>
Signed-off-by: Martin Renvoize <[hidden email]>

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Martin Renvoize <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Passed QA

--- Comment #10 from Martin Renvoize <[hidden email]> ---
It's great to see such teamwork going on here guys, well done.

Patch works well and causes no regressions.. Passing QA

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

--- Comment #11 from Martin Renvoize <[hidden email]> ---
As an addendum to the advice given here, you might find git-bz helpful Eric.
https://wiki.koha-community.org/wiki/Git_bz_configuration

It's automates the creation and upload of patches to bugzilla (as well as
making the application of such patches to your development environment very
simple too)

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22551] Stray "//" appears at bottom of opac-detail.tt

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22551

Nick Clemens <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]
             Status|Passed QA                   |Pushed to Master

--- Comment #12 from Nick Clemens <[hidden email]> ---
Awesome work all!

Pushed to master for 19.05

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/