Bug 12411 - Encoding broken in the syspref template plugin
Summary: Encoding broken in the syspref template plugin
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-12 15:14 UTC by Jonathan Druart
Modified: 2016-12-05 21:23 UTC (History)
6 users (show)

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


Attachments
Bug 12411: Preferences values should be utf8 encoded (1.10 KB, patch)
2014-06-12 15:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12411: Preferences values should be utf8 encoded (1.19 KB, patch)
2014-06-19 16:27 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 12411 - Encoding broken in the syspref template plugin (1.26 KB, application/mbox)
2014-06-20 16:07 UTC, Kyle M Hall
Details
Bug 12411 - Encoding broken in the syspref template plugin (1.26 KB, patch)
2014-06-20 16:07 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-06-12 15:14:10 UTC
This will fix the encoding issues existing in 3.14 and 3.16.
The issue still appear in master but is fixed by bug 11944.

If you try to add non latin characters in a pref like OPACMySummaryNote or NoLoginInstructions, the display is broken.
Comment 1 Jonathan Druart 2014-06-12 15:15:32 UTC Comment hidden (obsolete)
Comment 2 Paola Rossi 2014-06-13 14:31:59 UTC
On master 3.17.00.007, without applying 12411 [and without applying 11944] the display is not broken.
Comment 3 Jonathan Druart 2014-06-13 14:44:15 UTC
Which sysprefs did you test?
Comment 4 Paola Rossi 2014-06-13 16:12:25 UTC
(In reply to Jonathan Druart from comment #3)
> Which sysprefs did you test?

I've tested OPACMySummaryNote and NoLoginInstructions
Comment 5 Jonathan Druart 2014-06-19 09:21:13 UTC
Without this patch, I filled "NoLoginInstructions" with "Hello éàç". Go on opac-user.pl and I see  "Hello ���"

Template 'en' and bootstrap theme.
Comment 6 Paola Rossi 2014-06-19 16:25:37 UTC
(In reply to Jonathan Druart from comment #5)
> Without this patch, I filled "NoLoginInstructions" with "Hello éàç". Go on
> opac-user.pl and I see  "Hello ���"
> 
> Template 'en' and bootstrap theme.

Today I've made some tests on master 3.17.00.008 without applying the patch.
The cases I made were on both the two prefs.

With the string "Something éàç", the behaviour of master was the one you said. I see:
Something ���

On the contrary, with the string "Μαρωνίτης 実 éàç" I see no strange char. I see:
Μαρωνίτης 実 éàç 
--------------------------------------------------------------------
éàç are all in [128:255], whilest the other chars are [256:+).
--------------------------------------------------------------------

Afterwards I've applied the patch. 
Both the two cases were OK.

So I've passed the patch to "Signed Off" status.
Comment 7 Paola Rossi 2014-06-19 16:27:13 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2014-06-20 16:07:02 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2014-06-20 16:07:55 UTC
Created attachment 29113 [details] [review]
Bug 12411 - Encoding broken in the syspref template plugin
Comment 10 Jonathan Druart 2014-08-25 07:46:23 UTC
This patch should not be pushed to master (unless bug 11944 is not pushed until the 3.18 release).
I cc Galen and Fridolin, RM for 3.16 and 3.14.
Comment 11 Tomás Cohen Arazi 2014-11-21 23:52:03 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 12 Mason James 2015-07-21 02:31:57 UTC
(In reply to Jonathan Druart from comment #0)
> This will fix the encoding issues existing in 3.14 and 3.16.
> The issue still appear in master but is fixed by bug 11944.
> 
> If you try to add non latin characters in a pref like OPACMySummaryNote or
> NoLoginInstructions, the display is broken.

(In reply to Jonathan Druart from comment #10)
> This patch should not be pushed to master (unless bug 11944 is not pushed
> until the 3.18 release).
> I cc Galen and Fridolin, RM for 3.16 and 3.14.

Pushed to 3.16.x, will be in 3.16.13

Fridolin, you might want to add this to 3.14.x?
Comment 13 Mason James 2015-07-25 13:14:23 UTC
Pushed to 3.16.x, will be in 3.16.13