[Kde-pim] Re: Review Request: Fix non-ascii symbols in From and To symbols replaced to "?" when sending email

[Thomas McGuire]

> On April 13, 2011, 9:17 a.m., Thomas McGuire wrote:
> > Christian, thank you for looking into this and apologies for not answering on the bug report.
> > The patch seems to work around the problem, however I would prefer a fix in a more appropriate place. The To/From/CC headers are first set in SkeletonMessageJobPrivate::doStart() in messagecomposer/skeletonmessagejob.cpp, I assume this is the place in which the charset is not set correctly (or simply not set at all). That function also sets the subject, for which it works correctly.
> > 
> > In addition, could you add a unit test for this, to ensure that the problem stays fixed? Just add a quick test to tests/composertest.cpp for this.
> 
> Cristian Oneț wrote:
>     I totally agree with the fact that this is just a workaround for the issue.
>     
>     As for the fix being made in messagecomposer/skeletonmessagejob.cpp I have also thought of that and I'm afraid that it will not work since the KMime::Message object created there will be serialized as a byte array in composer.cpp:462 and the byte array is used to create a new KMime::Message at composer.cpp:463-465 (the bug appears at this point). The real problem is that the from/to/replyTo/cc/bcc elements contained by this instance of KMime::Message object have the RFC2047 Charset set to Latin1. I dont get it why, when parsing a RFC2047 serialized element which had the charset UTF-8, the charset of the resulting KMime object is not set to UTF-8. I think that this is the real root of the problem.
>     
>     I could add a test case but if I don't have the proper fix yet... should I submit a review request containing only the test case?
> 
> Thomas McGuire wrote:
>     Seems like that in ComposerPrivate::composeFinalStep(), allData already is an invalid message, for the new unit tests either the wrong charset (ISO-8859-1) or no charset at all is used.
>     What happens after that point, in line 463-465, is not relevant, since the message is buggy before already.

I have added a unit test now, which fails, even with the patch applied.
I don't have time to investiage this any further right now. Might even be that the QString::fromUtf8() doesn't work as I expect it to.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101109/#review2603
-----------------------------------------------------------


On April 12, 2011, 8:34 p.m., Cristian Oneț wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101109/
> -----------------------------------------------------------
> 
> (Updated April 12, 2011, 8:34 p.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Summary
> -------
> 
> As stated in the bug report this might not be the cleanest fix that bug can have but since no kdepim developer has yet stepped up to fix the bugI submitted this review request hoping to draw some attention to this issue.
> 
> The bug report also contains the description of the source of the bug so there is no point in duplicating it here.
> 
> 
> This addresses bug 263761.
>     http://bugs.kde.org/show_bug.cgi?id=263761
> 
> 
> Diffs
> -----
> 
>   messagecomposer/composer.cpp ec3cd2f 
> 
> Diff: http://git.reviewboard.kde.org/r/101109/diff
> 
> 
> Testing
> -------
> 
> Run kmail2 and add contacts with non-ascii characters in their name. Save as a draft or send the mail and observe that the non-ascii characters are successfully preserved.
> 
> 
> Thanks,
> 
> Cristian
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/