|
Description
Alex Buckley
2022-07-21 21:50:03 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 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. 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">
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. 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 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
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 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. 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> Didn't write it in the sign-off because I'm scared of the python timeouts but everything works as expected. Great work. [U+1F44D] 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> 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 :( Tested in Firefox in a private window to make sure the cache was not to blame. Also restarted all the things. (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 I can reproduce the problem with this ISBN: 0590426850 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> 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 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> Pushed to master for 22.11. Nice work everyone, thanks! Backported to 22.05.x for 22.05.06 Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved. Thanks! Pushed to 21.11 for 21.11.12 |