Bug 21875

Summary: Handling subject line in Letters.pm
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Architecture, internals, and plumbingAssignee: Marcel de Rooy <m.de.rooy>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, jonathan.druart, katrin.fischer, martin.renvoize, nick, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Who signed the patch off: Text to go in the release notes:
This enhancement improves the display of subject lines in messages so that they correctly show non-Latin characters.
Version(s) released in:
Attachments: Bug 21875: Handling subject line in Letters.pm
test assumptions
Bug 21875: Handling subject line in Letters.pm
Bug 21875: Handling subject line in Letters.pm
Bug 21875: Handling subject line in Letters.pm
Bug 21875: Handling subject line in Letters.pm

Description Marcel de Rooy 2018-11-23 10:34:53 UTC
See bug 21874 for discussion
Comment 1 Marcel de Rooy 2018-11-23 10:37:23 UTC Comment hidden (obsolete)
Comment 2 Martin Renvoize 2018-11-23 11:27:32 UTC
Created attachment 82611 [details]
test assumptions
Comment 3 Marcel de Rooy 2018-11-26 08:40:40 UTC
(In reply to Martin Renvoize from comment #2)
> Created attachment 82611 [details]
> test assumptions

Thanks for trying.
You prove that encode(decode(encode))=encode and encode(encode)) != encode.

This does not justify keeping a decode where it does not belong. The subject field coming from the database is plain text and should not be handled as a mime header. Even if it contained mime encoded text [highly theoretical only], it should not be seen as mime but as plain text and displayed as-is.
Comment 4 Marcel de Rooy 2018-12-04 14:56:22 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2018-12-04 14:57:53 UTC
Test plan (Bugzilla only to prevent git bz encoding stuff):

[1] Make Koha send a queue message with a plain ASCII subject line. Verify the subject line at the receiving side.
Possible in many ways. Here is one: Enable list sharing and send a share invitation on a list with a plain ASCII text name like "L2".
[2] Now send a subject line with some Latin chars. Verify.
Rename your list L2 from step 1 to L2áéò. Send another share invitation.
[3] Now send a subject line with some 'real' Unicode chars. Verify.
Rename L2áéò to L2áéò癱覂. Repeat the invite.
[4] Now include MIME encoding in the list name. (To make Martin happy ;) Verify.
Rename L2áéò癱覂 to =?UTF-8?B?IHBlcmwgSsO8?= (include the equal signs)
Since I gave the list a strange and ugly name, I would like to see that name in the subject line. (You will also see it in the message body.)
Note: Without this patch the subject line is decoded to "Share list perl Jü". This is wrong, because it is not my list name!
Bonus: Try some other email address/mail program too.
Comment 6 Marcel de Rooy 2018-12-04 14:58:50 UTC
Created attachment 82841 [details] [review]
Bug 21875: Handling subject line in Letters.pm

The variable name $utf8 is very misleading: it contains MIME-Header encoding.
$message->{subject} comes from the database and is in perl internal format;
it should NOT be decoded as a MIME-Header.
After encoding to MIME-Header, previously another (useless) encoding to
UTF-8 was done. Since the string is plain ASCII, this is useless and
theoretically wrong. We should stay in MIME-Header.

Test plan:
See Bugzilla comment5.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2018-12-04 15:01:46 UTC
Martin,
Please check the fourth step in the test plan on comment5. Hope it does better explain my point of view.
Thanks,
Marcel
Comment 8 Jonathan Druart 2019-02-14 20:59:54 UTC
Original code comes from
  commit 7c08d7c81edf6b25d74c71d57414dc9da78810d8
  Bug #6130 - Letters: Fixing Subject encoding
Which reads wrong.

This patch looks ok to me.
Comment 9 Jonathan Druart 2019-02-14 21:00:56 UTC
Created attachment 85139 [details] [review]
Bug 21875: Handling subject line in Letters.pm

The variable name $utf8 is very misleading: it contains MIME-Header encoding.
$message->{subject} comes from the database and is in perl internal format;
it should NOT be decoded as a MIME-Header.
After encoding to MIME-Header, previously another (useless) encoding to
UTF-8 was done. Since the string is plain ASCII, this is useless and
theoretically wrong. We should stay in MIME-Header.

Test plan:
See Bugzilla comment5.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2019-02-14 21:01:30 UTC
Tests are not trivial to write here.
Comment 11 Marcel de Rooy 2019-02-15 06:35:24 UTC
(In reply to Jonathan Druart from comment #9)
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Thanks
Comment 12 Martin Renvoize 2019-02-20 09:07:14 UTC
OK, I think I'm happy to yield here.. Passing QA
Comment 13 Martin Renvoize 2019-02-20 09:08:10 UTC
Created attachment 85317 [details] [review]
Bug 21875: Handling subject line in Letters.pm

The variable name $utf8 is very misleading: it contains MIME-Header encoding.
$message->{subject} comes from the database and is in perl internal format;
it should NOT be decoded as a MIME-Header.
After encoding to MIME-Header, previously another (useless) encoding to
UTF-8 was done. Since the string is plain ASCII, this is useless and
theoretically wrong. We should stay in MIME-Header.

Test plan:
See Bugzilla comment5.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Marcel de Rooy 2019-02-20 12:04:43 UTC
(In reply to Martin Renvoize from comment #12)
> OK, I think I'm happy to yield here.. Passing QA

Thx.
Comment 15 Nick Clemens 2019-02-27 14:16:48 UTC
Awesome work all!

Pushed to master for 19.05
Comment 16 Martin Renvoize 2019-03-01 11:29:53 UTC
Enhancement will not be backported to 18.11.x series.