Post by Jakob Schroeter
the author of the compface library agreed to change its license to
LGPL. So it should be possible to add my patches for kmail and knode
to kdepim. This is wished for in BR28319 and BR92261.
Please find the patch at
The KXFace class is added to libkdepim. It is mostly based on
libcompface. In KMail, X-Faces can be set per identity, with preview.
They are displayed in the message reader alongside the headers for
the fancy and standard header styles. It is currently not yet
possible to disable the X-Face display as wished for on kmail-devel.
The KNode part adds support for displaying X-Faces only, you would
have to manually create an appropriate X-Face header.
I would like to commit by next friday if there are no serious
objections by then.
What do you think?
I general the patch is okay and it works quite nicely. But it's not
ready for committing. Let's go into the details:
- The X-Face image should only be shown with the fancy header style. In
particular this means that moving FancyHeaderStyle::imgToDataUrl to
HeaderStyle::imgToDataUrl is not necessary.
- The X-Face image should only be shown if there's no image for the
sender in the address book, i.e. if photoURL.isEmpty(). See
headerstyle.cpp around line 460.
- I think "X-Face" is a bad description for the tab. I suggest using
"Photo" instead. On the tab there should then be some text which
explains that currently only black-white photos are supported.
- The license of this file should be GPL + Qt exception as for example
- It's not acceptable (usability-wise) that the user has to create the
X-Face header himself. The user should just have to select an image
file. Then an X-Face should be created from this image (i.e. scale it
to 48x48 and then make it black-white). Internally the X-Face should be
- The links are not necessary when the user can simply select an image
- It doesn't seem to be necessary to link against libkmime and to add
- Please give comp and uncomp more meaningful names. From the name it's
completely unclear to me what those methods do.
- I don't like all the #defines in kxface.h. This will probably lead to
conflicts. Therefore please move all defines which are not needed in
kxface.h to kxface.cpp and replace the remaining defines by private
static const members of KXFace or by a private typedefs (in case of
- Pass QString as 'const QString &' in comp, uncomp and toPixmap.
- KXFace::toPixmap doesn't look very efficient. tmp should be char
instead of QString. And the long if-else statement should be a switch
- KXFace::comp doesn't guard against a possible buffer overflow.
+ char *fbuf = (char *)malloc(MAX_XFACE_LENGTH);
+ memset(fbuf, '\0', MAX_XFACE_LENGTH);
+ strncpy(fbuf, str.latin1(), str.length());
You have to add a str.truncate( MAX_XFACE_LENGTH ) in the first line.
Interestingly KXFace::uncomp checks the length of the parameter.
- KXFace::x and KXFace::y should be static methods and the parameter
should be passed as 'const int pos'.
Now a few things which could be done to improve the feature:
- In the configuration: Add the possibility to load the X-Face from the
image in the user's own address book entry.
- Add the possibility to store the X-Face in the address book entry of
the sender (via an action in the context menu).