Bug 31217 - Fix Coce JavaScript to hide single-pixel cover images in the OPAC lightbox gallery
Summary: Fix Coce JavaScript to hide single-pixel cover images in the OPAC lightbox ga...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Alex Buckley
QA Contact: Testopia
URL:
Keywords:
Depends on: 28180
Blocks:
  Show dependency treegraph
 
Reported: 2022-07-21 21:50 UTC by Alex Buckley
Modified: 2022-10-23 09:50 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,22.05.06,21.11.12


Attachments
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (1.61 KB, patch)
2022-07-21 22:02 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (1.69 KB, patch)
2022-07-25 03:56 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (3.01 KB, patch)
2022-07-26 05:42 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (3.07 KB, patch)
2022-07-28 08:18 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (3.12 KB, patch)
2022-07-28 10:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (3.10 KB, patch)
2022-08-16 21:55 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images (3.16 KB, patch)
2022-08-18 08:16 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2022-07-21 21:50:03 UTC
The shift to using a lightbox gallery to display cover images in the detail pages of the OPAC has stopped Coce from hiding 1-pixel cover images.

This line ( https://github.com/Koha-Community/Koha/blob/0177b88eb33d398da4eb5ffb6344226930c3346b/koha-tmpl/opac-tmpl/bootstrap/js/coce.js#L39 ) should hide the coce-coverimg box from the OPAC for 1-pixel images. 

However, coce-coverimg is an id, not a class, in the OPAC detail page, therefore, this JavaScript does not hide anything in the OPAC.
Comment 1 Alex Buckley 2022-07-21 22:02:25 UTC
Created attachment 137995 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because OPAC coce.js targets coce-coverimg as a class, when it is
actually an ID.

Test plan:
1. Set:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. View the biblio in the OPAC. Observe an empty 'Coce image from
Amazon.com' box loads

4. Apply patch and restart services

5. Reload the same biblio record in the OPAC. Observe the empty 'Coce
image from Amazon.com' is not displayed

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 2 Alex Buckley 2022-07-21 22:05:26 UTC
This is now ready for testing!

This is the coce-coverimg in opac-detail.tt ( https://github.com/Koha-Community/Koha/blob/0177b88eb33d398da4eb5ffb6344226930c3346b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt#L105 ) as you can see it is an id, not a class.
Comment 3 Jonathan Druart 2022-07-22 05:59:14 UTC
Hi Alex,

I didn't test the patch, bug had a look at it.

A couple of remarks:
1. if you have the id you don't need to use a parent, the line could simply be
  $("#coce-coverimg").remove();
2. I am wondering if staff-side as well:

koha-tmpl/intranet-tmpl/js/coce.js:                                $(this).closest(".coce-coverimg").remove();

We are using an id on the detail page
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt:                        <div class="cover-image" id="coce-coverimg">

BUT a class on the result list
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:                    <div id="coce-coverimg-[% SEARCH_RESULT.biblionumber | html %]" class="cover-image coce-coverimg">
Comment 4 David Nind 2022-07-25 01:19:45 UTC
I was able to successfully test this in an "Ingognito" window using Google Chrome:
- without the patch, it displays a spinner and 'Coce image from Amazon'
- after the patch (and clearing the cached and restarting), no image was displayed

For whatever reason (probably privacy and other settings) I didn't observe the same in Firefox - no image or placeholder was displayed in the OAPC details page.

Happy to sign off once Jonathan's question(s) are resolved.
Comment 5 Alex Buckley 2022-07-25 03:56:45 UTC
Created attachment 138070 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because OPAC coce.js targets coce-coverimg as a class, when it is
actually an ID.

Test plan:
1. Set:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. Open a "Incognito" window in your browser (recommended to use Google
Chrome to test this)

4. View the biblio in the OPAC. Observe a 'Coce image from
Amazon' box loads with a spinner

5. Apply patch and restart services

6. Clear your browser cache

7. Reload the same biblio record in the OPAC. Observe no image is
displayed.

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 6 Alex Buckley 2022-07-25 03:59:42 UTC
Hi Jonathan and David, 

Thank you both very much for your input. 

I have done the following:

- Changed my patch to '$("#coce-coverimg").remove();'

- Created a new bug report for making a similar change on the staff-side (bug 31228)

- Amended my commit test plan based on your notes David. 

Ready to test again!

Alex
Comment 7 Alex Buckley 2022-07-26 05:42:31 UTC
Created attachment 138111 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because opac-detail.tt has coce-coverimg as an ID, but OPAC
coce.js targets coce-coverimg as a class.

opac-detail.tt should be changed to have coce-coverimg as a class.

Test plan:
1. Set your system preferences:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. Open a "Incognito" window in your browser (recommended to use Google
Chrome to test this)

4. View the biblio detail page in the OPAC. Observe a 'Coce image from
Amazon' box loads with a spinner

5. Apply patch and restart services

6. Clear your browser cache

7. Reload the same biblio detail page in the OPAC. Observe no image is
displayed.

8. View another OPAC biblio detail page that does have a successfully
loading coce image and confirm the source of that image is listed: For
example 'Coce image from Amazon.com' is displayed

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Comment 8 Alex Buckley 2022-07-26 06:10:06 UTC
I've gone ahead and added patches on this bug report and 31228 to switch to using coce-coverimg as a class consistently in the OPAC and staff client. 

I have found that browser caching can make this a tricky thing to test.

Ready for testing again.
Comment 9 Paul Derscheid 2022-07-28 08:18:50 UTC
Created attachment 138192 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because opac-detail.tt has coce-coverimg as an ID, but OPAC
coce.js targets coce-coverimg as a class.

opac-detail.tt should be changed to have coce-coverimg as a class.

Test plan:
1. Set your system preferences:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. Open a "Incognito" window in your browser (recommended to use Google
Chrome to test this)

4. View the biblio detail page in the OPAC. Observe a 'Coce image from
Amazon' box loads with a spinner

5. Apply patch and restart services

6. Clear your browser cache

7. Reload the same biblio detail page in the OPAC. Observe no image is
displayed.

8. View another OPAC biblio detail page that does have a successfully
loading coce image and confirm the source of that image is listed: For
example 'Coce image from Amazon.com' is displayed

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 10 Paul Derscheid 2022-07-28 08:20:15 UTC
Didn't write it in the sign-off because I'm scared of the python timeouts but everything works as expected. Great work. 👍
Comment 11 Owen Leonard 2022-07-28 10:42:47 UTC
Created attachment 138198 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because opac-detail.tt has coce-coverimg as an ID, but OPAC
coce.js targets coce-coverimg as a class.

opac-detail.tt should be changed to have coce-coverimg as a class.

Test plan:
1. Set your system preferences:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. Open a "Incognito" window in your browser (recommended to use Google
Chrome to test this)

4. View the biblio detail page in the OPAC. Observe a 'Coce image from
Amazon' box loads with a spinner

5. Apply patch and restart services

6. Clear your browser cache

7. Reload the same biblio detail page in the OPAC. Observe no image is
displayed.

8. View another OPAC biblio detail page that does have a successfully
loading coce image and confirm the source of that image is listed: For
example 'Coce image from Amazon.com' is displayed

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 12 Katrin Fischer 2022-08-08 19:57:28 UTC
I believe that this should work, but I have trouble testing. I can see the cover in the OPAC result list, but the detail pages all remain empty, with and without the patch :(
Comment 13 Katrin Fischer 2022-08-08 19:58:07 UTC
Tested in Firefox in a private window to make sure the cache was not to blame. Also restarted all the things.
Comment 14 Alex Buckley 2022-08-09 05:37:33 UTC
(In reply to Katrin Fischer from comment #13)
> Tested in Firefox in a private window to make sure the cache was not to
> blame. Also restarted all the things.

Hey Katrin, 

Thank you for trying to test this!

I wonder if you try testing with Google Chrome in an incognito window (as David did in comment #4) do you see the cover image on the OPAC detail page?

Thanks,
Alex
Comment 15 Owen Leonard 2022-08-09 15:02:22 UTC
I can reproduce the problem with this ISBN: 0590426850
Comment 16 Alex Buckley 2022-08-16 21:55:29 UTC
Created attachment 139218 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because opac-detail.tt has coce-coverimg as an ID, but OPAC
coce.js targets coce-coverimg as a class.

opac-detail.tt should be changed to have coce-coverimg as a class.

Test plan:
1. Set your system preferences:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. Open a "Incognito" window in your browser (recommended to use Google
Chrome to test this)

4. View the biblio detail page in the OPAC. Observe a 'Coce image from
Amazon' box loads with a spinner

5. Apply patch and restart services

6. Clear your browser cache

7. Reload the same biblio detail page in the OPAC. Observe no image is
displayed.

8. View another OPAC biblio detail page that does have a successfully
loading coce image and confirm the source of that image is listed: For
example 'Coce image from Amazon.com' is displayed

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 17 Alex Buckley 2022-08-16 21:56:58 UTC
I have updated this patch so it is consistent with the staff client Coce fix - bug 31228

Now instead of line 1535 of koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt being:
} else if( div.attr("class") == "cover-image coce-coverimg" ){

I use:
} else if( div.hasClass("coce-coverimg" ) ){

Please let me know if I should remove the signoff lines or change the status of this bug report :)

Many thanks,
Alex
Comment 18 Fridolin Somers 2022-08-18 08:16:55 UTC
Created attachment 139344 [details] [review]
Bug 31217: OPAC coce.js is not hiding the coce-coverimg div for single-pixel images

This is because opac-detail.tt has coce-coverimg as an ID, but OPAC
coce.js targets coce-coverimg as a class.

opac-detail.tt should be changed to have coce-coverimg as a class.

Test plan:
1. Set your system preferences:
- CoceHost = https://coce.mykoha.co.nz
- CoceProviders = Select all
- OpacCoce = Enable

2. Add the ISBN of '9780262336499' to a biblio

3. Open a "Incognito" window in your browser (recommended to use Google
Chrome to test this)

4. View the biblio detail page in the OPAC. Observe a 'Coce image from
Amazon' box loads with a spinner

5. Apply patch and restart services

6. Clear your browser cache

7. Reload the same biblio detail page in the OPAC. Observe no image is
displayed.

8. View another OPAC biblio detail page that does have a successfully
loading coce image and confirm the source of that image is listed: For
example 'Coce image from Amazon.com' is displayed

Sponsored-by: Toi Ohomai Institute of Technology, New Zealand
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 19 Tomás Cohen Arazi 2022-08-18 12:05:42 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 20 Lucas Gass 2022-10-03 22:45:26 UTC
Backported to 22.05.x for 22.05.06
Comment 21 Victor Grousset/tuxayo 2022-10-17 00:22:42 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 22 Arthur Suzuki 2022-10-22 22:08:50 UTC
Thanks!

Pushed to 21.11 for 21.11.12