I had some incidental cases where diacritical characters a.o. were wrongly encoded in Koha. This appeared to originate from the Template output routine. This routine encodes all variables before sending them out. But merging these values with other non-encoded variables [somewhere in the template process call] may occasionally let Perl make a wrong choice when encoding/decoding. The attached patches are proposed to resolve the situation and make Koha more consistent. Patch 1 Adds specific encoding parameter when creating Template object. Note that the instruction binmode => ':utf8' could have been added also to the process method. But since it outputs to a Perl variable, this is not needed. Finally, instead of the old encoding of all parameters before processing, the output of process is encoded to UTF-8 octets. Note that Perl could have handled this internally, but this would require adding a use open statement, setting IO layers to UTF-8 in all scripts. Patch 2 Add UTF-8 encoding to intranet xslt files in output clause. Patch 3 Add UTF-8 encoding to opac xslt files also. Does also fix head and body tags in MARC21slim2OPACMARCdetail.xsl. Patch 4 Simplifies opac-showmarc template (eliminating opac-bottom include). Makes encoding for card and html view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. Patch 5 Simplifies showmarc template (eliminating intranet-bottom include). Makes encoding for card view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. All patches should be combined. Finding a test case may be quite hard. Please choose some records with special characters. And check in several opac places and in staff that they are displayed correctly. Especially check MARC view/plain MARC view in opac and MARC/Card view in intranet results. Also check normal view in XSLT mode and non-XSLT mode.
Created attachment 12755 [details] [review] Patch 1 for Templates module
Created attachment 12756 [details] [review] Patch 2 for intranet xslt
Created attachment 12757 [details] [review] Patch 3 for opac xslt
Created attachment 12758 [details] [review] Patch 4 for opac-showmarc
Created attachment 12759 [details] [review] Patch 5 for staff showmarc
Created attachment 12986 [details] [review] Bug 8872 Encoding issues in Templates.pm Adds specific encoding parameter when creating Template object. Note that the instruction binmode => ':utf8' could have been added also to the process method. But since it outputs to a Perl variable, this is not needed. Finally, instead of the old encoding of all parameters before processing, the output of process is encoded to UTF-8 octets. Note that Perl could have handled this internally, but this would require adding a use open statement, setting IO layers to UTF-8 in all scripts. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Successfully tested on a record that displayed the UTF-8 "unknown character" glyph before the patch, at least in plain MARC, and the correct character after.
Created attachment 12987 [details] [review] Bug 8872: Add UTF-8 encoding to intranet xslt files Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 12988 [details] [review] Bug 8872 Add UTF-8 encoding to opac xslt files Does also fix head and body tags in MARC21slim2OPACMARCdetail.xsl. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 12989 [details] [review] Bug 8872: Changes for opac-showmarc Simplifies template (eliminating opac-bottom include). Makes encoding for card and html view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 12990 [details] [review] Bug 8872: Changes for intranet showmarc Simplifies template (eliminating intranet-bottom include). Makes encoding for card view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
I'm a little bit reluctant to push this patc for 3.10 because it changes things in the core of Koha. If we miss something, it can have very bad side effect. Do people here agree to wait for after 3.10.0 release ?
(In reply to comment #12) > I'm a little bit reluctant to push this patc for 3.10 because it changes > things in the core of Koha. If we miss something, it can have very bad side > effect. Do people here agree to wait for after 3.10.0 release ? Well, I do not mind holding this patch a little longer. Probably other patches touching core modules should wait now too. About the very bad side effect: We use this in production for some time already. Note that it should still be QAed. Did you do that?
Created attachment 13171 [details] [review] Simplifies template (eliminating intranet-bottom include). Makes encoding for card view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Created attachment 13172 [details] [review] Bug 8872: Changes for intranet showmarc Simplifies template (eliminating intranet-bottom include). Makes encoding for card view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
(In reply to comment #15) > Created attachment 13172 [details] [review] > Bug 8872: Changes for intranet showmarc > > Simplifies template (eliminating intranet-bottom include). Makes encoding > for card view more consistent with approach in Templates module. Rearranges > a few lines in script for consistency and performance. > > Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> i had a small merge conflict, so i rebased this patch against master
Thanks for rebasing! But I discovered now that this patch needs a small followup for opac facets and opac search results paging. Somehow diacritics are not handled consistently there yet..
Created attachment 13173 [details] [review] Subject: [PATCH] Bug 8872: Changes for opac-showmarc Simplifies template (eliminating opac-bottom include). Makes encoding for card and html view more consistent with approach in Templates module. Rearranges a few lines in script for consistency and performance. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
(In reply to comment #18) > Created attachment 13173 [details] [review] > Subject: [PATCH] Bug 8872: Changes for opac-showmarc > > Simplifies template (eliminating opac-bottom include). > Makes encoding for card and html view more consistent with approach in > Templates module. > Rearranges a few lines in script for consistency and performance. > > Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> meh, and a rebase for this one too...
(In reply to comment #17) > Thanks for rebasing! > But I discovered now that this patch needs a small followup for opac facets > and opac search results paging. Somehow diacritics are not handled > consistently there yet.. wow, great spotting! :) Marcel, do you suggest we wait for a follow-up patch before QA-ing? or, should we QA this patch-set 'as is' now?
Please go ahead. A followup will need additional signoff ;)
Mason: Perhaps we should still wait a little. I keep finding some instances where input from outside (like in a url parameter) is not decoded when passing it to perl. That should be the ideal way to approach it, but if we not cover all those instances these strings will be double encoded. Like opac facets coming from zebra render without decoding. Etc..
Created attachment 13244 [details] [review] Bug 8872: Changes for opac-showmarc
Comment on attachment 12986 [details] [review] Bug 8872 Encoding issues in Templates.pm Marking the first patch (for Templates.pm) as obsolete.
I have removed the patch for Templates.pm since this would require some more changes to Koha than planned for this report. Koha should follow the principle: decode everything coming in from outside, encode everything when sending it to outside. But in several places this does not happen: especially for url parameters and template file contents. The first patch did solve the template file contents but since it would not touch all places where url parameters are passed in, it is not fully working. What remains in this report, is a fix for xslt files and opac-showmarc and showmarc. I think that in this form the patches could be pushed (after QA). I will ask Jared to confirm since he signed off on the full set of five patches.
Created attachment 13245 [details] [review] Bug 8872: Changes for intranet showmarc
Created attachment 13248 [details] [review] Bug 8872: Changes for intranet showmarc Changing one line only: adding empty string to remove uninit-warning from log.
(In reply to comment #25) > I have removed the patch for Templates.pm since this would require some more > changes to Koha than planned for this report. > Koha should follow the principle: decode everything coming in from outside, > encode everything when sending it to outside. But in several places this > does not happen: especially for url parameters and template file contents. > The first patch did solve the template file contents but since it would not > touch all places where url parameters are passed in, it is not fully working. > > What remains in this report, is a fix for xslt files and opac-showmarc and > showmarc. I think that in this form the patches could be pushed (after QA). > > I will ask Jared to confirm since he signed off on the full set of five > patches. Agreed.
(In reply to comment #21) > Please go ahead. > A followup will need additional signoff ;) tested OPAC and staff with no regressions, passing QA... mason@xen1:~/g/head$ koha-qa.pl -c 4 testing 4 commit(s) (applied to 62ce0ba 'Merge branch 'bug_8842' into 3.12-ma') d86e82b 8872 Changes for intranet showmarc - catalogue/showmarc.pl - koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/showmarc.tt 03ac92c 8872 Changes for opac-showmarc - koha-tmpl/opac-tmpl/prog/en/modules/opac-showmarc.tt - opac/opac-showmarc.pl 7b0279c Bug 8872 Add UTF-8 encoding to opac xslt files - koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl - koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACMARCdetail.xsl - koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACResults.xsl - koha-tmpl/opac-tmpl/prog/en/xslt/NORMARCslim2OPACDetail.xsl - koha-tmpl/opac-tmpl/prog/en/xslt/NORMARCslim2OPACResults.xsl - koha-tmpl/opac-tmpl/prog/en/xslt/UNIMARCslim2OPACDetail.xsl - koha-tmpl/opac-tmpl/prog/en/xslt/UNIMARCslim2OPACResults.xsl b373dbc Bug 8872: Add UTF-8 encoding to intranet xslt files - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2ATOM.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2DC.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2MODS.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2OAIDC.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2RDFDC.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2RDFDC.xsl.1 - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2RSS2.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetResults.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim9522852.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2OAIDC.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2intranetDetail.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2intranetResults.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARC_compact.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2OAIDC.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2intranetDetail.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2intranetResults.xsl - koha-tmpl/intranet-tmpl/prog/en/xslt/oilsMARC21slim2HTML.xsl * catalogue/showmarc.pl OK * opac/opac-showmarc.pl OK * koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/showmarc.tt OK * koha-tmpl/opac-tmpl/prog/en/modules/opac-showmarc.tt OK * koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACDetail.xsl OK * koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACMARCdetail.xsl OK * koha-tmpl/opac-tmpl/prog/en/xslt/MARC21slim2OPACResults.xsl OK * koha-tmpl/opac-tmpl/prog/en/xslt/NORMARCslim2OPACDetail.xsl OK * koha-tmpl/opac-tmpl/prog/en/xslt/NORMARCslim2OPACResults.xsl OK * koha-tmpl/opac-tmpl/prog/en/xslt/UNIMARCslim2OPACDetail.xsl OK * koha-tmpl/opac-tmpl/prog/en/xslt/UNIMARCslim2OPACResults.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2ATOM.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2DC.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2MODS.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2OAIDC.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2RDFDC.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2RSS2.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetResults.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim9522852.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2OAIDC.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2intranetDetail.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/NORMARCslim2intranetResults.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARC_compact.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2OAIDC.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2intranetDetail.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2intranetResults.xsl OK * koha-tmpl/intranet-tmpl/prog/en/xslt/oilsMARC21slim2HTML.xsl OK
This patch has been pushed to master.
Patch pushed to branch 3.10.x (I much more prefer those ones than the previous ones -from side-effect risk POV- !)
Pushed to 3.8.x, will be in 3.8.7
Patches do not apply to 3.6.x, please reopen and submit a patch for 3.6.x if necessary. Thanks!