Some templates in staff interface do not end with include intranet-bottom.inc. > tail -n1 koha-tmpl/intranet-tmpl/prog/en/modules/*/*.tt | grep -B2 '</html>' ==> koha-tmpl/intranet-tmpl/prog/en/modules/acqui/showorder.tt <== </html> -- ==> koha-tmpl/intranet-tmpl/prog/en/modules/basket/basket.tt <== </html> ==> koha-tmpl/intranet-tmpl/prog/en/modules/basket/downloadcart.tt <== </html> ==> koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasketform.tt <== </html> ==> koha-tmpl/intranet-tmpl/prog/en/modules/batch/print-notices.tt <== </html> -- ==> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/showelastic.tt <== </html> ==> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/showmarc.tt <== </html> -- ==> koha-tmpl/intranet-tmpl/prog/en/modules/reports/convert_report.tt <== </html> -- ==> koha-tmpl/intranet-tmpl/prog/en/modules/tools/preview_letter.tt <== </html> -- ==> koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/downloadshelf.tt <== </html> ==> koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/sendshelfform.tt <== </html> So for example it does not call hook "intranet_js". Should all templates end with this include ?
Popup windows and pages like showmarc.tt which are loaded via AJAX do not need the full contents of intranet-bottom. We would either need to add some logic to intranet-bottom.inc or add a different, trimmed-down version.
*** Bug 38740 has been marked as a duplicate of this bug. ***
koha-tmpl/intranet-tmpl/prog/en/modules/basket/basket.tt calls : [% INCLUDE 'intranet-bottom.inc' popup_window=1 %] That will add ending tags body and html this must be at the end
other suspects : koha-tmpl/intranet-tmpl/prog/en/includes/mana/mana-report-search-result.inc koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/cn_browser.tt koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc_field_210c_bis.tt koha-tmpl/intranet-tmpl/prog/en/modules/reserve/hold-group.tt koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt koha-tmpl/intranet-tmpl/prog/en/modules/tools/batch_modify_holds.tt > Should all templates end with this include ? Nope, some blocks may exist after it. Also there is the example curbside_pickups.tt with : [% UNLESS policy.enabled %] <div class="alert alert-warning"> Curbside pickups are not enabled for your library. </div> [% INCLUDE 'intranet-bottom.inc' %] [% STOP %] [% END %]
Check with : git grep -A1 "INCLUDE 'intranet-bottom.inc'" | grep -v INCLUDE | grep '</'
(In reply to Fridolin Somers from comment #3) > koha-tmpl/intranet-tmpl/prog/en/modules/basket/basket.tt > calls : > [% INCLUDE 'intranet-bottom.inc' popup_window=1 %] > That will add ending tags body and html > this must be at the end Caused by bug 37033.
Created attachment 187506 [details] [review] Bug 38739: Fix mana-report-search-result.inc
Created attachment 187507 [details] [review] Bug 38739: Fix admin/file_transports.pl
Created attachment 187508 [details] [review] Bug 38739: Fix tools/batch_modify_holds.tt
Created attachment 187509 [details] [review] Bug 38739: Fix some other wrong templates
Created attachment 187510 [details] [review] Bug 38739: Fix basket/basket.tt
Not sure how to test this one. Changed the assignee. The patches no longer apply 8-(... git bz apply 38739 Bug 38739 - Templates not ending with include intranet-bottom.inc in staff interface 187506 - Bug 38739: Fix mana-report-search-result.inc 187507 - Bug 38739: Fix admin/file_transports.pl 187508 - Bug 38739: Fix tools/batch_modify_holds.tt 187509 - Bug 38739: Fix some other wrong templates 187510 - Bug 38739: Fix basket/basket.tt Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 38739: Fix mana-report-search-result.inc Applying: Bug 38739: Fix admin/file_transports.pl Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/file_transports.tt error: Failed to merge in the changes. Patch failed at 0001 Bug 38739: Fix admin/file_transports.pl
Created attachment 190335 [details] [review] Bug 38739: Fix mana-report-search-result.inc Patch from commit e55c96c
Created attachment 190336 [details] [review] Bug 38739: Fix admin/file_transports.pl Patch from commit c017f7a
Created attachment 190337 [details] [review] Bug 38739: Fix some other wrong templates Patch from commit 5cd3df6
Created attachment 190338 [details] [review] Bug 38739: Fix basket/basket.tt Patch from commit 3178c9a
Created attachment 190339 [details] [review] Bug 38739: Fix tools/batch_modify_holds.tt Patch from commit a52c574
Test plan: Use `git show -w` on the different commit to confirm that: * main-container.inc is used to replace the div.main and main tags * the extra closing div tag at the end of the files is removed You can also test the UI and confirm that the sidebar is correctly displayed on the different pages modified by those patches.
Created attachment 190340 [details] [review] Bug 38739: Fix mana-report-search-result.inc Patch from commit c4c0853
Created attachment 190341 [details] [review] Bug 38739: Fix admin/file_transports.pl Patch from commit a10f7ee
Created attachment 190342 [details] [review] Bug 38739: Fix some other wrong templates Patch from commit 7c8466c
Created attachment 190343 [details] [review] Bug 38739: Fix basket/basket.tt Patch from commit dd70679
Created attachment 190344 [details] [review] Bug 38739: Fix tools/batch_modify_holds.tt Patch from commit 27ac86a
Created attachment 190421 [details] [review] Bug 38739: Fix mana-report-search-result.inc Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 190422 [details] [review] Bug 38739: Fix admin/file_transports.pl Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 190423 [details] [review] Bug 38739: Fix some other wrong templates Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 190424 [details] [review] Bug 38739: Fix basket/basket.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 190425 [details] [review] Bug 38739: Fix tools/batch_modify_holds.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 190426 [details] [review] Bug 38739: (follow-up) Remove extra messages.inc include Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 190780 [details] [review] Bug 38739: Fix mana-report-search-result.inc Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 190781 [details] [review] Bug 38739: Fix admin/file_transports.pl Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 190782 [details] [review] Bug 38739: Fix some other wrong templates Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 190783 [details] [review] Bug 38739: Fix basket/basket.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 190784 [details] [review] Bug 38739: Fix tools/batch_modify_holds.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 190785 [details] [review] Bug 38739: (follow-up) Remove extra messages.inc include Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
I've been going at the changed pages and doing a diff before and after the patches on - the final HTML in the inspector (after eventual JS does it's stuff, but the browser will fix some stuff like stray tags) - the HTTP response And also comparing the HTMLs results the W3C validator. The changes are great, in hold-group.tt, on main, the cookieConsentBar was duplicated :o In the final html in the inspector, it is also present in the holds group modal. It was invisible but still, it was there and duplicated. So now it's fixed. audio-alert was also duplicated. Overall on the pages, several unclosed <div> are fixed. I think also some other tags that were stray. Works, makes sense, QA script happy, code looks good, passing QA :)
Nice work everyone! Pushed to main for 26.05