Summary: | Display the URI in 856u as an Image | ||
---|---|---|---|
Product: | Koha | Reporter: | Doug Dearden <dearden> |
Component: | OPAC | Assignee: | Doug Dearden <dearden> |
Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
Severity: | enhancement | ||
Priority: | PATCH-Sent (DO NOT USE) | CC: | chris, jcamins, kmkale, koha.sekjal, nengard |
Version: | 3.4 | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | Sponsored | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Deadline: | 2011-02-15 | ||
Attachments: |
Patch file - first attempt
Patch changes to Results and Details Coding work done ready for QA testing Patch with further refinements Text file with documentation example (edit at will). Implements Jared's MIME type suggestion Non-english system pref files system pref files - all Signed-off and squashed patch Reposting Jared's earlier patch - no changes made, signed off |
Description
Doug Dearden
2011-02-10 20:53:19 UTC
Created attachment 3127 [details] [review] Patch file - first attempt 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. 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. 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 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. Created attachment 3136 [details] [review] Patch changes to Results and Details Changes made to OPAC XSLT results and details files to incorporate checking the 856q field for a valid MIME image extension before displaying the URI in the 856u field as an image. 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. Created attachment 3158 [details]
Coding work done ready for QA testing
I have this working with the MARC21 display files for results and details on the OPAC. I also have it working for the Unimarc detail file on the OPAC. I was unable to get it working for the Unimarc results pages on the OPAC, as there is another conditional test around the 856u field looking for a ‘0’ in ind2. It is also labeling this area ‘Availability’ instead of ‘Online Resources’. I’m not sure what is going on there, but didn’t want to mess up something by changing that around so left well enough alone. In addition I have a new preference for the staff client – and renamed the OPAC preference to distinguish the two. I revised the pref and sql files accordingly. Per the current doc the xslt display file for Results is not working in the staff client, but I got the respective MARC21 and Unimarc intranet detail files working. As far as I can tell this is now ready for QA testing and hopefully inclusion in a future release. I will post the file up to the bug entry.
Still to be done – text to be included in the documentation, and a script that will populate the 856q field with a MIME extension based on the URI in the 856u field.
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 Created attachment 3295 [details] [review] Patch with further refinements I have reworked this after communicating with Ian Walls about design. There are two system preferences - OPACDisplay856uAsImage and Display856uAsImage . I have this working so the image processing takes precedence over the creation of text links for the URI stored in the 856u, but only if the appropriate system preference is turned on and there is a valid mime extension in the corresponding 856q field (Ian's suggestion there). Also, I changed this bug to be a 3.4 enhancement rather than 3.2, as it is a feature not a fix. Ian Walls indicated he would probably not have time to do sign off on this, so if there is someone else who can that would be great. Created attachment 3297 [details]
Text file with documentation example (edit at will).
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 (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. 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 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 Created attachment 3344 [details] [review] Implements Jared's MIME type suggestion This implements Jared's suggestion to add the MIME types - anything that starts with "image/" in the 856q field will also enable the display of the image, along with any valid MIME extension (jpg, png, bmp, etc.). This patch is only the XSLT files, so I didn't obsolete the earlier one as the prefs, sql, and XSLT.pm files from that patch are still required. Thanks, Doug 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 (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 I found the info on the systemprefs - I'll take care of that. Doug Created attachment 3353 [details] [review] Non-english system pref files Contains the non-english system pref files. Also has the fr-FR text file for the system pref file - but that was a mistake so ignore it. 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 Created attachment 3482 [details] [review] system pref files - all All system pref files, english and other languages. Hopefully fixes broken syspref files patch I uploaded last time. Created attachment 3540 [details] [review] Signed-off and squashed patch Doug, I hope you don't mind but I took the liberty of squashing your three patch files into one and filling out the commit message a bit. I have rebased the patch against the latest Master, tested thoroughly, and confirmed that everything seems to be working. Regards, Jared 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 Pushed, please test The default value for these new preferences should be 'neither' but it appears instead to be the first value in each pull down menu. PS. that was on an upgrade - but of course it should be the default in upgrades and new installs. 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 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 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 |