All templates should be valid XHTML. I'm finding many new instances of unescaped ampersands which need to be corrected.
Created attachment 6342 [details] [review] Proposed fix Escaping ampersands.
Created attachment 6702 [details] [review] Bug 7127 - Templates must be valid XHTML Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
QA comment: * how to prove the validity of XHTML ? Is there a jenkin test for that ?
QA: Looks good. Updating status..
QA comment Chris = is there a unit test for XHTML validity ? + merge problem: Applying: Bug 7127 - Templates must be valid XHTML /home/paul/koha.dev/koha-community/.git/rebase-apply/patch:64: space before tab in indent. <a href="/cgi-bin/koha/catalogue/search.pl?idx=hi&q=[% itemloo.itemnumber %]">[% itemloo.countanalytics %] analytics</a> warning: 1 line adds whitespace errors. Using index info to reconstruct a base tree... <stdin>:64: space before tab in indent. <a href="/cgi-bin/koha/catalogue/search.pl?idx=hi&q=[% itemloo.itemnumber %]">[% itemloo.countanalytics %] analytics</a> warning: 1 line applied after fixing whitespace errors. Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt Failed to merge in the changes. Patch failed at 0001 Bug 7127 - Templates must be valid XHTML
Created attachment 7140 [details] [review] Revised for current master
Created attachment 7339 [details] [review] Bug 7127 - Follow-up - Templates must be valid XHTML Corrections to OPAC search results and lists pages for validity errors: duplicate ids, invalidly placed elements, unescaped ampersands. The change to localcovers.js adds a check for <span> in addition to <div> so that the results template can use a <span> The <form> tag has been removed from the inline tag entry markup in order to correct a display error in Internet Explorer 8. Removing it doesn't affect functionality or validity. To test: Validate the HTML of the OPAC results page and an OPAC list contents page (/cgi-bin/koha/opac-shelves.pl?viewshelf=XX). Verify the functinality of adding tags on both pages with the TagsInputOnList system preference enabled. Verify that local cover images are display on search results and detail pages.
Attachment 7339 [details] can be applied and tested independently of attachment 7140 [details] [review].
Attachment 7339 [details] should also fix Bug 7452.
Created attachment 7444 [details] [review] Signed off Patch First patch successfully removes numerous & in templates in favour of &.
Second patch double |html filters on page-numbers.inc; while this may be benign, it won't pass QA.
Marking Passed QA for signed off patch only, since it's a safe template-only change.
Created attachment 7446 [details] [review] Bug 7127 - Follow-up - Templates must be valid XHTML Corrections to OPAC search results and lists pages for validity errors: duplicate ids, invalidly placed elements, unescaped ampersands. The change to localcovers.js adds a check for <span> in addition to <div> so that the results template can use a <span> The <form> tag has been removed from the inline tag entry markup in order to correct a display error in Internet Explorer 8. Removing it doesn't affect functionality or validity. To test: Validate the HTML of the OPAC results page and an OPAC list contents page (/cgi-bin/koha/opac-shelves.pl?viewshelf=XX). Verify the functinality of adding tags on both pages with the TagsInputOnList system preference enabled. Verify that local cover images are display on search results and detail pages. Includes fixes for errors spotted by QA Revisions
i'm not sure what to do with those patches. Should I push the 1st and reset status to need signoff?
Created attachment 7456 [details] [review] [SIGNED OFF] Bug 7127 - Follow-up - Templates must be valid XHTML Corrections to OPAC search results and lists pages for validity errors: duplicate ids, invalidly placed elements, unescaped ampersands. The change to localcovers.js adds a check for <span> in addition to <div> so that the results template can use a <span> The <form> tag has been removed from the inline tag entry markup in order to correct a display error in Internet Explorer 8. Removing it doesn't affect functionality or validity. To test: Validate the HTML of the OPAC results page and an OPAC list contents page (/cgi-bin/koha/opac-shelves.pl?viewshelf=XX). Verify the functinality of adding tags on both pages with the TagsInputOnList system preference enabled. Verify that local cover images are display on search results and detail pages. Includes fixes for errors spotted by QA Revisions Signed-off-by: Ian Walls <koha.sekjal@gmail.com>
Signed off and Passed QA on followup: both attached patches can now be committed.
*** Bug 7452 has been marked as a duplicate of this bug. ***
Created attachment 9760 [details] [review] Bug 7127 - Templates must be valid XHTML Fixing markup errors on OPAC detail page and OPAC results. Also conditionally including ratings JS on OPAC detail.
Created attachment 9788 [details] [review] Bug 7127 - Templates must be valid XHTML Fixing markup errors on OPAC detail page and OPAC results. Also conditionally including ratings JS on OPAC detail. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
QA comment: * this patch does not apply on new/bug_7127 branch, that is too old * template only change, for better XHTML compliance passed QA, will push them on master branch directly
Comment on attachment 9788 [details] [review] Bug 7127 - Templates must be valid XHTML this patch has been pushed
Should we leave this open or better have individual new bugs for new problems? That templates should be valid XHTML is part of the coding guidelines.
(In reply to comment #22) > Should we leave this open or better have individual new bugs for new > problems? I think in general individual bugs work better, and since we're using an HTML5 doctype now this bug should be closed.