Run any page that contains the "Search patrons" search bar through an HTML validator and you will see a lot of: Stray start tag span. Stray end tag span. This is because <option> tags contain spans. <span> is not a valid child element of <option> The HTML5 spec: https://html.spec.whatwg.org/multipage/form-elements.html#the-option-element There are essentially two options for what can be put inside an <option> element: 1. Test 2. Nothing These spans are added via the patronfields.inc
Created attachment 157876 [details] [review] Bug 35157: Remove spans from patronfields.inc To test: 1. Run any page that contains the "Search patrons" search bar through an HTML validator and you will see a lot of: Stray start tag span. Stray end tag span. 2. Apply patch 3. Try step 1 again, no more errors from the HTML validator. 4. Make sure the patron searches still work when limiting by 'Search field'.
Katrin, is there something that can be done here to help improve translations?
Created attachment 158010 [details] [review] Bug 35157: Remove spans from patronfields.inc To test: 1. Run any page that contains the "Search patrons" search bar through an HTML validator and you will see a lot of: Stray start tag span. Stray end tag span. 2. Apply patch 3. Try step 1 again, no more errors from the HTML validator. 4. Make sure the patron searches still work when limiting by 'Search field'. Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. I used the HTML Validator extension (https://www.gueury.com/mozilla/) with the Chromium browser, which shows under Developer Tools. (It is available on Firefox as well, but for some reason Firefox seemed to strip the <span> tags from the option element.)
As it stands, this will move all those words back into one single string, hurting translations. I believe we can use the same trick as in the reworked title tags instead. Have a look at bug 33077. It would also be worth checking if we haven't added spans to other strings appearing inside options. I think we only need to think of where to place the: +[% PROCESS 'i18n.inc' %]
(In reply to Katrin Fischer from comment #5) > As it stands, this will move all those words back into one single string, > hurting translations. I believe we can use the same trick as in the reworked > title tags instead. Have a look at bug 33077. > > It would also be worth checking if we haven't added spans to other strings > appearing inside options. > > I think we only need to think of where to place the: > +[% PROCESS 'i18n.inc' %] If my grepping is correct we need to make sure the i18n include is in the following files: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: [% INCLUDE 'cataloging-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbooks.tt: [% INCLUDE 'cataloging-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt: [% INCLUDE 'cataloging-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/cataloging-home.tt: [% INCLUDE 'cataloging-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/concerns.tt: [% INCLUDE 'cataloging-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/merge.tt: [% INCLUDE 'cataloging-search.inc' %]q koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/advsearch.tt: [% INCLUDE 'adv-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt: [% INCLUDE 'home-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt: [% INCLUDE 'home-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/accountline-details.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/alert-subscriptions.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/apikeys.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/discharge.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/discharges.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/files.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/holdshistory.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/housebound.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/ill-requests.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/mancredit.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/maninvoice.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/member-flags.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/member-password.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/members-update.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/merge-patrons.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/notices.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/purchase-suggestions.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/recallshistory.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/routing-lists.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/statistics.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/two_factor_auth.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt: [% INCLUDE 'patron-search-header.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt: [% INCLUDE 'patron-search-header.inc' %] So we need to add it to these 2 templates: koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt koha-tmpl/intranet-tmpl/prog/en/modules/members/alert-subscriptions.tt
Created attachment 158226 [details] [review] Bug 35157: Remove spans from patronfields.inc To test: 1. Run any page that contains the "Search patrons" search bar through an HTML validator and you will see a lot of: Stray start tag span. Stray end tag span. 2. Apply patch 3. Try step 1 again, no more errors from the HTML validator. 4. Make sure the patron searches still work when limiting by 'Search field'.
Created attachment 159112 [details] [review] Bug 35157: Remove spans from patronfields.inc Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
I tested this using this plugin: https://chrispederick.com/work/web-developer/ before the patch there were 56 stray start and end tags found tag span. After the patch there were 0. Just a note there was one other invalid html error and one warning found by the validator that maybe need another patch? I ran the validator on http://localhost:8081/cgi-bin/koha/members/members-home.pl Error: Stray end tag td. From line 1010, column 39; to line 1010, column 43 Expires on</td>↩ Warning: Empty heading. From line 1037, column 17; to line 1037, column 64 <h4 class="modal-title" id="patronPreviewLabel"></h4>↩
(In reply to Lucas Gass from comment #6) > If my grepping is correct we need to make sure the i18n include is in the > following files: > [...] > So we need to add it to these 2 templates: > > koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt > koha-tmpl/intranet-tmpl/prog/en/modules/members/alert-subscriptions.tt After double checking, it's correct :) In the last weeks, intranet-main.tt also got the i18n include from another ticket so it's end it's just alert-subscriptions.tt
*in the end
Created attachment 159197 [details] [review] Bug 35157: Remove spans from patronfields.inc To test: 1. Run any page that contains the "Search patrons" search bar through an HTML validator and you will see a lot of: Stray start tag span. Stray end tag span. 2. Apply patch 3. Try step 1 again, no more errors from the HTML validator. 4. Make sure the patron searches still work when limiting by 'Search field'. Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Works, makes sense, QA script happy, code looks good, passing QA :) (@Lucas: somehow your patch attached on comment 7 didn't have the test plan in the commit message. But it was only in the BZ comment. So it was lost on signoff.)a
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.01