Bug 6554 - Resolve encoding problems with corrected UTF8 handling in templates
Summary: Resolve encoding problems with corrected UTF8 handling in templates
Status: RESOLVED MOVED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P3 critical
Assignee: Ketan Kulkarni
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 9951 10019 10070 10074
  Show dependency tree / graph
 
Reported: 2011-07-05 15:33 UTC by Serhij Dubyk
Modified: 2014-03-14 15:33 UTC (History)
24 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:

Attachments
members-home.pl (Ukrainian) (42.78 KB, image/png)
2011-07-05 15:33 UTC, Serhij Dubyk 		Details
member.pl (Ukrainian) (45.40 KB, image/png)
2011-07-05 15:34 UTC, Serhij Dubyk 		Details
members-home.pl (Russian) (35.98 KB, image/png)
2011-07-05 15:36 UTC, Serhij Dubyk 		Details
members-home.pl (Greek) (35.81 KB, image/png)
2011-07-05 15:36 UTC, Serhij Dubyk 		Details
members-home.pl (French) (33.40 KB, image/png)
2011-07-05 15:37 UTC, Serhij Dubyk 		Details
members-home.pl (Arabic) (34.91 KB, image/png)
2011-07-05 15:38 UTC, Serhij Dubyk 		Details
-- /cgi-bin/koha/members/members-home.pl (Russian) (100.38 KB, image/png)
2011-10-18 19:06 UTC, Serhij Dubyk 		Details
Clean up Koha utf-8 string handling (3.25 KB, patch)
2011-10-24 18:11 UTC, Dobrica Pavlinusic 		Details | Diff | Splinter Review
encoding problem in member-home.pl in 3.6.0 (another one) (117.34 KB, image/png)
2011-10-26 16:55 UTC, Paul Poulain 		Details
member.pl (Ukrainian) after patch1 (222.46 KB, image/png)
2011-10-27 08:06 UTC, Serhij Dubyk 		Details
OPAC search (Ukrainian, after patch1) (230.37 KB, image/png)
2011-10-27 08:08 UTC, Serhij Dubyk 		Details
Staff home page (Ukrainian) after patch1 (188.48 KB, image/png)
2011-10-27 08:09 UTC, Serhij Dubyk 		Details
remove unused code, cleanup ident and correctly mark templates as utf-8 (4.28 KB, patch)
2011-10-27 11:03 UTC, Dobrica Pavlinusic 		Details | Diff | Splinter Review
OPAC search (Ukrainian, after patch2) (228.94 KB, image/png)
2011-10-27 14:42 UTC, Serhij Dubyk 		Details
STAFF search (Ukrainian, after patch 2) (218.94 KB, image/png)
2011-10-27 14:43 UTC, Serhij Dubyk 		Details
member search (ukrainian, example of good page after patch 2) (237.57 KB, image/png)
2011-10-27 14:44 UTC, Serhij Dubyk 		Details
z39-50 edit page (ukrainian, after patch 2) (220.13 KB, image/png)
2011-10-27 14:45 UTC, Serhij Dubyk 		Details
preferences page (ukrainian, after patch 2) (216.99 KB, image/png)
2011-10-27 14:47 UTC, Serhij Dubyk 		Details
added session, CGI queries and all Zebra/PAR utf8 decode (7.15 KB, patch)
2011-10-27 19:29 UTC, Dobrica Pavlinusic 		Details | Diff | Splinter Review
opac_search.pl (ukrainian, after patch 3) (221.85 KB, image/png)
2011-10-27 22:07 UTC, Serhij Dubyk 		Details
staff search.pl (ukrainian, after patch 3) (221.19 KB, image/png)
2011-10-27 22:08 UTC, Serhij Dubyk 		Details
addbiblio.pl dublicate warning (ukrainian, after patch 3) (191.21 KB, image/png)
2011-10-28 23:50 UTC, Serhij Dubyk 		Details
Bug 6554 - added session CGI queries and all ZebraPAR utf8_decode (8.42 KB, patch)
2012-06-10 14:20 UTC, MJ Ray (software.coop) 		Details | Diff | Splinter Review
Bug 6554 - make Koha internally utf-8 clean (16.88 KB, patch)
2013-03-22 13:04 UTC, Dobrica Pavlinusic 		Details | Diff | Splinter Review
Bug 6554 - make Koha internally utf-8 clean (16.51 KB, patch)
2013-03-22 15:07 UTC, Dobrica Pavlinusic 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554 - make Koha internally utf-8 clean (17.04 KB, patch)
2013-03-25 15:22 UTC, Owen Leonard 		Details | Diff | Splinter Review
Bug 6554 - followup - encode output, XSLT encoding from templates (1.57 KB, patch)
2013-03-26 19:51 UTC, Dobrica Pavlinusic 		Details | Diff | Splinter Review
Bug 6554 QA Followup (1.86 KB, patch)
2013-03-27 12:44 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Merge branch 'bug_9919' into 3.12-master (1.52 KB, patch)
2013-03-27 12:48 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554 - make Koha internally utf-8 clean (17.09 KB, patch)
2013-03-27 12:55 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554 QA Followup (1.86 KB, patch)
2013-03-27 12:57 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
6554 Followup for preferences.pl (922 bytes, patch)
2013-03-28 11:55 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554 Followup for circulation patron search (1.20 KB, patch)
2013-03-28 12:09 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
6554 Followup for preferences.pl (1.08 KB, patch)
2013-03-28 12:11 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554: Followup for serial search (1.35 KB, patch)
2013-03-28 12:26 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554: Followup for acquisitions (2.00 KB, patch)
2013-03-28 12:41 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554: Followup for preferences.pl (1.01 KB, patch)
2013-03-28 13:24 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554: Followup for preferences.pl (1003 bytes, patch)
2013-03-28 13:46 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 6554 Followup for circulation patron search (1.06 KB, patch)
2013-03-28 13:50 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554 - make Koha internally utf-8 clean (17.15 KB, patch)
2013-03-28 16:06 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554 QA Followup (1.90 KB, patch)
2013-03-28 16:07 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554: Followup for serial search (1.39 KB, patch)
2013-03-28 16:07 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554: Followup for acquisitions (2.02 KB, patch)
2013-03-28 16:07 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554: Followup for preferences.pl (1.03 KB, patch)
2013-03-28 16:07 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6554 Followup for circulation patron search (1.07 KB, patch)
2013-03-28 16:07 UTC, Bernardo Gonzalez Kriegel 		Details | Diff | Splinter Review
Bug 6554: Followup for decoding in userenv (1.17 KB, patch)
2013-04-04 06:47 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Proposed Patch (1.55 KB, patch)
2013-04-04 10:25 UTC, Colin Campbell 		Details | Diff | Splinter Review
Bug 6554 Fix error caused by modifying $_ in a map (1.63 KB, patch)
2013-04-04 10:42 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
NonLatin user name on Koha pages (41.79 KB, image/png)
2013-04-09 21:56 UTC, Karam Qubsi 		Details
Non latin user names on the Pereferences page (good) (56.42 KB, image/png)
2013-04-09 21:59 UTC, Karam Qubsi 		Details
Search for preferences in Cyrillic (114.12 KB, image/png)
2013-06-07 13:23 UTC, Serhij Dubyk 		Details
Search for circulation (45.20 KB, image/png)
2013-06-07 13:24 UTC, Serhij Dubyk 		Details
Members Home (42.11 KB, image/png)
2013-06-07 13:25 UTC, Serhij Dubyk 		Details
Search for members (63.81 KB, image/png)
2013-06-07 13:27 UTC, Serhij Dubyk 		Details
An update to Templates.pm (1.30 KB, patch)
2013-09-22 18:27 UTC, Ketan Kulkarni 		Details | Diff | Splinter Review
members-home czech (35.15 KB, image/png)
2013-09-24 08:42 UTC, Josef Moravec 		Details
Double encoding utf8 problem (4.72 KB, patch)
2013-09-26 09:34 UTC, Ketan Kulkarni 		Details | Diff | Splinter Review
utf8 double encoding problem enhancement (7.46 KB, patch)
2013-09-26 14:50 UTC, Ketan Kulkarni 		Details | Diff | Splinter Review
Edit patron categories after patch 21488 (125.88 KB, image/png)
2013-09-27 05:26 UTC, Josef Moravec 		Details
The template problem with utf8 (8.89 KB, patch)
2013-09-27 11:48 UTC, Ketan Kulkarni 		Details | Diff | Splinter Review
one more file updated ... opac-search.pl (9.84 KB, patch)
2013-09-29 06:05 UTC, Ketan Kulkarni 		Details | Diff | Splinter Review
bug 6554: double utf8 encoding fix (4.92 KB, patch)
2013-09-30 03:19 UTC, Srdjan Jankovic 		Details | Diff | Splinter Review
Replaced tabs with spaces (4.34 KB, patch)
2013-09-30 04:46 UTC, Ketan Kulkarni 		Details | Diff | Splinter Review
bug 6554: UTF8 double encoding fixes. Commits squashed into one. (4.43 KB, patch)
2013-09-30 05:23 UTC, Srdjan Jankovic 		Details | Diff | Splinter Review
UTF8 double encoding fixes. Commits squashed into one. (4.50 KB, patch)
2013-09-30 20:49 UTC, Biblibre Sandboxes 		Details | Diff | Splinter Review
My opac after search (120.90 KB, image/png)
2013-10-10 08:49 UTC, Ketan Kulkarni 		Details
My opac xslt preferences (133.94 KB, image/png)
2013-10-11 04:12 UTC, Ketan Kulkarni 		Details
opac-search (161.75 KB, image/png)
2013-10-14 07:19 UTC, Marcel de Rooy 		Details
OPAC search (178.17 KB, image/png)
2013-10-14 10:52 UTC, Bernardo Gonzalez Kriegel 		Details
OPAC search 3 (175.33 KB, image/png)
2013-10-14 11:24 UTC, Bernardo Gonzalez Kriegel 		Details
Screenshot for Patrons in Koha 3.14.0 (53.78 KB, image/png)
2013-11-22 19:37 UTC, Serhij Dubyk 		Details
[ALT] Bug 6554: Template::output should deal with object (4.75 KB, patch)
2013-11-25 15:46 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (41) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Serhij Dubyk 2011-07-05 15:33:19 UTC Comment hidden (obsolete)
Comment 1 Serhij Dubyk 2011-07-05 15:34:58 UTC 
Created attachment 4560 [details]
member.pl (Ukrainian)
Comment 2 Serhij Dubyk 2011-07-05 15:36:04 UTC 
Created attachment 4561 [details]
members-home.pl (Russian)
Comment 3 Serhij Dubyk 2011-07-05 15:36:42 UTC 
Created attachment 4562 [details]
members-home.pl (Greek)
Comment 4 Serhij Dubyk 2011-07-05 15:37:26 UTC 
Created attachment 4563 [details]
members-home.pl (French)
Comment 5 Serhij Dubyk 2011-07-05 15:38:57 UTC 
Created attachment 4564 [details]
members-home.pl (Arabic)
Comment 6 Serhij Dubyk 2011-10-18 19:06:54 UTC 
Created attachment 5962 [details]
-- /cgi-bin/koha/members/members-home.pl (Russian)

Tested in Koha 3.05.00.011.
Comment 7 Serhij Dubyk 2011-10-18 19:12:44 UTC 
(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>Коха &rsaquo; Посетители [% IF ( searching ) %] &rsaquo; Результаты поиска [% 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>&rsaquo; [% IF ( searching ) %]<a href="/cgi-bin/koha/members/members-home.pl">Посетители</a>&rsaquo; Результаты поиска[% 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&amp;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>&nbsp;</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&amp;destination=circ&amp;borrowernumber=[% resultsloo.borrowernumber %]&amp;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&amp;destination=circ&amp;borrowernumber=[% resultsloo.borrowernumber %]&amp;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&amp;destination=circ&amp;borrowernumber=[% resultsloo.borrowernumber %]&amp;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>
Comment 8 Dobrica Pavlinusic 2011-10-24 18:11:09 UTC Comment hidden (obsolete)
Comment 9 Paul Poulain 2011-10-26 16:54:30 UTC 
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)
Comment 10 Paul Poulain 2011-10-26 16:55:48 UTC 
Created attachment 6065 [details]
encoding problem in member-home.pl in 3.6.0 (another one)
Comment 11 Serhij Dubyk 2011-10-27 08:03:13 UTC 
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.
Comment 12 Serhij Dubyk 2011-10-27 08:06:10 UTC 
Created attachment 6070 [details]
member.pl (Ukrainian) after patch1
Comment 13 Serhij Dubyk 2011-10-27 08:08:16 UTC 
Created attachment 6071 [details]
OPAC search (Ukrainian, after patch1)
Comment 14 Serhij Dubyk 2011-10-27 08:09:05 UTC 
Created attachment 6072 [details]
Staff home page (Ukrainian) after patch1
Comment 15 Dobrica Pavlinusic 2011-10-27 11:03:04 UTC Comment hidden (obsolete)
Comment 16 Serhij Dubyk 2011-10-27 14:41:05 UTC 
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)
Comment 17 Serhij Dubyk 2011-10-27 14:42:10 UTC 
Created attachment 6076 [details]
OPAC search (Ukrainian, after patch2)
Comment 18 Serhij Dubyk 2011-10-27 14:43:21 UTC 
Created attachment 6077 [details]
STAFF search (Ukrainian, after patch 2)
Comment 19 Serhij Dubyk 2011-10-27 14:44:26 UTC 
Created attachment 6078 [details]
member search (ukrainian, example of good page after patch 2)
Comment 20 Serhij Dubyk 2011-10-27 14:45:30 UTC 
Created attachment 6079 [details]
z39-50 edit page (ukrainian, after patch 2)
Comment 21 Serhij Dubyk 2011-10-27 14:47:00 UTC 
Created attachment 6080 [details]
preferences page (ukrainian, after patch 2)
Comment 22 Dobrica Pavlinusic 2011-10-27 19:29:24 UTC Comment hidden (obsolete)
Comment 23 Serhij Dubyk 2011-10-27 22:05:39 UTC 
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).
Comment 24 Serhij Dubyk 2011-10-27 22:07:34 UTC 
Created attachment 6084 [details]
opac_search.pl (ukrainian, after  patch 3)
Comment 25 Serhij Dubyk 2011-10-27 22:08:42 UTC 
Created attachment 6085 [details]
staff search.pl (ukrainian, after patch 3)
Comment 26 Dobrica Pavlinusic 2011-10-28 09:50:53 UTC 
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.
Comment 27 Serhij Dubyk 2011-10-28 23:50:58 UTC 
Created attachment 6108 [details]
addbiblio.pl dublicate warning (ukrainian, after patch 3)

yet another place
Comment 28 Marcel de Rooy 2011-11-02 13:49:23 UTC 
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?
Comment 29 Marcel de Rooy 2011-11-02 13:56:00 UTC 
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.
Comment 30 Katrin Fischer 2011-12-21 06:35:36 UTC 
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
Comment 31 Jared Camins-Esakov 2012-06-09 19:18:31 UTC 
Dobrica,

Could you please resubmit the patch for this bug in git format?
Comment 32 MJ Ray (software.coop) 2012-06-10 14:20:53 UTC Comment hidden (obsolete)
Comment 33 Katrin Fischer 2012-06-17 05:25:22 UTC 
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?
Comment 34 Mirko Tietgen 2012-07-31 21:23:32 UTC 
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.
Comment 35 Dobrica Pavlinusic 2013-03-22 13:04:12 UTC Comment hidden (obsolete)
Comment 36 Dobrica Pavlinusic 2013-03-22 15:07:13 UTC Comment hidden (obsolete)
Comment 37 Owen Leonard 2013-03-25 15:22:46 UTC Comment hidden (obsolete)
Comment 38 Marcel de Rooy 2013-03-26 13:06:24 UTC 
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?
Comment 39 Marcel de Rooy 2013-03-26 13:36:27 UTC 
This would work for me:

Output.pm (binmode removed), output_with_http_headers: 
    $data =~ s/\&amp\;amp\; /\&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..
Comment 40 Marcel de Rooy 2013-03-26 13:43:32 UTC 
(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..
Comment 41 Dobrica Pavlinusic 2013-03-26 19:51:42 UTC Comment hidden (obsolete)
Comment 42 Dobrica Pavlinusic 2013-03-26 19:53:40 UTC 
(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 &amp; regexp needed? I added warn, and don't see any output from it.
Comment 43 Marcel de Rooy 2013-03-27 12:44:50 UTC Comment hidden (obsolete)
Comment 44 Marcel de Rooy 2013-03-27 12:47:33 UTC 
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.
Comment 45 Marcel de Rooy 2013-03-27 12:48:13 UTC Comment hidden (obsolete)
Comment 46 Marcel de Rooy 2013-03-27 12:48:46 UTC 
Comment on attachment 16930 [details] [review]
Merge branch 'bug_9919' into 3.12-master

Oops
Comment 47 Marcel de Rooy 2013-03-27 12:55:54 UTC Comment hidden (obsolete)
Comment 48 Marcel de Rooy 2013-03-27 12:57:58 UTC Comment hidden (obsolete)
Comment 49 Marcel de Rooy 2013-03-27 13:06:56 UTC 
Still another one: look for a diacritic in the title within Serials module.
Another encoding error visible.
Comment 50 Marcel de Rooy 2013-03-27 13:07:42 UTC 
Acquisitions: look for é in the vendor field.
Comment 51 Marcel de Rooy 2013-03-27 13:12:35 UTC 
(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 :)
Comment 52 Jonathan Druart 2013-03-28 11:08:25 UTC 
(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.
Comment 53 Marcel de Rooy 2013-03-28 11:24:18 UTC 
(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.
Comment 54 Marcel de Rooy 2013-03-28 11:33:06 UTC 
(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.
Comment 55 Marcel de Rooy 2013-03-28 11:55:04 UTC Comment hidden (obsolete)
Comment 56 Marcel de Rooy 2013-03-28 12:09:57 UTC Comment hidden (obsolete)
Comment 57 Marcel de Rooy 2013-03-28 12:11:24 UTC Comment hidden (obsolete)
Comment 58 Marcel de Rooy 2013-03-28 12:26:25 UTC Comment hidden (obsolete)
Comment 59 Marcel de Rooy 2013-03-28 12:41:21 UTC Comment hidden (obsolete)
Comment 60 Marcel de Rooy 2013-03-28 12:52:43 UTC 
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..
Comment 61 Marcel de Rooy 2013-03-28 13:22:24 UTC 
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.
Comment 62 Marcel de Rooy 2013-03-28 13:24:57 UTC Comment hidden (obsolete)
Comment 63 Marcel de Rooy 2013-03-28 13:46:54 UTC Comment hidden (obsolete)
Comment 64 Marcel de Rooy 2013-03-28 13:50:34 UTC Comment hidden (obsolete)
Comment 65 Marcel de Rooy 2013-03-28 13:52:09 UTC 
Resubmitted the followups for acq and prefs. They did not need URI::Escape after all. CGI handles that correctly. Simplification :)
Comment 66 Dobrica Pavlinusic 2013-03-28 15:00:20 UTC 
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.
Comment 67 Bernardo Gonzalez Kriegel 2013-03-28 16:06:55 UTC Comment hidden (obsolete)
Comment 68 Bernardo Gonzalez Kriegel 2013-03-28 16:07:05 UTC Comment hidden (obsolete)
Comment 69 Bernardo Gonzalez Kriegel 2013-03-28 16:07:13 UTC Comment hidden (obsolete)
Comment 70 Bernardo Gonzalez Kriegel 2013-03-28 16:07:29 UTC Comment hidden (obsolete)
Comment 71 Bernardo Gonzalez Kriegel 2013-03-28 16:07:40 UTC Comment hidden (obsolete)
Comment 72 Bernardo Gonzalez Kriegel 2013-03-28 16:07:50 UTC Comment hidden (obsolete)
Comment 73 Bernardo Gonzalez Kriegel 2013-03-28 16:11:24 UTC 
I found that this solves the problem reported in Bug 9579.
Comment 74 Marcel de Rooy 2013-03-28 16:37:27 UTC 
(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.
Comment 75 Bernardo Gonzalez Kriegel 2013-03-28 17:24:16 UTC 
(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.
Comment 76 Mathieu Saby 2013-03-28 17:41:57 UTC 
Thank you Bernardo! From the title of this bug, I could never imagine it solved the problem of arabic facets...

Mathieu Saby
Comment 77 Marcel de Rooy 2013-03-29 07:41:40 UTC 
(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..
Comment 78 Marcel de Rooy 2013-03-29 08:19:43 UTC 
(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.
Comment 79 Marcel de Rooy 2013-03-29 08:42:48 UTC 
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 :)
Comment 80 Marcel de Rooy 2013-03-29 10:32:47 UTC 
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
Comment 81 Jared Camins-Esakov 2013-04-01 22:52:54 UTC 
This patch has been pushed to master.
Comment 82 Colin Campbell 2013-04-03 15:37:52 UTC 
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
Comment 83 Katrin Fischer 2013-04-03 15:52:56 UTC 
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?
Comment 84 Marcel de Rooy 2013-04-04 06:47:32 UTC Comment hidden (obsolete)
Comment 85 Marcel de Rooy 2013-04-04 06:48:35 UTC 
Colin:
I cannot reproduce either. But could you test the latest patch attached?
Thanks.
Comment 86 Colin Campbell 2013-04-04 08:03:42 UTC 
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
Comment 87 Jonathan Druart 2013-04-04 08:25:57 UTC 
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.
Comment 88 Paul Poulain 2013-04-04 09:01:14 UTC 
(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.
Comment 89 Marcel de Rooy 2013-04-04 10:19:14 UTC 
(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.
Comment 90 Marcel de Rooy 2013-04-04 10:20:30 UTC 
(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..
Comment 91 Marcel de Rooy 2013-04-04 10:20:59 UTC 
Last patch needs signoff.
Comment 92 Colin Campbell 2013-04-04 10:25:28 UTC Comment hidden (obsolete)
Comment 93 Marcel de Rooy 2013-04-04 10:26:37 UTC 
(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.
Comment 94 Marcel de Rooy 2013-04-04 10:42:56 UTC Comment hidden (obsolete)
Comment 95 Marcel de Rooy 2013-04-04 10:47:21 UTC 
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
Comment 96 Karam Qubsi 2013-04-09 21:51:25 UTC 
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 :)
Comment 97 Karam Qubsi 2013-04-09 21:56:53 UTC Comment hidden (obsolete)
Comment 98 Karam Qubsi 2013-04-09 21:59:24 UTC Comment hidden (obsolete)
Comment 99 Bernardo Gonzalez Kriegel 2013-04-10 00:39:01 UTC 
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.
Comment 100 Marcel de Rooy 2013-04-10 07:47:54 UTC 
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.
Comment 101 Jared Camins-Esakov 2013-04-18 13:29:54 UTC 
This patch has been pushed to master and 3.12.x.
Comment 102 Jared Camins-Esakov 2013-04-26 02:40:21 UTC 
I have reverted this patch and the other related encoding patches from 3.12.x. These problems can be addressed for 3.14.
Comment 103 Galen Charlton 2013-05-01 18:51:14 UTC 
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.
Comment 104 Marcel de Rooy 2013-05-02 06:43:41 UTC 
(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?
Comment 105 Liz Rea 2013-05-02 21:30:15 UTC 
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
Comment 106 Galen Charlton 2013-05-02 21:47:12 UTC 
(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.
Comment 107 Jonathan Druart 2013-05-03 11:53:19 UTC 
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
Comment 108 Dobrica Pavlinusic 2013-05-03 12:37:10 UTC 
(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.
Comment 109 Galen Charlton 2013-05-03 15:01:47 UTC 
(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.
Comment 110 Marcel de Rooy 2013-05-06 07:26:13 UTC 
(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.
Comment 111 Chris Cormack 2013-05-07 07:59:09 UTC 
(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');
Comment 112 Serhij Dubyk 2013-06-07 13:21:46 UTC 
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.
Comment 113 Serhij Dubyk 2013-06-07 13:23:27 UTC 
Created attachment 18746 [details]
Search for preferences in Cyrillic
Comment 114 Serhij Dubyk 2013-06-07 13:24:53 UTC 
Created attachment 18747 [details]
Search for circulation
Comment 115 Serhij Dubyk 2013-06-07 13:25:30 UTC 
Created attachment 18749 [details]
Members Home
Comment 116 Serhij Dubyk 2013-06-07 13:27:32 UTC 
Created attachment 18750 [details]
Search for members
Comment 117 Marcel de Rooy 2013-06-07 20:09:33 UTC 
(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 :)
Comment 118 Pongtawat 2013-06-18 09:16:19 UTC 
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?
Comment 119 Michal Denar 2013-09-04 11:15:25 UTC 
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
Comment 120 skobyanka@mail.ru 2013-09-07 09:51:31 UTC 
(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 %]
Comment 121 Ketan Kulkarni 2013-09-22 10:51:51 UTC 
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.
Comment 122 Ketan Kulkarni 2013-09-22 18:27:37 UTC Comment hidden (obsolete)
Comment 123 I'm just a bot 2013-09-24 05:35:44 UTC 
Patch applied cleanly, go forth and signoff
Comment 124 Chris Cormack 2013-09-24 07:15:23 UTC 
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
Comment 125 Ketan Kulkarni 2013-09-24 07:53:39 UTC 
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
Comment 126 Chris Cormack 2013-09-24 08:09:09 UTC 
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?
Comment 127 Ketan Kulkarni 2013-09-24 08:16:44 UTC 
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?
Comment 128 Chris Cormack 2013-09-24 08:24:07 UTC 
(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&amp;categorycode=B">Board</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=HB">Home Bound</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=IL">Inter-Library Loan</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=J">Juvenile</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=K">Kid</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=L">Library</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=PT">Patron</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=PERSON">person</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=SC">School</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=S">Staff</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=STAFF">staff</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=ST">Student</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=T">Teacher</a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode=YA">Young Adult</a></li>
                

After the patch
 <li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li><li><a href="/cgi-bin/koha/members/memberentry.pl?op=add&amp;categorycode="></a></li>
Comment 129 Ketan Kulkarni 2013-09-24 08:39:05 UTC 
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.
Comment 130 Josef Moravec 2013-09-24 08:42:21 UTC 
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.
Comment 131 Chris Cormack 2013-09-24 08:55:10 UTC 
(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.
Comment 132 Katrin Fischer 2013-09-24 08:58:55 UTC 
Ah, that sounds like bug 10864 - showing up on all our 3.12 installations for patron categories with umlauts.
Comment 133 Ketan Kulkarni 2013-09-26 09:34:38 UTC Comment hidden (obsolete)
Comment 134 Ketan Kulkarni 2013-09-26 11:14:38 UTC 
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?
Comment 135 Marcel de Rooy 2013-09-26 11:23:05 UTC 
(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..
Comment 136 Ketan Kulkarni 2013-09-26 14:50:22 UTC Comment hidden (obsolete)
Comment 137 Josef Moravec 2013-09-26 19:57:40 UTC 
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)
Comment 138 Ketan Kulkarni 2013-09-27 01:44:20 UTC 
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)
Comment 139 Josef Moravec 2013-09-27 05:26:53 UTC 
Created attachment 21506 [details]
Edit patron categories after patch 21488
Comment 140 Josef Moravec 2013-09-27 05:30:47 UTC 
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)
Comment 141 Ketan Kulkarni 2013-09-27 11:10:05 UTC 
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)
Comment 142 Ketan Kulkarni 2013-09-27 11:48:22 UTC Comment hidden (obsolete)
Comment 143 Ketan Kulkarni 2013-09-29 06:05:26 UTC Comment hidden (obsolete)
Comment 144 Srdjan Jankovic 2013-09-30 03:19:17 UTC Comment hidden (obsolete)
Comment 145 Srdjan Jankovic 2013-09-30 03:28:13 UTC 
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.
Comment 146 Ketan Kulkarni 2013-09-30 03:34:28 UTC 
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.
Comment 147 Ketan Kulkarni 2013-09-30 03:39:18 UTC 
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.
Comment 148 Bernardo Gonzalez Kriegel 2013-09-30 03:48:09 UTC 
> 
> 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
Comment 149 Ketan Kulkarni 2013-09-30 04:46:06 UTC Comment hidden (obsolete)
Comment 150 Srdjan Jankovic 2013-09-30 05:23:04 UTC Comment hidden (obsolete)
Comment 151 Biblibre Sandboxes 2013-09-30 20:46:14 UTC 
Patch tested with a sandbox, by Josef Moravec <josef.moravec@gmail.com>
Comment 152 Biblibre Sandboxes 2013-09-30 20:49:26 UTC 
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>
Comment 153 Tomás Cohen Arazi (tcohen) 2013-10-02 15:32:34 UTC 
I marked 10068 as 'see also', not because of the patches, but to mention the first comment.
Comment 154 Dobrica Pavlinusic 2013-10-03 10:43:09 UTC 
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.
Comment 155 Ketan Kulkarni 2013-10-03 11:06:20 UTC 
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.
Comment 156 Bernardo Gonzalez Kriegel 2013-10-03 12:59:04 UTC 
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.
Comment 157 Ketan Kulkarni 2013-10-04 01:25:06 UTC 
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.
Comment 158 Tomás Cohen Arazi (tcohen) 2013-10-04 15:35:30 UTC 
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.
Comment 159 Ketan Kulkarni 2013-10-05 03:12:20 UTC 
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.
Comment 160 Chris Cormack 2013-10-05 03:15:08 UTC 
(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.
Comment 161 Dobrica Pavlinusic 2013-10-07 13:17:58 UTC 
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.
Comment 162 Marcel de Rooy 2013-10-10 07:50:50 UTC 
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
Comment 163 Ketan Kulkarni 2013-10-10 08:49:29 UTC 
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.
Comment 164 Marcel de Rooy 2013-10-10 10:18:28 UTC 
(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.
Comment 165 Ketan Kulkarni 2013-10-11 04:12:56 UTC 
Created attachment 21955 [details]
My opac xslt preferences

Please see my OPAC preferences.
Comment 166 Marcel de Rooy 2013-10-14 06:44:38 UTC 
(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.
Comment 167 Marcel de Rooy 2013-10-14 07:19:48 UTC 
Created attachment 21978 [details]
opac-search

Please look at mangled characters with diacritics in the XSLT blocks and facets.
Comment 168 Marcel de Rooy 2013-10-14 08:35:31 UTC 
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..
Comment 169 Bernardo Gonzalez Kriegel 2013-10-14 10:52:22 UTC 
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.
Comment 170 Marcel de Rooy 2013-10-14 11:13:22 UTC 
(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?
Comment 171 Bernardo Gonzalez Kriegel 2013-10-14 11:24:41 UTC 
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)
Comment 172 Marcel de Rooy 2013-10-14 11:34:31 UTC 
(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?
Comment 173 Bernardo Gonzalez Kriegel 2013-10-14 11:48:17 UTC 
(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
Comment 174 Serhij Dubyk 2013-11-22 19:37:50 UTC 
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.
Comment 175 Tomás Cohen Arazi (tcohen) 2013-11-22 20:39:57 UTC 
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.
Comment 176 Tomás Cohen Arazi (tcohen) 2013-11-22 20:46:56 UTC 
(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?
Comment 177 Chris Cormack 2013-11-22 21:36:06 UTC 
(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
Comment 178 Chris Cormack 2013-11-22 22:09:13 UTC 
Patch on bug 11290
Comment 179 Jonathan Druart 2013-11-25 15:46:20 UTC Comment hidden (obsolete)
Comment 180 Jonathan Druart 2013-11-25 15:48:10 UTC 
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.
Comment 181 Jonathan Druart 2013-11-26 08:35:49 UTC 

*** This bug has been marked as a duplicate of bug 11302 ***
Comment 182 Marcel de Rooy 2013-11-26 14:06:08 UTC 
(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.
Comment 183 Jonathan Druart 2013-11-26 14:21:01 UTC 
This one became too long. It could be reopened if 11302 is wrong.
Comment 184 Jacek Ablewicz 2013-12-05 18:06:44 UTC 
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 ?
Comment 185 Chris Cormack 2013-12-05 19:46:43 UTC 
Please can we open new bugs for each instance, with new valid patches, this bug is impossible to maintain any more.
Comment 186 Jonathan Druart 2013-12-06 09:39:03 UTC 
(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.
Comment 187 Jonathan Druart 2014-03-14 15:33:08 UTC 
For more fun, please see bug 11944.