It looks like bug 26933 might've broke OPAC local cover images in the search results due to a possible race condition. In koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt around line 630 there is a window.load() call which runs 'wait_for_images(verify_cover_images)'. If you're only using OPACLocalCoverImages, there is no wait, so verify_cover_images() is called immediately. That function removes the .cover-image element if there are no images. In the case of OPACLocalCoverImages, there are no images yet, so the element gets removed. Now there are no images yet because 'KOHA.LocalCover.GetCoverFromBibnumber(false);', the function which populates the local cover image elements, hasn't even run yet. Note that it's called within a $(document).ready() which is written lower than the window.load() It's possible that different configurations would yield different results, if they load different resources at different times, which could affect when the handlers run. I've observed the above when using a fairly vanilla koha-testing-docker.
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt 555 // if( $(coverSlide).find(".cover-image").length < 1 ){ 556 // $(coverSlide).remove(); 557 // } else { 558 // This is a suboptimal workaround; we should do this via load, but 559 // the image code is scattered all over now. We come here now after 560 // window load and wait_for_images (so load completed). 561 var check_complete = 1; 562 $(coverSlide).find("img").each( function() { 563 if( !this.complete || this.naturalHeight == 0 ) check_complete = 0; 564 }); 565 if( check_complete ) $(coverSlide).removeClass('cover-slides'); 566 // } Why are those lines commented? It works (at first glance) if I bring them back.
(In reply to Jonathan Druart from comment #1) > Why are those lines commented? > > It works (at first glance) if I bring them back. I've been curious about this as well. Wondering if Owen has any insight into this one.
Created attachment 179170 [details] [review] Bug 38981: Local cover images failing to load in OPAC search results This patch uncomments some code which was incorrectly left commented after debugging.
In Chrome with this patch it works good. In Firefox, it works about 1/10 times I load the results page. Is there some kind of race condition that Firefox is slower at that is causing this not to work there?
(In reply to Lucas Gass (lukeg) from comment #4) > In Chrome with this patch it works good. In Firefox, it works about 1/10 > times I load the results page. > > Is there some kind of race condition that Firefox is slower at that is > causing this not to work there? That could be. My initial observations were around some weird interplay between window.load and $(document).ready() working a bit differently than I would've expected...
(In reply to David Cook from comment #5) > (In reply to Lucas Gass (lukeg) from comment #4) > > In Chrome with this patch it works good. In Firefox, it works about 1/10 > > times I load the results page. > > > > Is there some kind of race condition that Firefox is slower at that is > > causing this not to work there? > > That could be. My initial observations were around some weird interplay > between window.load and $(document).ready() working a bit differently than I > would've expected... Using window.load is correct here, we need to wait for the images to be loaded.
We have fixed this problem on the staff interface long ago. The use of GetCoverFromBibnumber is problematic (and it's also why shelfbrowser is broken, see bug 31207) We should pass the image numbers to the template, the same way we do for opac-detail 60 [% IF ( OPACLocalCoverImages ) %] 61 [% IF localimages.count %] 62 [% FOREACH image IN localimages %] 63 <div class="cover-image local-coverimg"> 64 <a href="/cgi-bin/koha/opac-image.pl?imagenumber=[% image.imagenumber | uri %]" title="Local cover image"> 65 <img 66 src="/cgi-bin/koha/opac-image.pl?thumbnail=1&imagenumber=[% image.imagenumber | uri %]" 67 alt="Local cover image" 68 data-link="/cgi-bin/koha/opac-imageviewer.pl?biblionumber=[% biblionumber | uri %]&imagenumber=[% image.imagenumber | uri %]" 69 /> 70 </a> 71 <div class="hint">Local cover image</div> 72 </div> 73 [% END %] 74 [% END %] 75 [% END %]
(In reply to Jonathan Druart from comment #7) > We have fixed this problem on the staff interface long ago. Bug 25031: Improve handling of multiple covers on the biblio detail page in the staff client
Created attachment 179210 [details] [review] Bug 38981: Directly use the opac-image link
Why not... simply linking to the opac-image url?...
(In reply to Jonathan Druart from comment #10) > Why not... simply linking to the opac-image url?... I've been wondering that too. Did you test it? I wonder if it has to do with the lightbox being used?
Created attachment 179431 [details] [review] Bug 38981: Local cover images failing to load in OPAC search results This patch uncomments some code which was incorrectly left commented after debugging. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 179432 [details] [review] Bug 38981: Directly use the opac-image link Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. Enable the OPACLocalCoverImages and AllowMultipleCovers system preferences. 2. Add some cover images to a couple of records. 3. Add multiple cover images for one record. 4. Use two different browsers (Firefox and a Chromium-based browser) and in the OPAC run a search that returns records with the cover images. 5. Note that: - For Firefox: there are no local cover images shown in the search results, but they are displayed on the record details page - For Chromium-based browsers: things work as expected - local cover images are shown in the search results and the record details page 6. Apply the patch. 7. Repeat step 4. 8. Note that Firefox and Chromium browsers both now work as expected - local cover images are correctly shown in the OPAC search results and details pages. Note: The OPAC search results only show one local cover image, even with AllowMultipleCovers enabled. Multiple covers are viewable on the details page in the OPAC.
Created attachment 179560 [details] [review] Bug 38981: Local cover images failing to load in OPAC search results This patch uncomments some code which was incorrectly left commented after debugging. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 179561 [details] [review] Bug 38981: Directly use the opac-image link Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Thanks a lot, Lucas! I've been meaning to come back to this, but just haven't had the time.
Pushed for 25.05! Well done everyone, thank you!