GNOME Bugzilla – Bug 627402
Support marking FolksPersonas as "me"
Last modified: 2010-09-21 09:23:10 UTC
libfolks needs to have some way of indicating that an Individual and its Personas are the user, since various things should be handled differently for them (such as setting aliases). The user's Individual should probably also be shown differently in UIs.
Created attachment 168567 [details] [review] Squashed diff of the 627402-is-user branch http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/627402-is-user Note that this branch is based on http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/626725-personas-changed, since it makes some changes to the IndividualAggregator.
Review of attachment 168567 [details] [review]: IndividualAggregator.link_personas() needs to use a foreach(...) for iterating the personas list. Currently the lambda function tries to throw an error, which is invalid (since the lambda's type doesn't throw an error). Other than that, the code looks fine, but I hit a number of bogus favorites warnings when running Empathy on top of this; eg, TelepathyBackend-WARNING **: tpf-persona-store.vala:260: unknown persona 'user4@localhost' in favourites list (it's normally a valid favorite).
Created attachment 169170 [details] [review] Squashed diff of the 627402-is-user-rebase1 branch http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/627402-is-user-rebase1
Review of attachment 169170 [details] [review]: It looks like Tpf.PersonaStore.self_contact isn't defined: tpf-persona-store.vala:263.17-263.33: error: The name `self_contact' does not exist in the context of `Tpf.PersonaStore' (this.self_contact != null && this.handle_persona_map.size > 1)))
Review of attachment 169170 [details] [review]: Disregard previous review; acquire ++
Review of attachment 169170 [details] [review]: Actually, I used the branch a little longer and hit this: ================================== Core:ERROR:individual.c:1390:_folks_individual_set_personas: assertion failed: (folks_persona_get_is_user (p) == self->priv->_is_user) [Thread 0x7fffe60e1710 (LWP 16835) exited] [Thread 0x7fffe4cd1710 (LWP 16837) exited] Program received signal SIGABRT, Aborted. 0x00007fffed011175 in *__GI_raise (sig=<value optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory. in ../nptl/sysdeps/unix/sysv/linux/raise.c (gdb) bt
+ Trace 223542
Created attachment 169580 [details] [review] Squashed diff of the 627402-is-user-attempt2 branch http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/627402-is-user-attempt2 This second attempt changes the guarantees provided by the branch slightly. Previously, the branch was only allowing Personas with Persona.is_user == Individual.is_user to link to a given Individual. This resulted in all the contacts for user accounts being linked together, but not (for example) being linked to contacts for each account as added to another account. The branch now takes the approach that Personas with Persona.is_user == true are still explicitly linked to IndividualAggregator.user, but there's nothing preventing other Personas from being linked to this Individual (either by the user adding links, or implicitly due to sharing an e-mail address or something). The quirk with this approach is that the user's link-local XMPP contact will now be linked, which is fine from a security point of view, but doesn't quite fit in with the policy that link-local XMPP accounts aren't linked at all. I don't think this is a problem though, since Empathy will most likely hide the Individual with Individual.is_user == true. However, that introduces a different problem: if the Individual with Individual.is_user == true is hidden, people will no longer be able to add themselves as contacts (e.g. on a different account) and talk to themselves (they won't show up in their own contact list, because the contact will be implicitly linked with IndividualAggregator.user and subsequently hidden). I'm not sure this is much to be concerned about.
(In reply to comment #7) > However, that introduces a different problem: if the Individual with > Individual.is_user == true is hidden, people will no longer be able to add > themselves as contacts (e.g. on a different account) and talk to themselves > (they won't show up in their own contact list, because the contact will be > implicitly linked with IndividualAggregator.user and subsequently hidden). I'm > not sure this is much to be concerned about. After some discussion on IRC, this is something to be concerned about, since quite a few people use this “feature” to send themselves files, or test things. A fairly simple solution which shouldn't complicate Empathy's code too much is just to filter out all Personas where Persona.is_user == true from Empathy (probably in the EmpathyIndividualManager). In the case that this filters out all Personas from an Individual, the Individual is ignored. This should happen without any extra work, as we already do this for Individuals which (e.g.) only contain a Kf.Persona.
(Mass changing milestones; please search for this phrase to delete the bug spam.)
Review of attachment 169580 [details] [review]: Second patch, in tpf-persona-store.vala: + ((this.self_contact == null && this.self_contact isn't defined at this point, so it looks like this was a slightly skewed re-base. Not worth fixing, just noting. In the final patch, + warning ("Failed to create persona from contact '%s' (%p)", It'd be nice to make this "Failed to create self persona[...]" to make it a distinct warning. + * @since 0.1.17 There are several of these throughout; they should be changed to @since 0.3.0. After those changes, please merge.
commit 95a282dc35afcaea7534bde0fff2831d25957f83 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Aug 23 17:26:35 2010 +0100 Support the Persona.is_user property in the telepathy backend Closes: bgo#627402 NEWS | 3 + backends/telepathy/lib/tpf-persona-store.vala | 72 ++++++++++++++++++++++++- backends/telepathy/lib/tpf-persona.vala | 6 ++- folks/persona-store.vala | 8 +++ 4 files changed, 86 insertions(+), 3 deletions(-) commit 0457ec58cc8ee691ed3ed568a147af4a3a83a294 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Aug 23 17:26:18 2010 +0100 Support the Persona.is_user property in the key-file backend Helps: bgo#627402 backends/key-file/kf-persona.vala | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) commit 61322635e38d64130d7333ab366aa5e32a4c73a8 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Sep 6 13:49:31 2010 +0100 Implicitly trust Personas which are marked as being the user We assume that the backend is in complete control of its user accounts, so we can fully trust Personas which have Persona.is_user == true. Helps: bgo#627402 folks/individual.vala | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) commit 24732788143198877b528dcf06f147a77ee2b7e7 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Aug 23 16:31:49 2010 +0100 Add a "user" property to the IndividualAggregator This contains the Individual representing the user who owns all the relevant accounts in each backend. This also changes the linking code so that Personas with is_user == true are always linked, and only ever linked to other Personas with is_user == true (i.e. Personas are always linked to Personas with like is_user values). This ensures that there's only ever one Individual with is_user == true. Helps: bgo#627402 NEWS | 1 + backends/telepathy/lib/tpf-persona-store.vala | 10 ++++++- folks/individual-aggregator.vala | 34 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) commit a124e5c7cfffd3896e51f708d52eb0a16f795d7a Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Aug 23 16:11:08 2010 +0100 Add an "is-user" property to Individual and Persona This is true iff the Individual or Persona is the owner of the relevant accounts. Helps: bgo#627402 NEWS | 2 ++ folks/individual.vala | 41 +++++++++++++++++++++++++++++++++++++++++ folks/persona.vala | 10 ++++++++++ 3 files changed, 53 insertions(+), 0 deletions(-) commit 78d808e6771cd2395bca6aa47c3b2f28391e129b Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Aug 31 15:19:31 2010 +0100 Use foreach{} rather than .foreach() in IndividualAggregator.link_personas() folks/individual-aggregator.vala | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)