Highlighted terms include code in traditional and bootstrap catalogue

Bug #1923225 reported by Jennifer Pringle
54
This bug affects 10 people
Affects Status Importance Assigned to Milestone
Evergreen
High
Unassigned
3.5
High
Unassigned
3.6
High
Unassigned

Bug Description

Evergreen 3.7beta with Bootstrap disabled

When a record is retrieved via ISBN Numeric Search in the both the OPAC and traditional catalogue in the staff client the ISBN includes code under Record Details.

ISBN: <b class='oils_SH identifier isbn'>9780358315070</b>

This does not happen on the Patron View in the new catalogue.

This and the equivalent doesn't occur when doing a Numeric Search for item barcode, call number, ISSN, or UPC.

Revision history for this message
Jennifer Pringle (jpringle-u) wrote :
description: updated
Revision history for this message
Christine Morgan (cmorgan-z) wrote :

Also seeing this display in 3.6.3.

Changed in evergreen:
status: New → Confirmed
Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

This is likely a side effect of the fix for bug 1902965.

Revision history for this message
Christine Morgan (cmorgan-z) wrote :

We are seeing code displayed around highlighted search terms in the record display.

With highlighting enabled, if I search for "Harry Potter and the" I see the following display of subject headings:
<b class='oils_SH subject complete'>Potter</b>, <b class='oils_SH subject complete'>Harry</b> (Fictitious character) -- Drama

Wizards -- England -- Drama

Magic -- England -- Drama

Hogwarts School of Witchcraft <b class='oils_SH subject complete'>and</b> Wizardry (Imaginary place) -- Drama

Schools -- England -- Drama

The summary note displays as:
Rescued from <b class='oils_SH keyword abstract'>the</b> outrageous neglect of his aunt <b class='oils_SH keyword abstract'>and</b> uncle, a young boy with a great destiny proves his worth while attending Hogwarts School for Wizards <b class='oils_SH keyword abstract'>and</b> Witches.

If I then click the "Disable Highlighting'' button, the display is normal.

We are also seeing this issue in the Bootstrap catalog.

Changed in evergreen:
assignee: nobody → Jeff Davis (jdavis-sitka)
Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

Working branch user/jeffdavis/lp1923225-escape-display-fields has a potential fix (top two commits):

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1923225-escape-display-fields

The first commit avoids over-escaping ISBNs and ISSNs when display fields are used in the TPAC and the Bootstrap OPAC. The second commit handles the issue in comment #4, where display fields display incorrectly when run through the accordion macro without being truncated (i.e. that field can have a "Read More" link, but either the field is too short to need it or else truncation is disabled).

Changed in evergreen:
assignee: Jeff Davis (jdavis-sitka) → nobody
tags: added: pullrequest
Changed in evergreen:
assignee: nobody → Evergreen Bug Maintenance (bugmaster)
Revision history for this message
Galen Charlton (gmc) wrote :

The patches as written didn't seem to fix what Christine identified in #4, but I've written a follow-up that has the underlying stored procedures that generate the display field values (highlighted and not highlighted) do HTML-escaping lower down the stack. Patches, including my signoffs on Jeff's patches, are found in:

collab/gmcharlt/lp1923225-more-html-escaping-tweaks

Revision history for this message
Galen Charlton (gmc) wrote :

By the way, I don't think the patch is remotely perfect; for example, subject search links that contain HTML tags would be broken (which we might not care much about), but also ones with ampersands (which we might care about).

For reference, here is the test record I've been using:

=LDR 00574nam a2200205 a 4500
=001 2
=003 CONS
=005 19970807205236.0
=008 970701s1978\fr\u000\0\fre\u
=020 \\$a2130354467 <script>alert('ISBN!')</script>
=050 00$aML1263$b.F47
=100 1\$aFerchault, Guy <script>alert('author!')</script>
=245 03$aLe concerto <script>alert('title!')</script> and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff and stuff <script>alert('extended title!')</script> /$cGuy Ferchault.
=250 \\$a1st ed. <script>alert('edition!')</script>
=260 \\$aParis :$bPresses universitaires de France <script>alert('publisher!')</script>,$c1978.
=300 \\$a127 p. :$bmusique
=490 \\$aQue sais-je? <script>alert('series!')</script>;$v1717.
=500 \\$aVery scary <script>alert('note!')</script>
=504 \\$aBibliogr.
=650 \6$aConcerto <script>alert('subject!')</script>$xFrance <script>alert('subdivision!')</script>
=650 \6$aStuff & nonsense
=856 41$uhttps://www.evergreen-ils.org/?arg1=foo&arg2=bar$yEvergreen <script>alert('URL label!')</script>
=901 \\$a2$bAUTOGEN$c2$tbiblio

Changed in evergreen:
importance: Undecided → Medium
Revision history for this message
Blake GH (bmagic) wrote :

Adding screenshot with subject example

Changed in evergreen:
assignee: Evergreen Bug Maintenance (bugmaster) → Jeff Davis (jdavis-sitka)
Revision history for this message
Michele Morgan (mmorgan) wrote :

Adding a screenshot of the full record in the bootstrap opac retrieved using keyword search terms 'harry potter'

Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

I've pushed an additional commit to the collab branch that cleans up double-escaping of HTML in a few places:

1. Under "Search for related items by series" in the Bootstrap OPAC and TPAC.
2. "Summary, etc." in the Bootstrap OPAC only.

To confirm the issue and fix, try a search for "harry potter" with the Concerto dataset, view the record for "Harry Potter and the Goblet of Fire" with highlighting enabled, and compare the affected fields before and after applying the fix.

Changed in evergreen:
assignee: Jeff Davis (jdavis-sitka) → nobody
Changed in evergreen:
milestone: none → 3.7.1
Revision history for this message
Michele Morgan (mmorgan) wrote :

I'm adding my signoff to this. I've tested the tpac and bootstrap opac on a master Concerto system with a few additional bib records added for testing. I have seen no issues. It's an enormous improvement in the patron facing catalog.

I'd vote for merging and dealing with any remaining issues in a separate bug.

I pushed my signoff here:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mmorgan/lp1923225_signoff

tags: added: signedoff
Michele Morgan (mmorgan)
summary: - ISBN display includes code in traditional catalogue
+ Highlighted terms include code in traditional and bootstrap catalogue
Changed in evergreen:
importance: Medium → High
Galen Charlton (gmc)
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

Pushed all the way back to rel_3_5. Thanks, Jeff and Michele!

I do expect that we'll need to do a bit more whack-a-mole, but I think we're close.

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers