[Kde-pim] [PATCH] kmail recipient picker fixes

[Sander van Grieken]

On Monday 12 November 2007 20:15:12 Thomas McGuire wrote:
> Hello,
>
> On Sunday 11 November 2007, Sander van Grieken wrote:
> > Sorry to post a big patch again, but please disregard previous patch and
> > use this one.
>
> OK, I've commited it now with revision 735840. I've made some small
> adjustments (adding const, replacing the deprecated replace() and style
> fixes) before commiting.
>
> However, there is still one problem, which leads to a crash. If you have
> two resources with the same name (try with kcmshell4 kresources), it will
> crash. The reason is probably insertCollection(), which compares by name,
> which won't work in this case.
> I suggest comparing by some unique ID string instead. I think it is best if
> the collection ID string is the same as the normal name, expect for
> resources, where it probably can be Resource::identifier(), which is
> supposed to be unique (see
> http://api.kde.org/4.0-api/kdepimlibs-apidocs/kresources/html/classKRES_1_1
>Resource.html ).
>
> There was (is?) a bug in KDEPIM that there were two resources with the same
> name by default, so this is not unimportant.

Yep that would be much better. I'll look into it.

> I also noticed (but didn't try) that the code won't work if you have the
> same category in multiple resources. In this case, it seems like only the
> addressees from the last inserted addressbook are inserted into the
> category in this case. The old code was also buggy, there the categorys
> would be created twice. I think the category logic in insertAddressBook()
> needs some adjustments. Maybe there should be a map in RecipientsPicker
> which maps category names to collections (similar to the local one, but on
> the class-level). Then in insertAddressBook(), try to get an existing
> category collection if one exits, else create a new one, then add
> the recipient.

I think it's always the same AddressBook instance that gets passed. If that's 
true, it seems like it's working correctly. Or is there something like 
multiple addressbooks?

> I hope you understood my confused writing here :)
>
> Please fix this, at least the crash with two resources with the same name
> (against the new trunk version, so the patch contains only those changes).
>
> Thanks again for the patch, it is always good to see problems in trunk get
> fixed.

It was an itch in need of scratching :)

I was also hoping this would find its way back into 3.5

grtz,
Sander
_______________________________________________
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/