https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36111

Katrin Fischer <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA

--- Comment #44 from Katrin Fischer <[email protected]> ---
I am not sure about the proposed patches here as what they actually do and what
is advertised in the bug title doesn't match up.

1) This changes the current behavior beyond not displaying $h: 

We won't be displaying any "Online resources" info if there isn't a $u. The LOC
documentation lists a few examüles where we don't have a $u:

https://www.loc.gov/marc/bibliographic/bd856.html

856     1#$amaine.maine.edu$cMust be decompressed with PKUNZIP$fresource.zip
856     0#$akentvm.bitnet$facadlist file1$facadlist file2$facadlist file3
856     0#$auicvm.bitnet$fAN2
856     2#$amadlab.sprl.umich.edu$p3000
856     10$zFTP access to PostScript version includes groups of article files
with .pdf
extension$aftp.cdc.gov$d/pub/EIS/vol*no*/adobe$f*.pdf$qapplication/pdf 

So are we sure there are no unintended side effects of this change?

2) I am not sure what this line does - but including style inline is usually
not quite a good sign:

<xsl:attribute name="style">pointer-events: none; color:
#202020;</xsl:attribute>

3) This does look overly complicated:

     <xsl:if test="marc:datafield[@tag=856]">
+        <xsl:if test="marc:datafield[@tag=856]/marc:subfield[@code='u']">

In full:

    <xsl:if test="marc:datafield[@tag=856]">
        <xsl:if test="marc:datafield[@tag=856]/marc:subfield[@code='u']">
        <span class="results_summary online_resources">
                <span class="label">Online resources: </span>
                    <xsl:for-each select="marc:datafield[@tag=856]">


So: Check if 856 exists... check if at least one of those has a $u... then loop
over all 856 again. If we really want to limit display to only 856 with $u it
would make sense to check for at least one with... and then adjust the for-each
to only loop over those with $u.

4) This will just result in a broken link when there is no $u, it won't take
care of removing the whole link from the HTML. It's either not needed as we
check $u before, or it results in a broken link:

+                                <xsl:when test="marc:subfield[@code='u']">
                                     <xsl:text>Click here to access
online</xsl:text>
-                                </xsl:otherwise>
+                                </xsl:when>


5) If we only select the 856 with $u for looping, this change won't be
required. Also: it doesn't look right. Before we ask:

Is this the last, then show " ", otherwise print |.
Now it's turned into a really complex expression.

if we have (y or 3 or z or u) and (u) - at least one u too many:

-                <xsl:otherwise> | </xsl:otherwise>
+                <xsl:when test="(marc:subfield[@code='y'] or
marc:subfield[@code='3'] or marc:subfield[@code='z'] or
marc:subfield[@code='u']) and marc:subfield[@code='u']"><xsl:text> |
</xsl:text></xsl:when>

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://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 via email to