Created attachment 4560 [details]
member.pl (Ukrainian)
Created attachment 4561 [details]
members-home.pl (Russian)
Created attachment 4562 [details]
members-home.pl (Greek)
Created attachment 4563 [details]
members-home.pl (French)
Created attachment 4564 [details]
members-home.pl (Arabic)
Created attachment 5962 [details]
-- /cgi-bin/koha/members/members-home.pl (Russian)
Tested in Koha 3.05.00.011.
(In reply to comment #6) > Created attachment 5962 [details] > -- /cgi-bin/koha/members/members-home.pl (Russian) > > Tested in Koha 3.05.00.011. Encoding of members.tt looks good: <pre> [% INCLUDE 'doc-head-open.inc' %] <title>Коха › Посетители [% IF ( searching ) %] › Результаты поиска [% END %]</title>[% INCLUDE 'doc-head-close.inc' %] </head> <body>[% INCLUDE 'header.inc' %] [% INCLUDE 'patron-search.inc' %] <div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Начало</a>› [% IF ( searching ) %]<a href="/cgi-bin/koha/members/members-home.pl">Посетители</a>› Результаты поиска[% ELSE %]Посетители[% END %]</div> <div id="doc2" class="yui-t7"> <div id="bd"> <div id="yui-main"> <div class="yui-b"> <div class="yui-g">[% INCLUDE 'patron-toolbar.inc' %] [% IF ( no_add ) %]<div class="dialog alert"><h3>Cannot add patron</h3>[% IF ( no_branches ) %]<p>There are <strong>no libraries defined</strong>. [% IF ( CAN_user_parameters ) %]Please <a href="/cgi-bin/koha/admin/branches.pl">add a library</a>.[% ELSE %]An administrator must define at least one library.[% END %]</p>[% END %] [% IF ( no_categories ) %]<p>There are <strong>no patron categories defined</strong>. [% IF ( CAN_user_parameters ) %]Please <a href="/cgi-bin/koha/admin/categorie.pl">add a patron category</a>.[% ELSE %]An administrator must define at least one patron category.[% END %]</p>[% END %]</div>[% END %] <div class="browse">Поиск по фамилии: [% FOREACH letter IN letters %] <a href="/cgi-bin/koha/members/member.pl?quicksearch=1&surname=[% letter.letter %]">[% letter.letter %]</a>[% END %] </div>[% IF ( resultsloop ) %] <div id="searchheader"> <h3>Результаты от [% from %] по [% to %] с [% numresults %] для поиска по запросу [% IF ( member ) %]'<span class="ex">[% member %]</span>'[% END %][% IF ( surname ) %]'<span class="ex">[% surname %]</span>'[% END %]</h3></div> <div class="searchresults"> <table id="memberresultst"> <thead> <tr> <th>Билет</th> <th>Наименование </th> <th>Категория</th> <th>Библиотека</th> <th>Истекает на</th> <th>Просрочки/Выдачи</th> <th>Пеня</th> <th>Примечание для оборота</th> <th> </th> </tr> </thead> <tbody>[% FOREACH resultsloo IN resultsloop %] [% IF ( resultsloo.overdue ) %] <tr class="problem">[% ELSE %] [% UNLESS ( loop.odd ) %] <tr class="highlight">[% ELSE %] <tr>[% END %] [% END %] <td>[% resultsloo.cardnumber %]</td> <td style="white-space: nowrap;"><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% resultsloo.borrowernumber %]">[% resultsloo.surname %], [% resultsloo.firstname %]</a> <br />[% resultsloo.address %][% IF ( resultsloo.address2 ) %]<br />[% resultsloo.address2 %][% END %][% IF ( resultsloo.city ) %]<br />[% resultsloo.city %][% IF ( resultsloo.state ) %],[% END %][% END %][% IF ( resultsloo.state ) %] [% resultsloo.state %][% END %] [% IF ( resultsloo.zipcode ) %] [% resultsloo.zipcode %][% END %][% IF ( resultsloo.country ) %], [% resultsloo.country %][% END %]</td> <td>[% resultsloo.category_description %] ([% resultsloo.category_type %])</td> <td>[% resultsloo.branchcode %]</td> <td>[% resultsloo.dateexpiry %]</td> <td>[% IF ( resultsloo.overdues ) %]<span class="overdue"><strong>[% resultsloo.overdues %]</strong></span>[% ELSE %][% resultsloo.overdues %][% END %]/[% resultsloo.issues %]</td> <td>[% IF ( resultsloo.fines < 0 ) %]<span class="credit">[% resultsloo.fines %]</span>[% ELSIF resultsloo.fines > 0 %] <span class="debit"><strong>[% resultsloo.fines %]</strong></span> [% ELSE %] [% resultsloo.fines %] [% END %]</td> <td>[% resultsloo.borrowernotes %]</td> <td>[% IF ( resultsloo.category_type ) %] <a href="/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=[% resultsloo.borrowernumber %]&category_type=[% resultsloo.category_type %]">Редактировать</a> [% ELSE %] <!-- try with categorycode if no category_type --> [% IF ( resultsloo.categorycode ) %] <a href="/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=[% resultsloo.borrowernumber %]&categorycode=[% resultsloo.categorycode %]">Редактировать</a> [% ELSE %] <!-- if no categorycode, set category_type to A by default --> <a href="/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=[% resultsloo.borrowernumber %]&category_type=A">Редактировать</a>[% END %] [% END %]</td> </tr>[% END %] </tbody> </table> <div class="pages">[% IF ( multipage ) %][% paginationbar %][% END %]</div> </div>[% ELSE %] [% IF ( searching ) %] <div class="dialog alert">Ничего не найдено</div>[% END %] [% END %] </div> </div> <div class="yui-g">[% INCLUDE 'members-menu.inc' %] </div> </div> </div> <script type="text/javascript" src="[% themelang %]/lib/jquery/plugins/jquery.tablesorter.min.js"></script> <script type="text/javascript">//<![CDATA[ $(document).ready(function() { $("#memberresultst").tablesorter({ sortList: [[1,0]], widgets: ['zebra'] }); }); //]]> </script>[% INCLUDE 'intranet-bottom.inc' %] </pre> Created attachment 6041 [details] [review] Clean up Koha utf-8 string handling We are having same problem, but in our case we don't use localized templates but have utf-8 characters inside MySQL which get double utf-8 encoded before they are sent to browser. If you feel brave, you can try attached patch with fixes problem for us. Below is short summary of changes from patch: This patch tries to clean up utf-8 handling in Koha. In current implementation (mostly commented out in this patch) uses heuristic to guess which strings need decoding from utf-8 to binary representation and doesn't support utf-8 characters in templates and has problems with utf-8 data from database. With this changes, Koha perl code always uses utf-8 encoding correctly. All incomming data from database is allready correctly marked as utf-8, and decoding of utf8 is required only from Zebra and XSLT transfers which don't set utf-8 flag correctly. For output, standard perl :utf8 handler is used removing various "wide character" warnings as side-effect. There is a patch provided, I update to "needs signoff" However, I must point that I can't reproduce the behaviour in french anymore. There used to be a problem on this specific page (member-home.pl) a few weeks ago, it's now fixed. I didn't reported the problem, because I thought it could be my translation that was broken. However, I just saw today that, if the list of categorycodes contains a diacritic, then the New button get them double encoded (see attachment) Created attachment 6065 [details]
encoding problem in member-home.pl in 3.6.0 (another one)
I apply this patch to 3.6.0 installed. Unfortunately, after this patch, the system of translated templates becomes completely broken. (also tried to generate the translated templates again - does not help). One plus - the result of patron search now is well displayed - see new attached member.pl.png and others. Created attachment 6070 [details]
member.pl (Ukrainian) after patch1
Created attachment 6071 [details]
OPAC search (Ukrainian, after patch1)
Created attachment 6072 [details]
Staff home page (Ukrainian) after patch1
Created attachment 6073 [details] [review] remove unused code, cleanup ident and correctly mark templates as utf-8 Attached patch fixes template encoding. It's artificially large since I also included removal of unused code, but only relevant change to last version is ENCODING => 'utf8' in C4::Template around like 65 in Template->new. This change is based on utf-8 hint from Template::FAQ since our templates don't have BOM The result of testing the second patch. I checked a few dozen pages opac and staff and saw: 1) pages of patron home andd search now looks good; 2) most of the tested pages too good; 3) a few pages or places where there are still problems - a) the library name on each page; b) search result in the catalog (opac and staff); c) page modify server Z39-50; d) page with system parameters (descriptions from ..pref..po) (also added screenshots for this) Created attachment 6076 [details]
OPAC search (Ukrainian, after patch2)
Created attachment 6077 [details]
STAFF search (Ukrainian, after patch 2)
Created attachment 6078 [details]
member search (ukrainian, example of good page after patch 2)
Created attachment 6079 [details]
z39-50 edit page (ukrainian, after patch 2)
Created attachment 6080 [details]
preferences page (ukrainian, after patch 2)
Created attachment 6083 [details] [review] added session, CGI queries and all Zebra/PAR utf8 decode Thanks for report. Attached patch adds: - encoding for data from session (like library name) - utf8 force for CGI queries which include utf-8 text[1] - decode all utf8 results from Zebra/PAR 1: I would guess that I missed some of pages where utf-8 text is posted to Koha, but setting it everywhere would break upload according to CGI docs, so I'm conservative here. I applied the patch and re-checked as many pages as able. It seems there are only two places where the problem appears with encoding, it is 1) Search OPAC and 2) Search STAFF. Your patch however partially corrected these pages (on added 2 screenshot I marked these improvements by green). Created attachment 6084 [details]
opac_search.pl (ukrainian, after patch 3)
Created attachment 6085 [details]
staff search.pl (ukrainian, after patch 3)
This is very strange. Do tables in your database use utf-8 encoding? Could you dump your Koha database somewhere and mail me link to dpavlin@rot13.org so I can test with your data? I already installed languages, so this would help me in debugging without bothering you for each update. Created attachment 6108 [details]
addbiblio.pl dublicate warning (ukrainian, after patch 3)
yet another place
Just some "fast" preliminary QA remarks from glimsing through the patches. I agree that encoding data in Template.pm is actually not correct and that specifying utf8 on std output with binmode or use open is a better practice than what is done in current Koha. Check showmarc.p/opac-showmarc.pl where I recently decoded output from template to prevent double encoding. This is a place that therefore is touched also by your patch. Another recent bug report pointed out that using :encoding(utf8) is preffered above :utf8. The latter form is a lazy one in Perl. I saw that you decode output from the Zoom connection in Search.pm. Without further investigation I am inclined to think that this is not (always) correct. Apparently, you assume that the output from Zoom is utf8 octets. Isn't it Latin and doesn't Zoom allow you to pass a charset parameter like charset=latin,utf8 in order to change encoding? Could you verify? Still another comment came up: It seems that you only need to decode data from XSLT processor if you did not specify UTF-8 already on the stylesheet itself. In most cases (or all?) the xslt templates already do that. It seems there is something wrong with the attached patch: [~/kohaclone (nso_6554)]> git bz apply 6554 Bug 6554 - Broken encoding in members home/search page added session, CGI queries and all Zebra/PAR utf8 decode Apply? [yn] y Patch format detection failed. Patch left in /tmp/added-session-CGI-queries-and-all-ZebraPAR-utf8-de-iXfemD.patch Dobrica, Could you please resubmit the patch for this bug in git format? Created attachment 10118 [details] [review] Bug 6554 - added session CGI queries and all ZebraPAR utf8_decode Rebased from added-session-CGI-queries-and-all-ZebraPAR-utf8-de-O_t1Z2.patch Original-Author: Dobrica Pavlinusic <dpavlin@rot13.org> I installed uk-UA (Ukrainian) and tried to replicate the display problems. I checked the item type page, creating new itemtypes with "ukrainian" (copied some strings from other places) descriptions, I checked the Z39.50 page and the system preferences. I am not sure how to test this, without being able to replicate the problem. Dobrica, can you help? Is this magically resolved without the patch? If not, please somebody provide a test plan pointing out 1. how to reproduce the problem 2. what to look for after patching I'm setting this to "in discussion" in the hope that something constructive will happen. Created attachment 16780 [details] [review] Bug 6554 - make Koha internally utf-8 clean In current implementation (mostly commented out in this patch) uses heuristic to guess which strings need decoding from utf-8 to binary representation and doesn't support utf-8 characters in templates and has problems with utf-8 data from database. With this changes, Koha perl code always uses utf-8 encoding correctly. All incomming data from database is allready correctly marked as utf-8, and decoding of utf8 is required only from Zebra and XSLT transfers which don't set utf-8 flag correctly. For output, standard perl :utf8 handler is used removing various "wide character" warnings as side-effect. Test scenario: 1. make sure that you have utf-8 characters in your biblio records, patrons, categories etc. 2. try to search records on intranet and opac which contain utf-8 characters 3. install language which has utf-8 characters, e.g. uk-UA dpavlin@koha-dev:/srv/koha/misc/translator(bug_6554) $ PERL5LIB=/srv/koha/ perl translate install uk-UA 4. switch language to uk-UA and verify that templates display correctly 5. test search and Z39.50 search and verify that caracters are correct Created attachment 16797 [details] [review] Bug 6554 - make Koha internally utf-8 clean In current implementation (mostly commented out in this patch) uses heuristic to guess which strings need decoding from utf-8 to binary representation and doesn't support utf-8 characters in templates and has problems with utf-8 data from database. With this changes, Koha perl code always uses utf-8 encoding correctly. All incomming data from database is allready correctly marked as utf-8, and decoding of utf8 is required only from Zebra and XSLT transfers which don't set utf-8 flag correctly. For output, standard perl :encoding(utf8) handler is used so it also removes various "wide character" warnings as side-effect. Test scenario: 1. make sure that you have utf-8 characters in your biblio records, patrons, categories etc. 2. try to search records on intranet and opac which contain utf-8 characters 3. install language which has utf-8 characters, e.g. uk-UA dpavlin@koha-dev:/srv/koha/misc/translator(bug_6554) $ PERL5LIB=/srv/koha/ perl translate install uk-UA 4. switch language to uk-UA and verify that templates display correctly 5. test search and Z39.50 search and verify that caracters are correct This version uses more strict :encoding(utf8) and removes unneeded decode of XSLT which comes back as correctly encoded strings. Created attachment 16857 [details] [review] [SIGNED-OFF] Bug 6554 - make Koha internally utf-8 clean In current implementation (mostly commented out in this patch) uses heuristic to guess which strings need decoding from utf-8 to binary representation and doesn't support utf-8 characters in templates and has problems with utf-8 data from database. With this changes, Koha perl code always uses utf-8 encoding correctly. All incomming data from database is allready correctly marked as utf-8, and decoding of utf8 is required only from Zebra and XSLT transfers which don't set utf-8 flag correctly. For output, standard perl :encoding(utf8) handler is used so it also removes various "wide character" warnings as side-effect. Test scenario: 1. make sure that you have utf-8 characters in your biblio records, patrons, categories etc. 2. try to search records on intranet and opac which contain utf-8 characters 3. install language which has utf-8 characters, e.g. uk-UA dpavlin@koha-dev:/srv/koha/misc/translator(bug_6554) $ PERL5LIB=/srv/koha/ perl translate install uk-UA 4. switch language to uk-UA and verify that templates display correctly 5. test search and Z39.50 search and verify that caracters are correct Signed-off-by: Owen Leonard <oleonard@myacpl.org> I followed the test plan, adding utf-8 characters to library names, patron categories, titles, and authorized values. I tried the uk-UA translation and everything looked good. When performing Z39.50 searches for titles containing utf-8 characters I got results which were still occasionally contaminated with dummy characters [?] but I assume this is Z39.50's fault not the patch's. Good to see this patch! I once tried to get a similar change into Koha with bug 8872. I stumbled over the numerous occurrences where url parameters in scripts are not properly decoded. Finally, the change in Templates was obsoleted. Note that I see you working with binmode in Output.pm; wouldn't it be better to just utf8-encode the data before you print in output_with_http_headers to prevent some kind of improper use? Furthermore, are you really sure you covered all instances in scripts? When using XSLT, I now have a lot of troubled characters. Note that the xslt files now do contain UTF-8 in the headers. Probably, we are mixing encoded data with decoded data now. Could you please have a look and send a followup? This would work for me: Output.pm (binmode removed), output_with_http_headers: $data =~ s/\&\;amp\; /\&\; /g; utf8::encode($data); print $query->header($options), $data; XSLT.pm (XSLTParse4Display): my $newxmlrecord = $stylesheet->{$xslfilename}->output_as_chars($results); ? utf8::decode($newxmlrecord); return $newxmlrecord; Note that output_string seems to be deprecated. Decoding newxmlrecord seems to be not necessary.. (In reply to comment #37) > When performing Z39.50 searches for titles containing utf-8 characters I > got results which were still occasionally contaminated with dummy > characters [?] but I assume this is Z39.50's fault not the patch's. Sure that your settings per server are correct? Lot of US targets still use MARC-8, some UTF-8.. Created attachment 16905 [details] [review] Bug 6554 - followup - encode output, XSLT encoding from templates Encode output data instead of using binmode to be more compliant with various usage (e.g. plack). XSLT output_string seems to be depriciated call, decoding of newxmlrecord seems to be not necessary. (In reply to comment #38) > Could you please have a look and send a followup? I totally agree so I submitted followup with correct acreditation. Is & regexp needed? I added warn, and don't see any output from it. Created attachment 16929 [details] [review] Bug 6554 QA Followup Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Removing binmode, now encoding data in output_with_http_headers. Replaced output_string by output_as_chars in XSLTParse4Display. Dobrica, We are not there yet. I went diagonally through opac and could not find a problem. But in staff I found this one: Do a patron search on name with diacritic, say musée. No patron matched 'musée' Wrong parameter decoding. Created attachment 16930 [details] [review] Merge branch 'bug_9919' into 3.12-master Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Simple fix for a bad problem. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com> Comment on attachment 16930 [details] [review] Merge branch 'bug_9919' into 3.12-master Oops Created attachment 16931 [details] [review] Bug 6554 - make Koha internally utf-8 clean In current implementation (mostly commented out in this patch) uses heuristic to guess which strings need decoding from utf-8 to binary representation and doesn't support utf-8 characters in templates and has problems with utf-8 data from database. With this changes, Koha perl code always uses utf-8 encoding correctly. All incomming data from database is allready correctly marked as utf-8, and decoding of utf8 is required only from Zebra and XSLT transfers which don't set utf-8 flag correctly. For output, standard perl :encoding(utf8) handler is used so it also removes various "wide character" warnings as side-effect. Test scenario: 1. make sure that you have utf-8 characters in your biblio records, patrons, categories etc. 2. try to search records on intranet and opac which contain utf-8 characters 3. install language which has utf-8 characters, e.g. uk-UA dpavlin@koha-dev:/srv/koha/misc/translator(bug_6554) $ PERL5LIB=/srv/koha/ perl translate install uk-UA 4. switch language to uk-UA and verify that templates display correctly 5. test search and Z39.50 search and verify that caracters are correct Signed-off-by: Owen Leonard <oleonard@myacpl.org> I followed the test plan, adding utf-8 characters to library names, patron categories, titles, and authorized values. I tried the uk-UA translation and everything looked good. When performing Z39.50 searches for titles containing utf-8 characters I got results which were still occasionally contaminated with dummy characters [?] but I assume this is Z39.50's fault not the patch's. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 16932 [details] [review] Bug 6554 QA Followup Removing binmode, now encoding data in output_with_http_headers. Replaced output_string by output_as_chars in XSLTParse4Display. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Still another one: look for a diacritic in the title within Serials module. Another encoding error visible. Acquisitions: look for é in the vendor field. (In reply to comment #50) > Acquisitions: look for é in the vendor field. Order search too. Administration: look for pref é. Does not exist, but encoding error. That makes them all for a short tour through the staff client (with comment 44, 49 and 50). Please check if you can find another one :) (In reply to comment #51) > (In reply to comment #50) > > Acquisitions: look for é in the vendor field. > Order search too. > > Administration: look for pref é. Does not exist, but encoding error. > > That makes them all for a short tour through the staff client (with comment > 44, 49 and 50). Please check if you can find another one :) I didn't really dive into this issue, but encoding errors don't appears without the last patch ("QA Followup"), at least for member and syspref search. (In reply to comment #52) > I didn't really dive into this issue, but encoding errors don't appears > without the last patch ("QA Followup"), at least for member and syspref > search. I will check it too. (In reply to comment #53) > (In reply to comment #52) > > I didn't really dive into this issue, but encoding errors don't appears > > without the last patch ("QA Followup"), at least for member and syspref > > search. > > I will check it too. Jonathan had a line commented out. So this is no issue. Created attachment 16960 [details] [review] 6554 Followup for preferences.pl Created attachment 16961 [details] [review] Bug 6554 Followup for circulation patron search Adds decoding for patron name. Test plan: Go to Circulation. Enter a name with a diacritic. Find that patron or get a message with the name you entered without converted characters. Created attachment 16962 [details] [review] 6554 Followup for preferences.pl Decodes searchfield. Test plan: Look for é in preferences. You should no longer see: é Created attachment 16963 [details] [review] Bug 6554: Followup for serial search Adds decoding for title, publisher and vendor. Test plan: Go to serials. Search for a diacritic in title. Check. Go to Advanced search (in Serials). Search for diacritic in title, vendor or publisher. Check. Created attachment 16964 [details] [review] Bug 6554: Followup for acquisitions Adds decoding for url parameter. Test plan: Search for expressions with diacritics in vendor search, orders search. Also try Orders search, Advanced search (within Acq). Check what you see. I resolved the points that i found. This should still receive some signoff. And some people willing to check if they can still find another problem with url parameter handling.. There is something with: 6554 Followup for preferences.pl Apply? [yn] y fatal: cannot convert from UTF-8utf-8 to UTF-8 Patch left in /tmp/6554-Followup-for-preferencespl-SMvTX9.patch Will submit that one again. Created attachment 16966 [details] [review] Bug 6554: Followup for preferences.pl Decodes searchfield. Test plan: Look for e accent in preferences. You should no longer see converted chars. Created attachment 16968 [details] [review] Bug 6554: Followup for preferences.pl Decodes searchfield. Test plan: Look for e accent in preferences. You should no longer see converted chars. Created attachment 16969 [details] [review] Bug 6554 Followup for circulation patron search Adds decoding for patron name. Test plan: Go to Circulation. Enter a name with a diacritic. Find that patron or get a message with the name you entered without converted characters. Resubmitted the followups for acq and prefs. They did not need URI::Escape after all. CGI handles that correctly. Simplification :) M. de Rooy thanks a lot for your effort in locating places which need additional utf-8 encoding and follow-up patches. I would prefer to leave utf-8 decoding of parameters to CGI module using: use CGI qw(-utf8); This ensures that we are decoding all parameters and produce much less code. Additional benefit is that we are not missing any parameters, possibly creating undecoded params which can break encoding down the road. Extract from CGI documentation: -utf8 This makes CGI.pm treat all parameters as UTF-8 strings. Use this with care, as it will interfere with the processing of binary uploads. It is better to manually select which fields are expected to return utf-8 strings and convert them using code like this: use Encode; my $arg = decode utf8=>param('foo'); Can I squash your followups in one patch and use CGI qw(-utf8) to solve it? I think we might also benefit from README.utf-8 which explains how to handle encoding in Koha for easier reference in future. Created attachment 16984 [details] [review] [SIGNED-OFF] Bug 6554 - make Koha internally utf-8 clean In current implementation (mostly commented out in this patch) uses heuristic to guess which strings need decoding from utf-8 to binary representation and doesn't support utf-8 characters in templates and has problems with utf-8 data from database. With this changes, Koha perl code always uses utf-8 encoding correctly. All incomming data from database is allready correctly marked as utf-8, and decoding of utf8 is required only from Zebra and XSLT transfers which don't set utf-8 flag correctly. For output, standard perl :encoding(utf8) handler is used so it also removes various "wide character" warnings as side-effect. Test scenario: 1. make sure that you have utf-8 characters in your biblio records, patrons, categories etc. 2. try to search records on intranet and opac which contain utf-8 characters 3. install language which has utf-8 characters, e.g. uk-UA dpavlin@koha-dev:/srv/koha/misc/translator(bug_6554) $ PERL5LIB=/srv/koha/ perl translate install uk-UA 4. switch language to uk-UA and verify that templates display correctly 5. test search and Z39.50 search and verify that caracters are correct Signed-off-by: Owen Leonard <oleonard@myacpl.org> I followed the test plan, adding utf-8 characters to library names, patron categories, titles, and authorized values. I tried the uk-UA translation and everything looked good. When performing Z39.50 searches for titles containing utf-8 characters I got results which were still occasionally contaminated with dummy characters [?] but I assume this is Z39.50's fault not the patch's. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Already signed, add mine. Created attachment 16985 [details] [review] [SIGNED-OFF] Bug 6554 QA Followup Removing binmode, now encoding data in output_with_http_headers. Replaced output_string by output_as_chars in XSLTParse4Display. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> No errors. Created attachment 16986 [details] [review] [SIGNED-OFF] Bug 6554: Followup for serial search Adds decoding for title, publisher and vendor. Test plan: Go to serials. Search for a diacritic in title. Check. Go to Advanced search (in Serials). Search for diacritic in title, vendor or publisher. Check. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works ok, no errors. Created attachment 16987 [details] [review] [SIGNED-OFF] Bug 6554: Followup for acquisitions Adds decoding for url parameter. Test plan: Search for expressions with diacritics in vendor search, orders search. Also try Orders search, Advanced search (within Acq). Check what you see. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Created attachment 16988 [details] [review] [SIGNED-OFF] Bug 6554: Followup for preferences.pl Decodes searchfield. Test plan: Look for e accent in preferences. You should no longer see converted chars. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works ok, no errors. Created attachment 16989 [details] [review] [SIGNED-OFF] Bug 6554 Followup for circulation patron search Adds decoding for patron name. Test plan: Go to Circulation. Enter a name with a diacritic. Find that patron or get a message with the name you entered without converted characters. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> I found that this solves the problem reported in Bug 9579. (In reply to comment #73) > I found that this solves the problem reported in Bug 9579. Thanks, Bernardo. If you are sure about solving the other problem, please mark it as such on that report. I try to have a look too tomorrow. (In reply to comment #74) > (In reply to comment #73) > > I found that this solves the problem reported in Bug 9579. > Thanks, Bernardo. > If you are sure about solving the other problem, please mark it as such on > that report. I try to have a look too tomorrow. I have my dev install loaded with a sample of arabic records (from Bug 9579), also did an install trying to figure out (using chr|icu, dom|grs1). When testing I tried a search and then the faceted search was right (that's the problem in 9579). So, your changes apparently solved that. I'll report on that bug. Thank you Bernardo! From the title of this bug, I could never imagine it solved the problem of arabic facets... Mathieu Saby (In reply to comment #76) > Thank you Bernardo! From the title of this bug, I could never imagine it > solved the problem of arabic facets... > > Mathieu Saby I agree that we should change the title of this report. It started with wrong encoding of members page, but it corrects UTF-8 handling in processing templates all through Koha. Renaming it from [Broken encoding in members home/search page] to: [Resolve encoding problems with corrected UTF8 handling in templates] If you have something better, feel free.. (In reply to comment #66) > I would prefer to leave utf-8 decoding of parameters to CGI module using: > use CGI qw(-utf8); > This ensures that we are decoding all parameters and produce much less code. > Additional benefit is that we are not missing any parameters, possibly > creating undecoded params which can break encoding down the road. > > Use this with care, as it will interfere with the processing of binary > uploads. It is better to manually select which fields are expected to return > utf-8 strings and convert them using code like this: > my $arg = decode utf8=>param('foo'); > > Can I squash your followups in one patch and use CGI qw(-utf8) to solve it? > > I think we might also benefit from README.utf-8 which explains how to handle > encoding in Koha for easier reference in future. Thanks, Dobrica. The choice between -utf8 flag for CGI and individual decodes is arbitrary. The utf8 flag may do some unneeded decodes and could affect binary uploads, but I am not sure if that really interferes with Koha somewhere. The decode option is marked as safer, adds little more code but probably does less decodes. Disadvantage is forgetting to decode one parameter. At this moment in time, I would prefer to leave the patches on this report as they are now. We tested; your change should go in now. I will open up a new report to catch the remaining places of incorrect parameter encoding. And will send a short mail to the dev list for further opinions on the best practice in this regard. This is easily addressed later. I also tested this with record containing Chinese characters. No problem with displaying the record, editing and saving. Dobrica: Note that some decodes are not always visible in the script. Addbiblio for instance 'hides' them in the Biblio module: TransformHtmlToMarc. Consistency in Koha is a large and continuing project :) QA Comment: This report is pending for some time now. Dobrica's change (incl. the first followup) is a great fix for UTF-8 handling in template processing. We make Koha adhere to "Decode what goes in, encode what goes out". It is a welcome fix for non-English languages with diacritics. But it does have quite some impact. All occurrences where cgi parameters with utf8 chars not being decoded, will show up. Note that you could argue that these place are actually existing bugs that were hidden by the templates bug. My strong recommendation is nevertheless to push this patch. We have repaired the most important and visible occurrences of wrong parameter handling. I opened up a new report, bug 9951, to add new ones. I checked the staff client via interface and went through all opac scripts. With tests from Owen, Dobrica, Bernardo and myself with various languages and characters, this patch has a solid base. So, have courage, RM :) Code looks good to me. qa script does not complain. Passed QA This patch has been pushed to master. This patch breaks Koha.. On login you receive Modification of a read-only value attempted at /home/cc/kohaclone/C4/Context.pm line 1107. Problem is the call to decode within map $_ is an alias to a value which may be read-only Hi Colin, I just checked and can log into master just fine - is there maybe another condition that needs to be met to make the problem occur? Created attachment 17179 [details] [review] Bug 6554: Followup for decoding in userenv Thanks to Colin. He reported this problem: Modification of a read-only value attempted at C4/Context.pm line 1107. $_ is an alias to a value which may be read-only in the map call. Could not reproduce this error, but this should prevent it. Colin: I cannot reproduce either. But could you test the latest patch attached? Thanks. I think the problem is with perls greater than 5.10 the map documentation suggests you should use a loop in this case. I' test and let you know I cannot reproduce either with perl v.5.14, but I confirm the code should be changed. If utf8::decode($_) modifies $_, the error will occur. However, I think this patch should be in a different report. (In reply to comment #86) > I think the problem is with perls greater than 5.10 the map documentation > suggests you should use a loop in this case. I' test and let you know could it be a problem of a login with mysql user/password ? There are problems/errors like this sometimes with the mysql login, that is borrowernumber=0 and many other infos missing. (In reply to comment #87) > However, I think this patch should be in a different report. I left it here because the actual change with the map statement is introduced in this report. It is not a side effect. (In reply to comment #88) > could it be a problem of a login with mysql user/password ? There are > problems/errors like this sometimes with the mysql login, that is > borrowernumber=0 and many other infos missing. I had no problems with the install login.. Last patch needs signoff. Created attachment 17186 [details] [review] Proposed Patch I've tested Marcel's patch and it works...however I've also sent my own version which removes the map. The reason is that multi-statement blocks in maps can lead to maintenance headaches and are a bit harder to track down if an error occurs (perlcritic on stricter settings complains). The map doc suggests a loop for this case. I've kept the comment in but on CGI::Session's homepage the utf8 bugs are marked as resolved (In reply to comment #92) > Created attachment 17186 [details] [review] [review] > Proposed Patch > > I've tested Marcel's patch and it works...however I've also sent my own > version which removes the map. The reason is that multi-statement blocks in > maps can lead to maintenance headaches and are a bit harder to track down if > an error occurs (perlcritic on stricter settings complains). The map doc > suggests a loop for this case. > > I've kept the comment in but on CGI::Session's homepage the utf8 bugs are > marked as resolved OK I will check it right away. Thanks. Created attachment 17187 [details] [review] Bug 6554 Fix error caused by modifying $_ in a map Error 'Modification of a read-only value attempted' triggered on login because of manipulation of $_ in the map Moved the mod to a loop as recommended in the doc for map Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> QA Comment: Patch of Colin is a followup rewriting the map statement introduced here in setuserenv (Context.pm). Several people could not reproduce the problem Colin reported. But this is a code improvement and fixes his problem. Passed QA Hiو I found an other problem related to this bug that not solved yet on the master if you create a new staff user named with non latin characters like : كرم " my name in Arabic " when log in there is no problem the name is appears but when you change the page go to any other page " like Circulation" the name is gooing to be like this :( ÙرÙ, and if we go back to the main page the name still in these characters Ùر٠If we go the the Global system preferences page there is no problem there I will attach photos about this Thanks :) Created attachment 17327 [details]
NonLatin user name on Koha pages
This is how the name كرم appears
Created attachment 17328 [details]
Non latin user names on the Pereferences page (good)
On the System preferences page there is no problems
I can reproduce this problem using Karam's name. What I found so far is: 1) Fresh login, session cookie (in DB) records correctly userid id: كر 2) click on logo (or any link) and now session cookie has userid mangled id: "\xD9\x83\xD8\xB1\xD9\x85" 3) click any page, same result 4) Go to Administration > System Preferences, and now session cookie has correct userid again (also loggedinusername) So, could be something related with encoding at cookie writing time. I confirm this error for a userid with utf8 chars. Can reproduce it. But I prefer to open a new report for it. Bug 10019. Sending a patch under that report now. This patch has been pushed to master and 3.12.x. I have reverted this patch and the other related encoding patches from 3.12.x. These problems can be addressed for 3.14. Following Jared, I have also reverted the patches for this bug (and related bugs) from master. This should not be viewed as a permanent reversion, although I have reservations about turning on CGI.pm's -utf8 switch across the board -- but I have even stronger reservations about the ensuant game of whack-a-mole we've been playing with new bugs arising from this patch. By next week I will put together a separate integration branch to use as a basis for testing this more thoroughly. (In reply to comment #103) > Following Jared, I have also reverted the patches for this bug (and related > bugs) from master. This should not be viewed as a permanent reversion, > although I have reservations about turning on CGI.pm's -utf8 switch across > the board -- but I have even stronger reservations about the ensuant game of > whack-a-mole we've been playing with new bugs arising from this patch. > > By next week I will put together a separate integration branch to use as a > basis for testing this more thoroughly. Here is my reaction on this action from you as new RM: I have spent a lot of time on helping to resolve the side-effects of this patch and you revert this without any discussion! I triggered a discussion on the subject by mailing the list and you did not even respond. Jared planned to revert the patch for 3.12.x but leave it in master. And even more, this work is being compared with some game I do not know about. On the whole: This is very discouraging and highly unmotivating. Please communicate with other developers and team members instead of starting your RM term with one-sided actions like this one. Do you think that this makes me spend more time on the subject? Regarding whack a mole: http://www.youtube.com/watch?v=D0n8N98mpes This may be the only time in bugzilla history where a link to a video of a game is appropriate. Liz (In reply to comment #104) > Here is my reaction on this action from you as new RM: I am also an _old_ RM, and stand by my decision on the following basis: - This was reverted in 3.12.x. Jared would have been perfectly in his rights to revert in master as well. If the patch series was too problematic to include in the main release, it does not necessarily belong in master, either. - Side-effects of this patch series were *getting in the way* of applying other patches necessary for the 3.12.x release. - The basis of the patch series relies on use of a flag to CGI.pm that it's own documentation warns against. Further, please bear in mind what I said: "This should not be viewed as a permanent reversion" And I mean it. I do not disagree that the underlying bug needs to be dealt with, but the number of side-effects shows that the current patch series is not ready for prime time. The integration branch will provide an opportunity for more thorough testing. > On the whole: This is very discouraging and highly unmotivating. Please > communicate with other developers and team members instead of starting your > RM term with one-sided actions like this one. Do you think that this makes > me spend more time on the subject? Considering the discussion and advance warning over IRC, I reject your implication that this decision was not communicated. The reversion is not meant as a personal slight against anybody. I do understand that you have put a lot of effort into this, and I understand that having patches that one has worked on getting reverted stings. I hope to encourage a different approach to patches and reversions: that they be treated as experiments and learning opportunities, and that credit be given to those who make the experiment, even if their patch doesn't end up getting committed in its original form. However, I acknowledge that this is not an easy thing. But I will also be blunt: I would not be doing my job as RM well if, when necessary, I didn't exercise my own judgment. One of the things I've learned the hard way is that at times, it is necessary to force a pause to recheck assumptions. I believe this is one of the moments for this bug. I encourage you to work on the integration branch, but if you prefer to take a break from this bug, I have every confidence that we will not lack for hands and eyes to test. Galen, I don't know why, a remainder of the first patch still exists on master, which cause some trouble with encoding. The binmode routine is called in master: sub output_html_with_http_headers { binmode( STDOUT, ":encoding(utf8)" ); my ( $query, $cookie, $data, $status ) = @_; output_with_http_headers( $query, $cookie, $data, 'html', $status ); } This line was not deleted by the revert commit. commit d542740ab8877234d043c8a11cb19a6004e72656 Author: Dobrica Pavlinusic <dpavlin@rot13.org> Date: Sun Jun 10 15:18:42 2012 +0100 Bug 6554 - make Koha internally utf-8 clean git show d542740ab8877234d043c8a11cb19a6004e72656 C4/Output.pm +++ b/C4/Output.pm @@ -308,6 +308,7 @@ sub output_with_http_headers { } sub output_html_with_http_headers { + binmode( STDOUT, ":encoding(utf8)" ); commit a7eb34f2c8a281ef83f711e581079a2232296c88 Author: Galen Charlton <gmc@esilibrary.com> CommitDate: Mon Apr 29 15:12:32 2013 -0700 Revert "Bug 6554 - make Koha internally utf-8 clean" git show a7eb34f2c8a281ef83f711e581079a2232296c88 C4/Output.pm returns nothing (In reply to comment #103) > This should not be viewed as a permanent reversion, > although I have reservations about turning on CGI.pm's -utf8 switch across > the board -- but I have even stronger reservations about the ensuant game of > whack-a-mole we've been playing with new bugs arising from this patch. Would you mind to elaborate your objections to CGI.pm's -utf8 switch? I think that it's cleaner to take extra care about code which uses file upload than to hand-decode every single parameter we are passing into Koha code. This is only my personal preference, and I am fine with whatever solution gets eventually prefered, but I main question is if I should re-submit my patches without -utf8 in them. (In reply to comment #107) > Galen, > I don't know why, a remainder of the first patch still exists on master, > which cause some trouble with encoding. > > The binmode routine is called in master: Thanks for catching this. I've pushed a patch to master to complete the revert. (In reply to comment #105) > This may be the only time in bugzilla history where a link to a video of a game is appropriate. :-) (In reply to comment #106) Thanks for your reply, but I still disagree. Nevertheless I propose to end that discussion here.. (In reply to comment #107) > I don't know why, a remainder of the first patch still exists on master, which cause some trouble with encoding. Communicated this with Jared on bug 10070. (In reply to comment #108) > (In reply to comment #103) > > This should not be viewed as a permanent reversion, > > although I have reservations about turning on CGI.pm's -utf8 switch across > > the board -- but I have even stronger reservations about the ensuant game of > > whack-a-mole we've been playing with new bugs arising from this patch. > > Would you mind to elaborate your objections to CGI.pm's -utf8 switch? I > think that it's cleaner to take extra care about code which uses file upload > than to hand-decode every single parameter we are passing into Koha code. > > This is only my personal preference, and I am fine with whatever solution > gets eventually prefered, but I main question is if I should re-submit my > patches without -utf8 in them. From the man page -utf8 This makes CGI.pm treat all parameters as UTF-8 strings. Use this with care, as it will interfere with the processing of binary uploads. It is better to manually select which fields are expected to return utf-8 strings and convert them using code like this: use Encode; my $arg = decode utf8=>param('foo'); In Koha 3.12 some encoding problems was returned. Currently found in 3 locations: http://localhost:8888/cgi-bin/koha/members/members-home.pl (main screen) http://localhost:8888/cgi-bin/koha/members/member.pl (search) http://localhost:8888/cgi-bin/koha/circ/circulation.pl (search and menu) http://localhost:8888/cgi-bin/koha/admin/preferences.pl (search for non-latin) Screenshots attached. Search by members completely can not be used in 3.12.0. Created attachment 18746 [details]
Search for preferences in Cyrillic
Created attachment 18747 [details]
Search for circulation
Created attachment 18749 [details]
Members Home
Created attachment 18750 [details]
Search for members
(In reply to comment #112) > In Koha 3.12 some encoding problems was returned. Currently found in 3 > locations: > http://localhost:8888/cgi-bin/koha/members/members-home.pl (main screen) > http://localhost:8888/cgi-bin/koha/members/member.pl (search) > http://localhost:8888/cgi-bin/koha/circ/circulation.pl (search and menu) > http://localhost:8888/cgi-bin/koha/admin/preferences.pl (search for > non-latin) > Screenshots attached. > Search by members completely can not be used in 3.12.0. Horrible pictures, Serhij. But I still think that we were on the right track with Dobrica's patch to make Koha internally utf8 clean. Such a change can be compared with the template engine change we had some years ago. You cannot solve every little problem after that in one run. But the wind blew in another direction :) I found this problem (like in comment 112) on 3.12. All was fine with 3.10. Is there any work-around while waiting for the patch? I found same problem in 3.12.x in Czech translation. Problem on: /cgi-bin/koha/members/members-home.pl /cgi-bin/koha/admin/preferences.pl?tab=circulation /cgi-bin/koha/admin/preferences.pl?tab=opac /cgi-bin/koha/admin/preferences.pl?tab=patrons (In reply to Mike from comment #119) > I found same problem in 3.12.x in Czech translation. Problem on: > /cgi-bin/koha/members/members-home.pl > /cgi-bin/koha/admin/preferences.pl?tab=circulation > /cgi-bin/koha/admin/preferences.pl?tab=opac > /cgi-bin/koha/admin/preferences.pl?tab=patrons I solved the problem in /cgi-bin/koha/members/members-home.pl by removing from the Advanced Search patron Search by category: /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/ru-RU/includes/patron-search.inc Delete: [% IF ( categories ) %] <p><label for="categorycode">Категория: </label><select name="categorycode" id="categorycode"> <option value="">Произвольная</option>[% FOREACH categorie IN categories %] [% IF ( categorie.selected ) %] <option value="[% categorie.categorycode %]" selected="selected">[% categorie.description %]</option>[% ELSE %] <option value="[% categorie.categorycode %]">[% categorie.description %]</option>[% END %] [% END %]</select></p> [% END %] Yes, I see this problem in 3.12.4. (In reply to Serhij Dubyk from comment #112) > In Koha 3.12 some encoding problems was returned. Currently found in 3 > locations: > http://localhost:8888/cgi-bin/koha/members/members-home.pl (main screen) > http://localhost:8888/cgi-bin/koha/members/member.pl (search) > http://localhost:8888/cgi-bin/koha/circ/circulation.pl (search and menu) > http://localhost:8888/cgi-bin/koha/admin/preferences.pl (search for > non-latin) > Screenshots attached. > Search by members completely can not be used in 3.12.0. Created attachment 21318 [details] [review] An update to Templates.pm The patch is made against tag v3.12.04. Patch applied cleanly, go forth and signoff Hi Ketan I appreciate your enthusiasm, and we definitely want more people submitting patches. But unfortunately you can't mark your own patch signed off. Here's a quick write up on how it works http://wiki.koha-community.org/wiki/Bug-enhancement-patch_Workflow I would be happy to try signing off on your patch. Can you give me a quick test plan of how to recreate the problem so I can test the patch fixes it?. And of course we would love it if you tested and signed off other's patches. Thanks Chris Hello Chris, Hello Chris, I am sorry, I did not know the process. I did think it was a little odd to sign-off myself. However, the message said "Patch applied cleanly, go forth and signoff" so I did it. Anyway, thanks for the information. Here's the test plan. * Enter Patron names in non-latin characters like Russian. There is a screenshot of submitted by a user earlier. That is in Russian. In my case, the patron names are in Devnagari. * Then carry out a search on Patron * It should show you Patron names as they were entered. * At the same time, other parts of the site like - Catalog search should work with unicode. * A failure scenario is when the results show strange characters on the web page. I'll work on testing others' patches too. I am a little new to the whole process yet. Regards, Ketan (In reply to Chris Cormack from comment #124) > Hi Ketan > > I appreciate your enthusiasm, and we definitely want more people submitting > patches. But unfortunately you can't mark your own patch signed off. > > Here's a quick write up on how it works > http://wiki.koha-community.org/wiki/Bug-enhancement-patch_Workflow > > I would be happy to try signing off on your patch. Can you give me a quick > test plan of how to recreate the problem so I can test the patch fixes it?. > > And of course we would love it if you tested and signed off other's patches. > > Thanks > > Chris Hi I'm having trouble recreating the problem, I can enter cyrillic characters and have them display fine without the patch. So I can't really test that it fixes the issue, as I can't make the bad characters show. Could you paste a string that breaks it for you? Hello Chris, Here are a couple strings. Try them in Patron search of 3.12.4. कुलकर्णी पाटील The above are from devnagari script used for Marathi language. Regards, Ketan (In reply to Chris Cormack from comment #126) > Hi > > I'm having trouble recreating the problem, I can enter cyrillic characters > and have them display fine without the patch. > > So I can't really test that it fixes the issue, as I can't make the bad > characters show. > > Could you paste a string that breaks it for you? (In reply to Ketan Kulkarni from comment #127) > Hello Chris, > > Here are a couple strings. Try them in Patron search of 3.12.4. > > कुलकर्णी > पाटील > > The above are from devnagari script used for Marathi language. > > Regards, > Ketan Hmm those work fine on master, without the patch. Maybe it is fixed in master already, could you test there? However I note that when I apply your patch it breaks the dropdown on the new patron page. Before the patch <li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=B">Board</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=HB">Home Bound</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=IL">Inter-Library Loan</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=J">Juvenile</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=K">Kid</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=L">Library</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=PT">Patron</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=PERSON">person</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=SC">School</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=S">Staff</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=STAFF">staff</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=ST">Student</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=T">Teacher</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode=YA">Young Adult</a></li> After the patch <li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&categorycode="></a></li> Ah yes. It breaks the dropdown of New Patron. Quite a few things have changed in the master after v3.12.4. I'll check the drop down part. Created attachment 21399 [details]
members-home czech
Instead of adding patron with non-latin characters in name, try to define patron categories with non-latin characters. It definitely breaks the members-home page for me.
(In reply to Josef Moravec from comment #130) > Created attachment 21399 [details] > members-home czech > > Instead of adding patron with non-latin characters in name, try to define > patron categories with non-latin characters. It definitely breaks the > members-home page for me. Excellent, thanks for that. I'll wait for a new patch and test it that way. Ah, that sounds like bug 10864 - showing up on all our 3.12 installations for patron categories with umlauts. Created attachment 21478 [details] [review] Double encoding utf8 problem Use of utf8::upgrade() instead of utf8::encode() appears to be working. While taking inputs, they should be decoded first whereas output should always be encoded. I have another patch ready which is an enhancement over the earlier patch I submitted. How can I discard the older patch and add a new one? (In reply to Ketan Kulkarni from comment #134) > I have another patch ready which is an enhancement over the earlier patch I > submitted. How can I discard the older patch and add a new one? You can obsolete a patch manually by clicking Details. With git-bz you can do it more or less automatically.. Created attachment 21488 [details] [review] utf8 double encoding problem enhancement Search, patron pages, patron search, facets need to be tested with different languages. I have done testing on Devnagari input (Marathi). I've just tried the last patch in sandbox, but unfortunately, it doesn't work for me. In fact, there are another pages broken after applying patch. (e.g. edit patron categories page) Hello Joseph, Could you please attach a screenshot the broken "edit patron categories" page. I am unable to reproduce the problem after the latest update. i.e. the last patch. Screenshot of any other page showing the problem will also do. Regards, Ketan (In reply to Josef Moravec from comment #137) > I've just tried the last patch in sandbox, but unfortunately, it doesn't > work for me. In fact, there are another pages broken after applying patch. > (e.g. edit patron categories page) Created attachment 21506 [details]
Edit patron categories after patch 21488
I've just uploaded the screenshot. But there is a weird issue... After night, on the same sandbox setup, the problem with catron categories somehow disappeared, but after new setup, with the same parameters, the problem again appeared. Problem on the members home page is still the same. (In reply to Ketan Kulkarni from comment #138) > Hello Joseph, > > Could you please attach a screenshot the broken "edit patron categories" > page. I am unable to reproduce the problem after the latest update. i.e. the > last patch. > > Screenshot of any other page showing the problem will also do. > > Regards, > Ketan > > (In reply to Josef Moravec from comment #137) > > I've just tried the last patch in sandbox, but unfortunately, it doesn't > > work for me. In fact, there are another pages broken after applying patch. > > (e.g. edit patron categories page) Thank you Josef for uploading the screenshot. We are getting closer :-) The problem is thus. Now, the data coming from the db appears ok. e.g. patron names, category names etc. However, the templates are not rendered as they should be. Let me see how we can fix this. Ketan (In reply to Josef Moravec from comment #140) > I've just uploaded the screenshot. > > But there is a weird issue... After night, on the same sandbox setup, the > problem with catron categories somehow disappeared, but after new setup, > with the same parameters, the problem again appeared. > > Problem on the members home page is still the same. > > > (In reply to Ketan Kulkarni from comment #138) > > Hello Joseph, > > > > Could you please attach a screenshot the broken "edit patron categories" > > page. I am unable to reproduce the problem after the latest update. i.e. the > > last patch. > > > > Screenshot of any other page showing the problem will also do. > > > > Regards, > > Ketan > > > > (In reply to Josef Moravec from comment #137) > > > I've just tried the last patch in sandbox, but unfortunately, it doesn't > > > work for me. In fact, there are another pages broken after applying patch. > > > (e.g. edit patron categories page) Created attachment 21509 [details] [review] The template problem with utf8 For non-latin templates, almost all pages need to be seen along with the patron categories page. Created attachment 21581 [details] [review] one more file updated ... opac-search.pl Created attachment 21629 [details] [review] bug 6554: double utf8 encoding fix * added ENCODING=>utf8 to Template->new * added utf8::decode for ms_query I cannot sign off, it has tabs instead of spaces. I have replaced the multi-patch with this consolidated one. Please check if the commit message is ok. Also, I don't have time to look deeper, but utf8::upgrade($some_var) if utf8::is_utf8($some_var); looks wrong, that should not be needed. Or at least looks like the problem is being addressed in the wrong place. ENCODING => 'utf8' makes sense. Encode::decode() would also makes sense I presume. Thank you Srdjan for consolidating the patch and also for removing the if() from utf8::upgrade($some_var) if utf8::is_utf8($some_var); The condition is not needed. Regards, Ketan (In reply to Srdjan Jankovic from comment #145) > I cannot sign off, it has tabs instead of spaces. > > I have replaced the multi-patch with this consolidated one. Please check if > the commit message is ok. > > Also, I don't have time to look deeper, but > utf8::upgrade($some_var) if utf8::is_utf8($some_var); > looks wrong, that should not be needed. Or at least looks like the problem > is being addressed in the wrong place. > > ENCODING => 'utf8' makes sense. > > Encode::decode() would also makes sense I presume. Oops. Sorry .. That was an oversight. I think we can safely remove the if() from those lines. I'd like to hear from fellow developers. How do I remove tabs and put spaces? I use vi editor and I use tab for indentation. Regards, Ketan (In reply to Ketan Kulkarni from comment #146) > Thank you Srdjan for consolidating the patch and also for removing the if() > from > > utf8::upgrade($some_var) if utf8::is_utf8($some_var); > > The condition is not needed. > > Regards, > Ketan > > (In reply to Srdjan Jankovic from comment #145) > > I cannot sign off, it has tabs instead of spaces. > > > > I have replaced the multi-patch with this consolidated one. Please check if > > the commit message is ok. > > > > Also, I don't have time to look deeper, but > > utf8::upgrade($some_var) if utf8::is_utf8($some_var); > > looks wrong, that should not be needed. Or at least looks like the problem > > is being addressed in the wrong place. > > > > ENCODING => 'utf8' makes sense. > > > > Encode::decode() would also makes sense I presume. > > How do I remove tabs and put spaces? I use vi editor and I use tab for > indentation. > Don't use tabs if you expect to pass QA http://stackoverflow.com/questions/426963/replace-tab-with-spaces-in-vim Created attachment 21631 [details] [review] Replaced tabs with spaces Created attachment 21632 [details] [review] bug 6554: UTF8 double encoding fixes. Commits squashed into one. double utf8 encoding fix .. second try minor formatting updates enhancement to the previous patch related to utf8 double encoding added ENCODING=>utf8 to new-> added space which is there in master added utf8::decode for ms_query replaced tabs with spaces replaced tabs with spaces removed extra space Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Patch tested with a sandbox, by Josef Moravec <josef.moravec@gmail.com> Created attachment 21682 [details] [review] UTF8 double encoding fixes. Commits squashed into one. double utf8 encoding fix .. second try minor formatting updates enhancement to the previous patch related to utf8 double encoding added ENCODING=>utf8 to new-> added space which is there in master added utf8::decode for ms_query replaced tabs with spaces replaced tabs with spaces removed extra space Signed-off-by: Srdjan <srdjan@catalyst.net.nz> http://bugs.koha-community.org/show_bug.cgi?id=6554 Signed-off-by: Josef Moravec <josef.moravec@gmail.com> I marked 10068 as 'see also', not because of the patches, but to mention the first comment. This also fixes Bug 9897 - members/member.pl double encoded utf-8 strings so I added it to see also so we can close it once this gets into master. Hello Tomás, Yes, we should move from utf8::encode/decode to Encode:: This particular bug (6554) is about double encoding that happens in Templates.pm. I guess we should treat 10068 as a separate thing. It is linked for sure with the same issue. However, it's more about encoding. This one deals with decoding. Ketan (In reply to Tomás Cohen Arazi from comment #153) > I marked 10068 as 'see also', not because of the patches, but to mention the > first comment. I think that for this to pass QA and eventually been pushed we need a test plan 1) A way to check what error this patch is trying to fix, i.e. how and where to look encoding problems (add sample record/data) 2) How it's fixed In it's current incarnation this is a simple patch, but it's not clear how to test. Hello, Here's the scenario to reproduce the error condition. * Create patron categories with non-latin characters and then perform a search on patrons using non-latin characters. The patch is trying to fix two things. * Double encoding of utf8 strings. This happens in Templates.pm. * In some places query needs to be uft8::decode'ed. I have done that to fix problems that showed up when Templates.pm was updated. After applying the patch, following pages need to be tested. * Patron search - Need to see all places on the results page that may contain non-latin characters. You need to have patron categories with non-latin characters. * Catalog search - Need to check places where non-latin characters appear * Non-latin templates e.g. koha in Russian language. I think this task will take a lot of time. Regards, Ketan (In reply to Bernardo Gonzalez Kriegel from comment #156) > I think that for this to pass QA and eventually been pushed we need a test > plan > > 1) A way to check what error this patch is trying to fix, i.e. how and where > to look encoding problems (add sample record/data) > 2) How it's fixed > > In it's current incarnation this is a simple patch, but it's not clear how > to test. Ketan, (In reply to Ketan Kulkarni from comment #155) > Hello Tomás, > > Yes, we should move from utf8::encode/decode to Encode:: > This particular bug (6554) is about double encoding that happens in > Templates.pm. Take a look at this work from Marcel: http://wiki.koha-community.org/wiki/Handling_UTF8_in_development If we (currently) do things wrong inside Koha regarding encoding/decoding of UTF-8 data (double encoding means we are encoding wrong somewhere), then fixing those symptms will just hide the problem (using utf8::upgrade will avoid the consequences but not fix the root cause, it might even hide new bugs introduced by future patches). It is up to the QA team and the RM to think of the best approach for tackling this so I'm just leaving this comment, also I appreciate you efforts and hope this gets fixed soon. Hello Tomás, I appreciate your concerns. I have read the web page. It's up to the QA and RM to decide. Nevertheless, the updates in the patch have been done in accordance with the principle - decode the input and encode the output. Although utf8::upgrade() is a temporary fix to "hide" the problems if any, does fix things to a great extent. Besides, upgrade() is an inbuilt perl function. And on production systems it's better we use upgrade() than just encode() if it is going to break things. Once we have a plan in place, this will be replaced by Encode::.... You see, there's talk about using Encode::... instead of utf8::.... In the current scenario, utf8::upgrade() appears to be a decent (and forgiving) way of handling errors in coding. There's another bug related to writing a generic subroutine for utf-decoding for input. The discussion is on. Considering the size of the code-base, UTF handling related updates are going to take time. The present patch tries to fix the problem that is there in the stable release temporarily. Regards, Ketan (In reply to Tomás Cohen Arazi from comment #158) > Ketan, > > (In reply to Ketan Kulkarni from comment #155) > > Hello Tomás, > > > > Yes, we should move from utf8::encode/decode to Encode:: > > This particular bug (6554) is about double encoding that happens in > > Templates.pm. > > Take a look at this work from Marcel: > http://wiki.koha-community.org/wiki/Handling_UTF8_in_development > > If we (currently) do things wrong inside Koha regarding encoding/decoding of > UTF-8 data (double encoding means we are encoding wrong somewhere), then > fixing those symptms will just hide the problem (using utf8::upgrade will > avoid the consequences but not fix the root cause, it might even hide new > bugs introduced by future patches). > > It is up to the QA team and the RM to think of the best approach for > tackling this so I'm just leaving this comment, also I appreciate you > efforts and hope this gets fixed soon. (In reply to Ketan Kulkarni from comment #159) > Hello Tomás, > > I appreciate your concerns. I have read the web page. It's up to the QA and > RM to decide. > Just one note here, Tomás is the release maintainer for the 3.12.x branch, so it is up to him to decide if this will go into that stable release. Just to add my $0.02 since I was original author of encoding fixes: this patch indeed goes against idea of correctly encoding everything in Koha, but does fix some really broken places in current code. I would appreciate if this patch was accepted upstream (encoding of borrower characters is my most pressing concern) and if nothing else it does mark few more places in which we will have to take a look when making koha utf-8 clean. Sorry, but applying this patch breaks a lot of encodings on several places in opac and staff. Just look at opac-search and catalogue search. I have lots of broken characters (that should have diacritics) in the results (based on XSLT) and in the facets. Failed QA Created attachment 21937 [details]
My opac after search
My opac search does not break after applying the patch. Could you please upload a screenshot? This is from the latest master.
(In reply to Ketan Kulkarni from comment #163) > Created attachment 21937 [details] > My opac after search > > My opac search does not break after applying the patch. Could you please > upload a screenshot? This is from the latest master. You are not using XSLT on OPAC? Please change these preferences and test. Created attachment 21955 [details]
My opac xslt preferences
Please see my OPAC preferences.
(In reply to Ketan Kulkarni from comment #165) > Created attachment 21955 [details] > My opac xslt preferences > > Please see my OPAC preferences. OK That looks good. Will post some more details today about what I came across. Created attachment 21978 [details]
opac-search
Please look at mangled characters with diacritics in the XSLT blocks and facets.
I attached an image for opac-search but opac-detail and opac-MARCdetail contain similar mangled chars. Also opac-shelves.pl, opac-readingrecord.pl, and maybe more.. At staff side: similar problems in catalogue/search, catalogue/detail.pl, virtualshelves/shelves.pl, circ/circulation.pl (confirm checkout had a mangled char), cataloguing/addbiblio.pl (oops: dangerous! saving mangled data), acqui/booksellers.pl (look for é), and maybe more.. Created attachment 21980 [details]
OPAC search
Marcel,
I take a screenshot of your devel site.
For me it looks right, without mangled chars.
That's one the problems with this bug,
different people see different things.
(In reply to Bernardo Gonzalez Kriegel from comment #169) > For me it looks right, without mangled chars. I got the impression that the mangled chars are from the Latin subset (diacritics on vowels like a e etc.) Special chars from Chinese or Japanese seem to be correct.. Can you confirm that? Or are Latin chars also correct in your case? Created attachment 21981 [details] OPAC search 3 (In reply to M. de Rooy from comment #170) > I got the impression that the mangled chars are from the Latin subset > (diacritics on vowels like a e etc.) Special chars from Chinese or Japanese > seem to be correct.. Can you confirm that? Or are Latin chars also correct > in your case? Problem is they look correct to me (latin diacritics). Attached same screen but with Firefox. When tested latest patchset I can't reproduce errors, I create patron categories in Chinese, Armenian, Indi, etc. All looked right (on master) (In reply to Bernardo Gonzalez Kriegel from comment #171) > Created attachment 21981 [details] > OPAC search 3 Hi Bernardo This is another image from my server but the 6554 patch is no longer active. That makes a difference :) > When tested latest patchset I can't reproduce errors, I create patron > categories in Chinese, Armenian, Indi, etc. All looked right (on master) OK. My question actually was: If you have only a few latin chars on your screen (so not combined with *real* eh-eh utf8 chars, are they correct? Do not know if you have such records in YOUR database? (In reply to M. de Rooy from comment #172) > OK. > My question actually was: If you have only a few latin chars on your screen > (so not combined with *real* eh-eh utf8 chars, are they correct? Do not know > if you have such records in YOUR database? Yes I have :) And they look correct Created attachment 23107 [details]
Screenshot for Patrons in Koha 3.14.0
Bug still exists. Any patrons category name in not ASCII will damage page. On screenshot one category presented on Ukrainian, if it is removed (or transliterated), the encoding problem disappears. But it is not a good solution.
Can we please close this bug and maybe open specific ones? Most of the screenshots aren't even reproducible. I'm not talking about the last one. Also, please always enumerate the steps to reproduce the problem. (In reply to Serhij Dubyk from comment #174) > Created attachment 23107 [details] > Screenshot for Patrons in Koha 3.14.0 > > Bug still exists. Any patrons category name in not ASCII will damage page. > On screenshot one category presented on Ukrainian, if it is removed (or > transliterated), the encoding problem disappears. But it is not a good > solution. What are the steps to reproduce? Correct me if I'm wrong: 1) In a fresh Koha, generate translation into XX 2) Select XX for the staff interface (i.e. enable it, and choose it) 3) Go to the Patrons page 4) Everything looks fine 5) In the admin page, create a patron category with (say) the non-ASCII character 'ñ'. 6) Go back to the Patrons page, everything is broken Is that the way to reproduce it? (In reply to Tomás Cohen Arazi from comment #176) > (In reply to Serhij Dubyk from comment #174) > > Created attachment 23107 [details] > > Screenshot for Patrons in Koha 3.14.0 > > > > Bug still exists. Any patrons category name in not ASCII will damage page. > > On screenshot one category presented on Ukrainian, if it is removed (or > > transliterated), the encoding problem disappears. But it is not a good > > solution. > > What are the steps to reproduce? Correct me if I'm wrong: > > 1) In a fresh Koha, generate translation into XX > 2) Select XX for the staff interface (i.e. enable it, and choose it) > 3) Go to the Patrons page > 4) Everything looks fine > 5) In the admin page, create a patron category with (say) the non-ASCII > character 'ñ'. > 6) Go back to the Patrons page, everything is broken > > Is that the way to reproduce it? It is caused by includes/patron-search.inc .. if you comment out the include call to it out, the page renders just fine. Will make a new bug and attach a patch when I figure out the fix Patch on bug 11290 Created attachment 23140 [details] [review] [ALT] Bug 6554: Template::output should deal with object if a ref is not HASH or ARRAY, C4::Template::output assumes it is a scalar. Which is wrong, it could be an object. Comment on attachment 23140 [details] [review] [ALT] Bug 6554: Template::output should deal with object Here is my try on utf8 issues. It is not perfect because we will have to maintain a blacklist. Please test. *** This bug has been marked as a duplicate of bug 11302 *** (In reply to Jonathan Druart from comment #181) > > *** This bug has been marked as a duplicate of bug 11302 *** Are you sure that you could close this report? The new report does not seem to be so generic as this one was. But your patch will certainly have my attention. Thanks. This one became too long. It could be reopened if 11302 is wrong. Looks like there is (somehow) similar bug also in circ/view_holdsqueue.tt . It only manifests itself if there are non-latin characters in collection code (authorised value CCODE) descriptions. Possible fixes: 1) Like in bug 11290 - inserting ' |html_entity' in line 80, after "KohaAuthorisedValues.GetByCode( 'CCODE', itemsloo.ccode )" 2) Replacing KohaAuthorisedValues template plugin with AuthorisedValues plugin. But why are there two different TT plugin versions in the first place; shouldn't AuthorisedValues be used (everywhere) instead of KohaAuthorisedValues ? Please can we open new bugs for each instance, with new valid patches, this bug is impossible to maintain any more. (In reply to Jacek Ablewicz from comment #184) > 2) Replacing KohaAuthorisedValues template plugin with AuthorisedValues > plugin. But why are there two different TT plugin versions in the first > place; shouldn't AuthorisedValues be used (everywhere) instead of > KohaAuthorisedValues ? Will be fixed by bug 10626. For more fun, please see bug 11944. |
Created attachment 4558 [details] members-home.pl (Ukrainian) Two pages /cgi-bin/koha/members/members-home.pl /cgi-bin/koha/members/member.pl displayed incorrectly for languages with multibytes encodings. Screenshot attached for Ukrainian, Russian, Greek French, Spanish, Arabic ... Template member.tt look good - all translated parts is readable.