[Bug 27669] Implement Location interface support

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=27669

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
  Status Whiteboard|                            |review-
           Keywords|patch                       |
         AssignedTo|telepathy-bugs at lists.freede |andrunko at gmail.com
                   |sktop.org                   |

--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-20 07:11:33 PDT ---
Please separate this into two branches, one for other contacts' locations
(which is quite easy) and one for setting our own location (which is much
harder, and could benefit from some spec work).

Other people's locations
========================

> +    if (!mPriv->requestedFeatures.contains(FeatureLocation)) {

Is requestedFeatures really the right member there?

> -QVariantMap Contact::location() const
> +ContactLocation *Contact::location() const

This seems to return a pointer that's only valid as long as the Contact is. Is
that appropriate? It should at least be documented.

It should probably either be a const pointer that's documented to have the same
validity as the Contact, or a refcounted Tp::ContactLocationPtr?

--------------------------------

Our own location
================

> + * This is useful if the user wants to obscure their exact location
> + * by reducing the precision or accuracy, instead of relying on the
> + * connection manager to do so.

This isn't true - you call setSelfLocation regardless of whether you want to
fuzz your location or not. How about this?

 * Set the user's location, to be advertised to other contacts.
 *
 * The connection manager will advertise this as precisely as
 * possible, so if you want to tell contacts an obscured or
 * reduced-precision version of the user's location, you must
 * reduce the precision yourself.
 *
 * Clients that interact with more than one connection should advertise the
same
 * reduced-accuracy location to all of them, so that contacts cannot obtain an
 * undesirably accurate location by assuming that random errors have been added
 * and averaging the locations advertised on multiple connections.

> Also the name FeatureSelfLocation may be misleading as there is no info about
> the current user self location retrieved, but only the access control.
> Suggestions are welcome.

Perhaps FeatureLocationSetting? When we've enhanced the spec to have a CanSet
flag (see the spec bug I'm about to file), this feature could fail to prepare
on Google Talk accounts (which don't have PEP, so Gabble can't advertise
locations on them).

> +    // TODO How to know when location access control changed?

You're right, there should be a change notification signal in tp-spec for this
(I'll file a bug). In the meantime, telepathy-qt4 should respond to Set() by
changing the cached value.

> +PendingOperation *Connection::selfSelfLocationAccessControl(

I think you mean setSelf, not selfSelf

> +        warning().nospace() << "Getting channels failed with " <<
> +            reply.error().name() << ":" << reply.error().message();

"Getting location properties failed", surely...

I'm not sure that having separate setters for the access control type and the
location is the right model; since geolocation is such sensitive information,
we should perhaps set the access control and the location at the same time?

In particular, the first SetLocation on a Connection is specified to set the
access control to an empty whitelist if possible (though I don't know whether
Gabble actually allows this); if you want anyone to actually see your location,
you'll need to set an access control model.

There should also be a flag for "can set location", for XMPP connections that
don't have PEP (notably Google Talk). I'll include that in the bug.

I think it'd probably be fair to make telepathy-qt4 only support setting
Location (with high-level API at least) if the connection is new enough to have
the enhanced API from the bug I'm about to file.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.