Bug 23589 - Discharge notice does not show non-latin characters
Summary: Discharge notice does not show non-latin characters
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Dobrica Pavlinusic
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-11 10:59 UTC by Theodoros Theodoropoulos
Modified: 2019-11-14 15:48 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.05


Attachments
Bug 23589: Discharge notice does not show non-latin characters (1.58 KB, patch)
2019-10-02 09:34 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 23589: Discharge notice does not show non-latin characters (1.63 KB, patch)
2019-10-02 10:44 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 23589: Discharge notice does not show non-latin characters (1.69 KB, patch)
2019-10-03 13:17 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Theodoros Theodoropoulos 2019-09-11 10:59:33 UTC
The discharge notice skips any non latin characters, whether in notice body or in patron details.

You can verify this by either:
a) modifying the default discharge notice slip and entering a non latin string [ie. "Δοκιμή" (which means testing in Greek)]
b) modifying a user firstname/lastname, etc and printing <<borrowers.firstname>> <<borrowers.surname>> in discharge notice slip

In current master (Sep. 2019), in both cases, the non-latin strings will be skipped in the pdf output.
Comment 1 Theodoros Theodoropoulos 2019-09-11 11:01:44 UTC
Clarification:

b) modifying a user firstname/lastname ADDING NON-LATIN CHARACTERS and printing <<borrowers.firstname>> <<borrowers.surname>> in discharge notice slip
Comment 2 Theodoros Theodoropoulos 2019-09-11 12:02:15 UTC
Additional findings:
- src/Koha/Patron/Discharge.pm uses PDF::FromHTML->new( encoding => 'utf-8') as it should
- temporary html notice IS created AND shows all characters properly, including non-latin ones!

It seems that the issue is created during html2pdf conversion and might have to do with the default font used.
There is an html2pdf utility (that is based on PDF::FromHTML module) that has the same behavior. This utility allows the user to specify the output font in the pdf. I've experimented with a couple of fonts and there was one case where I managed to see only one line with non-latin characters using a DejaVu font.
Comment 3 Dobrica Pavlinusic 2019-10-02 09:34:02 UTC
Created attachment 93402 [details] [review]
Bug 23589: Discharge notice does not show non-latin characters

It seems that discarge notices never got support for truetype
fonts, so notices are missing non-latin characters.
Comment 4 Dobrica Pavlinusic 2019-10-02 09:37:04 UTC
I had similar problem and here is a fix. You have to have truetype fonts defined in koha-conf.xml for this to work. If it does, I would appreciate sign-off :-)
Comment 5 Theodoros Theodoropoulos 2019-10-02 10:24:39 UTC
Hi Dobrica,

Thanks for your patch! Indeed it fixes the output. I will try to properly sign it off later.

Having said that, there I'm now at 18.11 and there is another issue (that exists regardless of your patch). The generated pdf file in /tmp/ is proper pdf, the one that gets downloaded when I click discharge is CORRUPTED (has html code appended in the bottom of the pdf file). Did you also have this issue?
Comment 6 sandboxes@biblibre.com 2019-10-02 10:44:11 UTC
Patch tested with a sandbox, by Theodoros Theodoropoulos <theod@lib.auth.gr>
Comment 7 sandboxes@biblibre.com 2019-10-02 10:44:34 UTC
Created attachment 93413 [details] [review]
Bug 23589: Discharge notice does not show non-latin characters

It seems that discarge notices never got support for truetype
fonts, so notices are missing non-latin characters.

Signed-off-by: Theodoros Theodoropoulos <theod@lib.auth.gr>
Comment 8 Theodoros Theodoropoulos 2019-10-02 10:51:26 UTC
Maybe you could also test (and sign off if it works for you) a patch I submitted for BZ23514 here?
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23514
Comment 9 Dobrica Pavlinusic 2019-10-02 18:22:36 UTC
(In reply to Theodoros Theodoropoulos from comment #5)
> Hi Dobrica,
> 
> Thanks for your patch! Indeed it fixes the output. I will try to properly
> sign it off later.
> 
> Having said that, there I'm now at 18.11 and there is another issue (that
> exists regardless of your patch). The generated pdf file in /tmp/ is proper
> pdf, the one that gets downloaded when I click discharge is CORRUPTED (has
> html code appended in the bottom of the pdf file). Did you also have this
> issue?

You are absolutely right. We indeed emit html (and headers) after pdf. I never noticed this before. This is on 19.05 for me. However, on current master I don't see anything after pdf. I thinks this is difference in configuration and not versions. 19.05 for me (with html junk) is running via plack while my development is plain cgi. Are you also running plack?
Comment 10 Dobrica Pavlinusic 2019-10-02 19:10:56 UTC
I can confirm that this is plack problem. If I implement something similar to safe_exit in C4::Auth it doesn't emit extra html after pdf.

However, this is not the only place where we have exit and while some code flows would exit anyway (since under plack exit doesn't work) I'm sure we have more places in code which should be fixed (ater redirects is one obvious one -- it would seem to work, but we are probably sending Location: header and whole html after it).

This will have to be another bug since it will touch many places and isn't really related to this one.
Comment 11 Theodoros Theodoropoulos 2019-10-03 05:28:08 UTC
Indeed, I can verify that I'm using plack too.

The new bug sounds a bit (more) critical... Although the fix would be fairly straight forward (replacing exit with safe_exit), identifying where is needed and testing that the patch does the trick might be tricky...
Comment 12 Theodoros Theodoropoulos 2019-10-03 12:00:14 UTC
Test plan for QA

1/ If using Plack, make sure https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23723 is applied. Otherwise, produced PDF may not be properly encoded.
2/ Go to System Preferences and set useDischarge: Allow
3/ Create (or edit) a patron, entering non-latin characters for example in Surname and/or First Name
4/ In Tools->Notices & slips, make sure you use standard notice settings in DISCHARGE, OR at least make sure you print <<borrowers.firstname>> and/or <<borrowers.surname>>
5/ Goto Patron page -> discharges -> generate discharge
6/ Produced PDF shows no data where <<borrowers.firstname>> and/or <<borrowers.surname>> should have appeared.

7/ apply patch

8/ Goto Patron page -> discharges -> generate discharge
9/ Produced PDF properly shows the data from <<borrowers.firstname>> and/or <<borrowers.surname>> columns
Comment 13 Katrin Fischer 2019-10-03 12:55:45 UTC
(In reply to Theodoros Theodoropoulos from comment #12)
> Test plan for QA
> 
> 1/ If using Plack, make sure
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23723 is applied.
> Otherwise, produced PDF may not be properly encoded.
> 2/ Go to System Preferences and set useDischarge: Allow
> 3/ Create (or edit) a patron, entering non-latin characters for example in
> Surname and/or First Name
> 4/ In Tools->Notices & slips, make sure you use standard notice settings in
> DISCHARGE, OR at least make sure you print <<borrowers.firstname>> and/or
> <<borrowers.surname>>
> 5/ Goto Patron page -> discharges -> generate discharge
> 6/ Produced PDF shows no data where <<borrowers.firstname>> and/or
> <<borrowers.surname>> should have appeared.
> 
> 7/ apply patch
> 
> 8/ Goto Patron page -> discharges -> generate discharge
> 9/ Produced PDF properly shows the data from <<borrowers.firstname>> and/or
> <<borrowers.surname>> columns

Perfect, thank you!
Comment 14 Katrin Fischer 2019-10-03 13:17:57 UTC
Created attachment 93595 [details] [review]
Bug 23589: Discharge notice does not show non-latin characters

It seems that discarge notices never got support for truetype
fonts, so notices are missing non-latin characters.

Signed-off-by: Theodoros Theodoropoulos <theod@lib.auth.gr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Martin Renvoize 2019-10-07 11:23:03 UTC
It's great to see you back Dobrica Pavlinusic, I've missed seeing your name popping up :)
Comment 16 Martin Renvoize 2019-10-07 11:58:22 UTC
Nice work!

Pushed to master for 19.11.00
Comment 17 Fridolin SOMERS 2019-11-14 15:48:42 UTC
Pushed to 19.05.x for 19.05.05