[Bug 34724] New: Remove use of onclick for opac-imageviewer.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Bug ID: 34724 Summary: Remove use of onclick for opac-imageviewer.pl Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org Depends on: 34723 Like bug 34570, we want to get rid of "onclick". Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34723 [Bug 34723] opac-imageviewer.pl not showing thumbnails -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Created attachment 155255 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155255&action=edit Bug 34724: Remove inline onclick handler for opac-imageviewer.pl This patch moves the click handler for thumbnails for opac-imageviewer.pl out of the inline HTML and out to an existing <script> element. Test plan: 0. Apply the patch (including dependent patches) 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=localcover 2. Change both *LocalCoverImages sysprefs to "Show" 3. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AllowMultipleCovers 4. Change "AllowMultipleCovers" to "Allow" 5. Click "Images (0)" tab 6. Download or make two JPEG images 7. Upload the images 8. Go to http://localhost:8080/cgi-bin/koha/opac-imageviewer.pl?biblionumber=29 9. See one large image in the centre and two smaller thumbnails on the right side 10. Click on the thumbnail of the image that isn't displayed in the centre 11. Note that the image changes 12. Click the other thumbnail and note that the image changes back -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34709 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34709 [Bug 34709] Eliminate event handler attributes in the OPAC -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155255|0 |1 is obsolete| | --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 155299 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155299&action=edit Bug 34724: Remove inline onclick handler for opac-imageviewer.pl This patch moves the click handler for thumbnails for opac-imageviewer.pl out of the inline HTML and out to an existing <script> element. Test plan: 0. Apply the patch (including dependent patches) 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=localcover 2. Change both *LocalCoverImages sysprefs to "Show" 3. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AllowMultipleCovers 4. Change "AllowMultipleCovers" to "Allow" 5. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29 6. Click "Images (0)" tab 7. Download or make two JPEG images 8. Upload the images 9. Go to http://localhost:8080/cgi-bin/koha/opac-imageviewer.pl?biblionumber=29 10. See one large image in the centre and two smaller thumbnails on the right side 11. Click on the thumbnail of the image that isn't displayed in the centre 12. Note that the image changes 13. Click the other thumbnail and note that the image changes back PA amended patch: Added missing test plan step Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #3 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 155300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155300&action=edit Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist The link does not contain translatable strings, it doesnt need to be repeated twice in if-else just for a css class change Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi David, I've submitted a follow-up patch. Please take a look and SO if you agree, or make obsolete if you don't. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155300|0 |1 is obsolete| | --- Comment #5 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 155300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155300 Bug 34724: (QA follow-up): The "src" and Javascript change looked all right, but not the change to "loop.first". I think the translation tools don't like Template Toolkit syntax embedded in HTML except for things like [% image.imagenumber | html %]. So even though there's a lot of duplicated HTML in the current template, it's for reasons. And I think that is the main reason. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim from comment #4)
Hi David, I've submitted a follow-up patch. Please take a look and SO if you agree, or make obsolete if you don't.
If you want to redo your patch to not include that loop.first change, then I'd SO it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- (In reply to David Cook from comment #5)
I think the translation tools don't like Template Toolkit syntax embedded in HTML except for things like [% image.imagenumber | html %].
Correct: https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolk... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 155445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155445&action=edit Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #9 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Cool! Thank you both! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155445|0 |1 is obsolete| | --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Created attachment 155472 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155472&action=edit Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to Pedro Amorim from comment #9)
Cool! Thank you both!
Np! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155299|0 |1 is obsolete| | --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 155758 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155758&action=edit Bug 34724: Remove inline onclick handler for opac-imageviewer.pl This patch moves the click handler for thumbnails for opac-imageviewer.pl out of the inline HTML and out to an existing <script> element. Test plan: 0. Apply the patch (including dependent patches) 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=localcover 2. Change both *LocalCoverImages sysprefs to "Show" 3. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AllowMultipleCovers 4. Change "AllowMultipleCovers" to "Allow" 5. Go to http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=29 6. Click "Images (0)" tab 7. Download or make two JPEG images 8. Upload the images 9. Go to http://localhost:8080/cgi-bin/koha/opac-imageviewer.pl?biblionumber=29 10. See one large image in the centre and two smaller thumbnails on the right side 11. Click on the thumbnail of the image that isn't displayed in the centre 12. Note that the image changes 13. Click the other thumbnail and note that the image changes back PA amended patch: Added missing test plan step Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155472|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 155759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155759&action=edit Bug 34724: (QA follow-up): Replace href from thumbnail link with a # to keep pointer event. The href link gets reconstructed by showCover anyway Remove return false from on click event, its not triggering a location change anymore because of the above Remove check for imagenumber, its a non-null primary key, if the image exists, then the imagenumber must also exist Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|23.11.00 |23.11.00,23.05.04 released in| | --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.11.00,23.05.04 |23.11.00,23.05.04,22.11.11 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34724 --- Comment #16 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to oldstable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org