Bug 33848 - Enabling Coce in the OPAC breaks cover images on bibliographic detail page
Summary: Enabling Coce in the OPAC breaks cover images on bibliographic detail page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL:
Keywords:
: 34125 (view as bug list)
Depends on: 32412
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-26 17:24 UTC by Owen Leonard
Modified: 2024-07-04 20:40 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.03,22.11.09,22.05.17
Circulation function:


Attachments
Bug 33848: Don't remove coce container from template (1.22 KB, patch)
2023-05-31 11:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33848: Enabling Coce in the OPAC breaks cover images on bibliographic detail page (2.61 KB, patch)
2023-06-27 10:43 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33848: Enabling Coce in the OPAC breaks cover images on bibliographic detail page (2.66 KB, patch)
2023-06-27 14:15 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 33848: Don't remove coce container from template (1.28 KB, patch)
2023-07-29 20:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33848: Enabling Coce in the OPAC breaks cover images on bibliographic detail page (2.72 KB, patch)
2023-07-29 20:35 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2023-05-26 17:24:14 UTC
If you enable multiple cover image services, including Coce, the bibliographic detail page in the OPAC doesn't display all covers.

Viewing a title with Amazon, Google, Coce, and OpenLibrary, only the Amazon and Google images display. In the console is an error message:

Uncaught TypeError: parent.find is not a function
    getURL http://localhost:8080/opac-tmpl/bootstrap/js/coce_22.1200048.js:20

The script stops processing other sources when Coce fails.
Comment 1 Jonathan Druart 2023-05-30 07:40:02 UTC
I am not recreating the problem. Do you recreate in ktd? If so which biblionumber?
Comment 2 Owen Leonard 2023-05-30 16:49:41 UTC
I'm able to reproduce this problem *intermittently* using Firefox, the sample data in master, and with these preferences enabled:

OPACAmazonCoverImages
OpacCoce
GoogleJackets
OpenLibraryCovers

I perform this search:

http://127.0.0.1:8080/cgi-bin/koha/opac-search.pl?q=su:%22C%20Computer%20program%20language%22

The first result, "The C programming language," shows four covers, one from each of the services listed above.

On the detail page for that result, it only shows three covers.
Comment 3 Jonathan Druart 2023-05-31 11:46:29 UTC
Created attachment 151864 [details] [review]
Bug 33848: Don't remove coce container from template

It's removed from coce.js
Comment 4 Jonathan Druart 2023-05-31 11:46:48 UTC
I think I've tracked it down, but there are several things that go wrong.

First I don't get the error reported in comment 0. However I do see images removed, randomly. I can see 2, 3 or 4.

This patch fix the Coce problem, but it is still not working for OpenLibrary. And it is not fixed staff side either.

Apparently we reach verify_images before the image is loaded the the code around "KOHA.coce.done" (32307, 28179, 28180) was supposed to prevent that...
Comment 5 Owen Leonard 2023-06-27 10:33:01 UTC
*** Bug 34125 has been marked as a duplicate of this bug. ***
Comment 6 Owen Leonard 2023-06-27 10:38:51 UTC
(In reply to Lucas Gass from comment on Bug 34125 #0)
> To recreate:
> 
> 1. Turn on Coce.
> 2. Set OPACURLOpenInNewWindow to 'Do'.
> 3. Go to an OPAC detail page that should return a Coce image.
> 4. Nothing loads, and if you look at the console you will see the error
> 'parent.find is not a function' coming from coce.js

I hadn't made a connection to OPACURLOpenInNewWindow, that makes things all the more confusing.
Comment 7 Owen Leonard 2023-06-27 10:43:26 UTC
Created attachment 152722 [details] [review]
Bug 33848: Enabling Coce in the OPAC breaks cover images on bibliographic detail page

Bug 32412 added an additional "context" parameter to KOHA.coce.getURL.
The code which pulls a Coce image for the "main" cover image slider
needs to include the correct context parameter for the images to load.

This patch also makes a minor correction to CSS in order to make the
cover image slider "dots" change color to highlight the current slide.

To test, apply the patch and rebuild the OPAC CSS.

- Enable multiple cover image sources in the OPAC including Coce, e.g.
  Amazon, Google, OpenLibrary, etc.
- Search for a title which will match multiple cover image sources. In
  the sample data a search for "Perl" will return good results.
- Note the number of cover images loading for a title in the search
  results.
- View the detail page for that title. There should be the same number
  of covers in the cover image slider.
- Clicking the slider navigation dots should work to show each image,
  and the dot should change color to highlight the current image.
Comment 8 Owen Leonard 2023-06-27 10:48:01 UTC
I'm not certain this is the correct solution but it seems to work. I don't know what the connection is to OPACURLOpenInNewWindow though.
Comment 9 Lucas Gass (lukeg) 2023-06-27 14:15:15 UTC
Created attachment 152747 [details] [review]
Bug 33848: Enabling Coce in the OPAC breaks cover images on bibliographic detail page

Bug 32412 added an additional "context" parameter to KOHA.coce.getURL.
The code which pulls a Coce image for the "main" cover image slider
needs to include the correct context parameter for the images to load.

This patch also makes a minor correction to CSS in order to make the
cover image slider "dots" change color to highlight the current slide.

To test, apply the patch and rebuild the OPAC CSS.

- Enable multiple cover image sources in the OPAC including Coce, e.g.
  Amazon, Google, OpenLibrary, etc.
- Search for a title which will match multiple cover image sources. In
  the sample data a search for "Perl" will return good results.
- Note the number of cover images loading for a title in the search
  results.
- View the detail page for that title. There should be the same number
  of covers in the cover image slider.
- Clicking the slider navigation dots should work to show each image,
  and the dot should change color to highlight the current image.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Jonathan Druart 2023-07-04 13:25:02 UTC
What about my patch here? Should I obsolete it?
Comment 11 Katrin Fischer 2023-07-29 20:35:39 UTC
Created attachment 154063 [details] [review]
Bug 33848: Don't remove coce container from template

It's removed from coce.js

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2023-07-29 20:35:42 UTC
Created attachment 154064 [details] [review]
Bug 33848: Enabling Coce in the OPAC breaks cover images on bibliographic detail page

Bug 32412 added an additional "context" parameter to KOHA.coce.getURL.
The code which pulls a Coce image for the "main" cover image slider
needs to include the correct context parameter for the images to load.

This patch also makes a minor correction to CSS in order to make the
cover image slider "dots" change color to highlight the current slide.

To test, apply the patch and rebuild the OPAC CSS.

- Enable multiple cover image sources in the OPAC including Coce, e.g.
  Amazon, Google, OpenLibrary, etc.
- Search for a title which will match multiple cover image sources. In
  the sample data a search for "Perl" will return good results.
- Note the number of cover images loading for a title in the search
  results.
- View the detail page for that title. There should be the same number
  of covers in the cover image slider.
- Clicking the slider navigation dots should work to show each image,
  and the dot should change color to highlight the current image.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2023-07-29 20:36:45 UTC
I've tested both patches together in Chromium and Firefox in Ubuntu. I had some difficulty reproducing the original bug, but I see improvements with this patch set, like highlighting of the currently visible picture, and no regressions.
Comment 14 Tomás Cohen Arazi (tcohen) 2023-08-15 09:13:46 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 15 Fridolin Somers 2023-08-17 20:11:03 UTC
Pushed to 23.05.x for 23.05.03
Comment 16 Pedro Amorim 2023-08-18 13:46:07 UTC
Nice work everyone!

Pushed to 22.11.x for next release
Comment 17 Lucas Gass (lukeg) 2023-11-07 14:21:58 UTC
Backported to 22.05.x for upcoming 22.05.17 release.