Bug 5738 - Display the URI in 856u as an Image
Summary: Display the URI in 856u as an Image
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: 3.4
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Deadline: 2011-02-15
Assignee: Doug Dearden
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-10 20:53 UTC by Doug Dearden
Modified: 2012-10-25 23:03 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Patch file - first attempt (12.60 KB, patch)
2011-02-10 22:54 UTC, Doug Dearden
Details | Diff | Splinter Review
Patch changes to Results and Details (12.74 KB, patch)
2011-02-11 22:34 UTC, Doug Dearden
Details | Diff | Splinter Review
Coding work done ready for QA testing (31.78 KB, application/octet-stream)
2011-02-15 17:48 UTC, Doug Dearden
Details
Patch with further refinements (26.18 KB, patch)
2011-03-14 20:38 UTC, Doug Dearden
Details | Diff | Splinter Review
Text file with documentation example (edit at will). (1.05 KB, text/plain)
2011-03-14 20:49 UTC, Doug Dearden
Details
Implements Jared's MIME type suggestion (15.51 KB, patch)
2011-03-17 15:11 UTC, Doug Dearden
Details | Diff | Splinter Review
Non-english system pref files (9.13 KB, patch)
2011-03-18 21:28 UTC, Doug Dearden
Details | Diff | Splinter Review
system pref files - all (12.69 KB, patch)
2011-03-28 21:16 UTC, Doug Dearden
Details | Diff | Splinter Review
Signed-off and squashed patch (36.09 KB, patch)
2011-03-30 18:45 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Reposting Jared's earlier patch - no changes made, signed off (36.13 KB, patch)
2011-03-31 21:27 UTC, Doug Dearden
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Doug Dearden 2011-02-10 20:53:19 UTC
Add an option to the OPAC to display the URI that is stored in the 856u field as an image instead of a text link.  This would be implemented in the OPAC XSLT style sheets for the Results and Details pages, and would require that the corresponding OPAC XSLT option be set.

This enhancement is designed to enable the staff to catalog images, then have them appear on the resulting pages in the OPAC when searching.  Clicking on the image would open it full size.  If the image option is in use, then the default link text would be disabled.  Custom text would still appear if entered in the "URLLinkText" option.  

Add system preference "Display856uAsImage" with options of Results page, Details page, Both Results and Details pages, Neither Results or Details page.
Comment 1 Doug Dearden 2011-02-10 22:54:48 UTC Comment hidden (obsolete)
Comment 2 Ian Walls 2011-02-10 23:47:07 UTC
I don't think this should be controlled by a system preference.  The MARC21 definition for 856 includes subfield q, Electronic format type. (http://www.loc.gov/marc/bibliographic/bd856.html)  If this is set to 'img', 'jpg', 'jpeg', 'png' or any other image type, the link should be displayed as an image, otherwise as a hyperlink.

I can work up a script to automatically add 856$q to a group of biblios, in support of this (since I assume most of your imported records would not have the subfield, given it's obscurity).  Doing it this way puts the granularity of this option at the biblio level, rather than the system level.
Comment 3 Doug Dearden 2011-02-11 16:40:53 UTC
I like Ian Walls' suggested change, but have some implementation questions.  Would we settle on a list of allowable entries for the 856q field?  There are a LOT of image file extensions, some old ones that may be in limited use but still out there, and we know there will be new ones in the future.  Would it make sense to just use "image" or "img" in the q subfield for any image file then trigger off that?  Or do we work up a list of the common image types and use that list only?

Also, I think it makes sense to leave the system preference in as well, as depending on where the images are loading from and the number of hits on a search this can effect page loading speed, particularly on the results page.  The system preference would allow control over whether the attempt is even made to display images on the results page thus keeping it from being sluggish.

I will add code to check the 856q subfield for 'img' for now.
Comment 4 Ian Walls 2011-02-11 16:54:22 UTC
856$q is free text, technically speaking, but by convention, it's often either a file extension or a MIME type, I believe.  To provide better control, we may want to associate this subfield with authorised value list of MIME types, so we'll have a smaller pool of options to check.

I'd want to get some kind of real-world feedback on what values are actually winding up in MARC records' 856$q subfields, but limiting things to MIME type for now may be the way to go.

I'll agree on the system preferences; perhaps for each of the four values (OPAC v. Staff client X results v. details), have the following options:

1. show hyperlink
2. show image in OPAC
3. show image in AJAXy lightbox kinda deal on click\

Admittedly, the last one could be accomplished using 1. and some JQuery, but having it build in would be nice for libraries
Comment 5 Doug Dearden 2011-02-11 21:45:41 UTC
I've been working on this today, and have added code to check the 856q field for any of the MIME image extensions.  A rather long "or" string, the only alternative I could think of being another "choose" list which didn't seem much better.  I also moved some things around to make it a little cleaner, and changed it so if an image is displayed no link text is displayed, including the custom text in the system preferences.  So far I have only worked on the OPAC side of things, but can add the code to the staff client side once things are settled out.  Also I've done the MARC21 version of Display and Results, but will add the feature into the unicode versions also.

Ian - I've got 1 and 2 in your list working, but have no idea how to implement "3. show image in AJAXy lightbox kinda deal on click" :)  There isn't much lag around clicking on the image though.  Where I saw performance issues was loading the Results page when I got 75+ hits with multiple images attached to each one.  Also I should mention that my test machine for this is an old 486, so this may not be much of an issue with some good hardware.

If you want to add the AJAXy lightbox let me know.  If you can work up the script you mentioned that would populate the 856q subfield based on the contents of the 856u subfield that would be great too.
Comment 6 Doug Dearden 2011-02-11 22:34:36 UTC Comment hidden (obsolete)
Comment 7 Doug Dearden 2011-02-11 22:50:21 UTC
I have a small (5 entry) database that I created for testing.  I did a dump of it using mysqldump then tried to attach it here but was unsuccessful - not allowed because it is not the right type.  If you want it for testing let me know and I'll email it to you.
Comment 8 Doug Dearden 2011-02-15 17:48:05 UTC Comment hidden (obsolete)
Comment 9 MJ Ray (software.coop) 2011-03-06 12:10:08 UTC
This bug is mentioned in:
bug 5738 Display the URI in 856u as an Image	- Enhancement http://lists.koha-community.org/pipermail/koha-patches/2011-February/013828.html
bug 5738 Display the URI in 856u as an Image	- Enhancement http://lists.koha-community.org/pipermail/koha-patches/2011-February/013854.html
Comment 10 Doug Dearden 2011-03-14 20:38:24 UTC Comment hidden (obsolete)
Comment 11 Doug Dearden 2011-03-14 20:49:23 UTC
Created attachment 3297 [details]
Text file with documentation example (edit at will).
Comment 12 Jared Camins-Esakov 2011-03-15 20:06:21 UTC
I haven't tested this yet, but I have a comment. This initial patch doesn't seem to support MIME types (e.g. "image/jpeg") in the subfield 'q'. Is there a specific reason why you feel that this feature shouldn't be activated by "image/jpeg", or would you be open to a follow-up patch?

Regards,
Jared
Comment 13 Doug Dearden 2011-03-16 15:06:19 UTC
(In reply to comment #12)
> I haven't tested this yet, but I have a comment. This initial patch doesn't
> seem to support MIME types (e.g. "image/jpeg") in the subfield 'q'. Is there a
> specific reason why you feel that this feature shouldn't be activated by
> "image/jpeg", or would you be open to a follow-up patch?
> 
> Regards,
> Jared

Jared - The image display is triggered off a MIME extension in 856q ("jpg", not "image/jpeg").  Currently it is looking for one of 23 MIME extensions - bmp, cod, gif, ief, jpe, jpeg, jpg, jfif, png, svg, tif, tiff, ras, cmx, ico, pnm, pbm, pgm, ppm, rgb, xbm, xpm, xwd .  I also added a generic "img" option.  I am fine with the feature being activated in whatever way the community feels is valid, but kept it to the MIME extensions for a couple of reasons.  First this creates a rather long "or" statement in the code that will get even longer if we add image/jpeg, image/gif, etc.  Secondly Ian Walls and I communicated about the need for a script that would populate the 856q field based on the value in the 856u field, and my thought on that was to check the file extension at the end of the URI in the 856u field, and if it is one of the 23 MIME extensions copy it into the 856q field (if empty).  

I suppose we could use MIME types instead of MIME extensions, but I don't see any reason to use both.
Comment 14 Jared Camins-Esakov 2011-03-16 16:19:18 UTC
Doug,

Would a follow-up patch (which I would be happy to do) activating the feature if the 856q started with "image/"  make sense to you?

Regards,
Jared
Comment 15 Doug Dearden 2011-03-16 19:26:33 UTC
Jared - I will add that (adding the substring 'image/' to the "or" statement will be easy enough).  Look for that sometime tomorrow.  If you could then test that would be great.

Also, my Perl skills are limited.  Would you be interested in working up a script to populate the 856q field based on what is in the 856u field?

Thanks,

Doug
Comment 16 Doug Dearden 2011-03-17 15:11:17 UTC Comment hidden (obsolete)
Comment 17 Jared Camins-Esakov 2011-03-18 17:41:10 UTC
Doug,

I tested it, and I wasn't able to get the images to display on the OPAC Results page or Intranet Details page, even with the corresponding XSLT enabled. I was testing with this record: demo.cpbibliography.com/cgi-bin/koha/opac-detail.pl?biblionumber=1 (at present, that server does not display any images). I'm not sure what I'm doing wrong. Is there a problem with the record that I'm not seeing? It looks *fantastic* on the OPAC Details page.

Also, you seem to be missing non-English sysprefs.

Regards,
Jared
Comment 18 Doug Dearden 2011-03-18 20:08:55 UTC
(In reply to comment #17)

Jared,

Try changing both the Staff Client and OPAC sysprefs to "neither", save them, then change them back to "both" and see if that changes the result.  I have updatedatabase.pl set to originally set these to OFF (neither results or details page) but alphabetically the "both" option comes first, and I think it displays as the default after updatedatabase.pl is run.  But really both are still off.  I'm not sure of the best way to fix that.  Maybe initially set them to "both", but I didn't want to presume that these should be on until the user turned them on.

Also, did you rebuild Zebra?  I think the results page doesn't work until after a Zebra rebuild.

Just to be clear, for the image to show up instead of the text link you have to have the corresponding XSLT option on, the Display856uAsImage and/or OPACDisplay856uAsImage system pref turned on, a valid link in the 856u field and a valid MIME extension or MIME type in the corresponding 856q field (jpg or image/jpeg should make it work, an empty 856q or some invalid text like "not" should cause the alternative text link to be displayed). 

Regarding the sysprefs.sql file, the doc I was reading on how to do a good patch mentioned the one file in ../installer/data/mysql/en/mandatory but not any other languages.  (I can't figure out where that guide is now, somewhere on the web site I thought but I have no idea where).  When I look in the other folders in ../installer/data/mysql there are sysprefs.sql files but the contents seem to be in english.  Do I just need to add the same SQL statements into these as well?  (If so, why are we duplicating that if it always in english, why not just use one file?)  Is there a translation file somewhere that needs to be updated?  Need some guidance.

Thanks,

Doug
Comment 19 Doug Dearden 2011-03-18 20:26:52 UTC
I found the info on the systemprefs - I'll take care of that.

Doug
Comment 20 Doug Dearden 2011-03-18 21:28:42 UTC Comment hidden (obsolete)
Comment 21 Jared Camins-Esakov 2011-03-22 12:15:42 UTC
Doug,

I am unable to apply the third patch:

Commit Body is:
--------------------------
Bug 5738 Display 856u as image - system prefs
--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Bug 5738 Display 856u as image - system prefs
fatal: sha1 information is lacking or useless (installer/data/mysql/de-DE/mandatory/sysprefs.sql).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 5738 Display 856u as image - system prefs
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

There is also some rebasing needed for the first patch, but I was able to do that no problem, and once I figure out why the images will only display in the OPAC Details page, I will reupload it.

Regards,
Jared
Comment 22 Doug Dearden 2011-03-28 21:16:12 UTC Comment hidden (obsolete)
Comment 23 Jared Camins-Esakov 2011-03-30 18:45:41 UTC Comment hidden (obsolete)
Comment 24 Doug Dearden 2011-03-31 21:27:59 UTC
Created attachment 3591 [details] [review]
Reposting Jared's earlier patch - no changes made, signed off

I have checked Jared's changes on this and it is working as designed so am attaching the patch and obsoleting the earlier one per his instructions, though no changes were made.  Signed off as far as I'm concerned

Doug
Comment 25 Chris Cormack 2011-04-01 00:42:00 UTC
Pushed, please test
Comment 26 Nicole C. Engard 2011-04-01 12:51:53 UTC
The default value for these new preferences should be 'neither' but it appears instead to be the first value in each pull down menu.
Comment 27 Nicole C. Engard 2011-04-01 12:52:36 UTC
PS. that was on an upgrade - but of course it should be the default in upgrades and new installs.
Comment 28 Nicole C. Engard 2011-04-01 12:55:00 UTC
Also can someone please give me a test case/documentation example? The one line preference is not enough for me to understand what this does? Can someone give me the marc fields I should fill in and what vaues they should have for this work?

Nicole
Comment 29 Doug Dearden 2011-04-01 16:24:11 UTC
Nicole,

To test:

Create a new (or edit an existing) record, adding an 856u field and populating it with a valid reference to an image file.  For example: http://www.publicdomainpictures.net/pictures/9000/nahled/mountain-stream-25271280314836ZDa9.jpg .  In addition to the 856u field, populate the 856q field with a valid mime extension (see list above in comment 13) or a valid mime type that starts with "image/", i.e. "image/jpeg".  To attache multiple images to the same record, repeat the 856 field and populate the u and q subfields as above.  Do not repeat the u and q subfields within the same 856 field, it won't work (known bug 4958).  In the system preferences under the OPAC and Staff Client sections turn on the XSLT options.  When the new system prefs are set to Both, Details only or Results only you should see an image displayed instead of the link text.  When you click on the image it should open to full size, in the current window or in a new window depending on the value in the system pref OPACURLOpenInNewWindow.  You may need to rebuild Zebra before it works.  

To summarize - XSLT on, new system pref on, 856u has valid picture, 856q has valid entry.  

If the 856u doesn't point to an image file, the alternative text will display instead.  If the 856q doesn't have a valid mime extension or type the alternative text will display instead.

Regarding the default being "neither".  In the updatedatabase.pl and sysprefs.sql file, the default is set to 'off' (which should translate to neither).  The system prefs do show up with Both enabled after update.  I think this can be fixed by putting a default in the .pref files, as it looks like it is reordering the preferences alphabetically then using the first one.  The wiki page at http://wiki.koha-community.org/wiki/System_Preferences mentions that there is a way to do this but does not give an example, and I did not find any other entries in the .pref files that seemed to have defaults defined.  Also, while "Both" appears after the new system pref is entered, I'm not sure it is in that state.  You may need to change it, save, then change it back to get "Both" turned on as the choice.

Thanks,

Doug
Comment 30 MJ Ray (software.coop) 2011-04-04 08:09:54 UTC
This bug is mentioned in:
Subject should have been - Bug 5738 Display 856u as Image - Need sign off http://lists.koha-community.org/pipermail/koha-patches/2011-March/014057.html
Bug 5738 - Display 856u as Image - recent suggestion implemented, need signoff http://lists.koha-community.org/pipermail/koha-patches/2011-March/014115.html
Bug 5738 Display 856u As Image http://lists.koha-community.org/pipermail/koha-patches/2011-March/014365.html
Bug 5738 Display 856u As Image http://lists.koha-community.org/pipermail/koha-patches/2011-March/014414.html