[Bug 25002] New: JS Includes should be wrapped with template comments
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Bug ID: 25002 Summary: JS Includes should be wrapped with template comments Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org When debugging, we are often faced with a nameless <script> area when it comes to inspecting the page.. it would be helpful to have an HTML comment wrapping these script in our includes so you can quickly identify where the JS is coming from. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- See first few minutes of the `Koha Virtual Hackfest - Code Spaghetti` live stream recording for further context. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #2 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- *** Bug 25003 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 --- Comment #3 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Martin beat me to it :) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |25003 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25003 [Bug 25003] Clearly identify when JavaScript errors are caused by OPACUserJS/IntranetUserJS -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |oleonard@myacpl.org |ity.org | Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 102041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102041&action=edit Bug 25002: JS Includes should be wrapped with template comments This patch adds HTML comments to Template::Toolkit include files which contain <script> tags so that it is clear where the embedded scripts can be found in the code. To test, apply the patch and view source on the following pages to verify the presence of the comments: Acquisitions home page: - acquisitions-toolbar.inc - validtor-strings.inc - js_includes.inc - format-price.inc Cataloging -> Add/Edit item: - additem.js.inc - columns_settings.inc - strings.inc - select2.inc - calendar.inc - str/cataloging_additem.inc Authorities home page: - authorities_js.inc Bibliographic detail page: - catalog-strings.inc Cataloging -> Advanced editor: - cateditor-ui.inc - cateditor-widgets-marc21.inc Administration -> Item types: - greybox.inc ILL requests: - ill-list-table-strings.inc Web installer - installer-intranet-bottom.inc Web installer -> Onboarding - installer-strings.inc Lists -> List contents -> Merge records - merge-record-strings.inc Patrons -> Patron -> Change password - password_check.inc - str/members-menu.inc Patrons -> Patron -> Print summary - slip-print.inc Circulation -> Check out - timepicker.inc Administration -> System preferences: - str/tinymce_i18n.inc - wysiwyg-systempreferences.inc Cataloging -> Z39.50 Search: - z3950_search.inc -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Small patch |--- --- Comment #5 from Owen Leonard <oleonard@myacpl.org> --- I hope this patch is what you had in mind. Going through the includes highlights the need to update and/or remove several of them now that we have proper JS i18n, but I didn't think it hurt to update them anyway in the meantime. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Architecture, internals, |Templates |and plumbing | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 David Roberts <david.roberts@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david.roberts@ptfs-europe.c | |om --- Comment #6 from David Roberts <david.roberts@ptfs-europe.com> --- I've been through this, and have a few comments. Firstly, I can't see where the additem.js.inc displays - it doesn't seem to be on the same page as all the other "Cataloging -> Add/Edit item:" comments that you've added. Secondly, <!-- / password_check.inc --> and <!-- password_check.inc --> seem to render the wrong way round - i.e. the closing tag displays first. Thirdly, str/tinymce_i18n.inc doesn't seem to have a closing comment. I haven't been able to test the web installer pages yet, and I can't get to the source on the Patrons -> Patron -> Print summary screen because the printer dialogue blocks access to it. Everything else seems perfect though :-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102041|0 |1 is obsolete| | --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 103374 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103374&action=edit Bug 25002: JS Includes should be wrapped with template comments This patch adds HTML comments to Template::Toolkit include files which contain <script> tags so that it is clear where the embedded scripts can be found in the code. To test, apply the patch and view source on the following pages to verify the presence of the comments: Acquisitions home page: - acquisitions-toolbar.inc - validtor-strings.inc - js_includes.inc - format-price.inc Acquisitions -> Add order from new record, Acquisitions -> Receive order: - additem.js.inc Cataloging -> Add/Edit item: - columns_settings.inc - strings.inc - select2.inc - calendar.inc - str/cataloging_additem.inc Authorities home page: - authorities_js.inc Bibliographic detail page: - catalog-strings.inc Cataloging -> Advanced editor: - cateditor-ui.inc - cateditor-widgets-marc21.inc Administration -> Item types: - greybox.inc ILL requests: - ill-list-table-strings.inc Web installer - installer-intranet-bottom.inc Web installer -> Onboarding - installer-strings.inc Lists -> List contents -> Merge records - merge-record-strings.inc Patrons -> Patron -> Change password - password_check.inc - str/members-menu.inc Patrons -> Patron -> Print summary - slip-print.inc Circulation -> Check out - timepicker.inc Administration -> System preferences: - str/tinymce_i18n.inc - wysiwyg-systempreferences.inc Cataloging -> Z39.50 Search: - z3950_search.inc -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 --- Comment #8 from Owen Leonard <oleonard@myacpl.org> --- Thanks for testing, David. And don't hesitate to mark something "Failed QA" when you see problems. If anything it gets my attention! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Frédéric Demians <frederic@tamil.fr> 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=25002 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103374|0 |1 is obsolete| | --- Comment #9 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 103841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103841&action=edit Bug 25002: JS Includes should be wrapped with template comments This patch adds HTML comments to Template::Toolkit include files which contain <script> tags so that it is clear where the embedded scripts can be found in the code. To test, apply the patch and view source on the following pages to verify the presence of the comments: Acquisitions home page: - acquisitions-toolbar.inc - validtor-strings.inc - js_includes.inc - format-price.inc Acquisitions -> Add order from new record, Acquisitions -> Receive order: - additem.js.inc Cataloging -> Add/Edit item: - columns_settings.inc - strings.inc - select2.inc - calendar.inc - str/cataloging_additem.inc Authorities home page: - authorities_js.inc Bibliographic detail page: - catalog-strings.inc Cataloging -> Advanced editor: - cateditor-ui.inc - cateditor-widgets-marc21.inc Administration -> Item types: - greybox.inc ILL requests: - ill-list-table-strings.inc Web installer - installer-intranet-bottom.inc Web installer -> Onboarding - installer-strings.inc Lists -> List contents -> Merge records - merge-record-strings.inc Patrons -> Patron -> Change password - password_check.inc - str/members-menu.inc Patrons -> Patron -> Print summary - slip-print.inc Circulation -> Check out - timepicker.inc Administration -> System preferences: - str/tinymce_i18n.inc - wysiwyg-systempreferences.inc Cataloging -> Z39.50 Search: - z3950_search.inc Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25003 Depends on|25003 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25003 [Bug 25003] Clearly identify when JavaScript errors are caused by OPACUserJS/IntranetUserJS -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=25002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103841|0 |1 is obsolete| | --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 103877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103877&action=edit Bug 25002: JS Includes should be wrapped with template comments This patch adds HTML comments to Template::Toolkit include files which contain <script> tags so that it is clear where the embedded scripts can be found in the code. To test, apply the patch and view source on the following pages to verify the presence of the comments: Acquisitions home page: - acquisitions-toolbar.inc - validtor-strings.inc - js_includes.inc - format-price.inc Acquisitions -> Add order from new record, Acquisitions -> Receive order: - additem.js.inc Cataloging -> Add/Edit item: - columns_settings.inc - strings.inc - select2.inc - calendar.inc - str/cataloging_additem.inc Authorities home page: - authorities_js.inc Bibliographic detail page: - catalog-strings.inc Cataloging -> Advanced editor: - cateditor-ui.inc - cateditor-widgets-marc21.inc Administration -> Item types: - greybox.inc ILL requests: - ill-list-table-strings.inc Web installer - installer-intranet-bottom.inc Web installer -> Onboarding - installer-strings.inc Lists -> List contents -> Merge records - merge-record-strings.inc Patrons -> Patron -> Change password - password_check.inc - str/members-menu.inc Patrons -> Patron -> Print summary - slip-print.inc Circulation -> Check out - timepicker.inc Administration -> System preferences: - str/tinymce_i18n.inc - wysiwyg-systempreferences.inc Cataloging -> Z39.50 Search: - z3950_search.inc Signed-off-by: Frédéric Demians <f.demians@tamil.fr> 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=25002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.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=25002 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #12 from Joy Nelson <joy@bywatersolutions.com> --- enhancement not backported to 19.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org