Bug 11688

Created attachment 10835 [details] [review]
Patch for supporting jabber:iq:last

I added your patch in our review system for easier review and merge.
http://projects.collabora.co.uk/~monkey/telepathy-gabble-bug-11688/

Shouldn't we use time_t instead of gdouble ?


connection_iq_last_cb: please check lm_message_node_get_attribute (iq, "from") != NULL


+  g_free( seconds);  ->  g_free (seconds);

According latest coding style, when you write a function call on multi lines, args have to be 4 spaces alligned.

Except that, patch looks good to me.

I updated the last version of your patch:
http://monkey.collabora.co.uk/telepathy-gabble-bug-11688/

From a code correctness and style perspective, the branch at
<http://monkey.collabora.co.uk/telepathy-gabble_bug-11688/> looks fine.
However, I have other issues with it.

Section 8 — Security Considerations — of XEP-0012 says:

> A client MUST provide a way for a human user to disable sending of Last
> Activity responses from the client's full JID <localpart@domain.tld/resource>.

Gabble would not provide such a way, and so would be in violation of a MUST.
This could be rectified by adding a send-last-activity connection property,
which should default to False so that existing UIs don't start silently sending
more information about you than you told it to.

Section 4 —  Online User Query — pseudo-defines the idle time to be:

> the last time that the user interacted with the client application

which Gabble would not be sending.  Gabble would be sending the last time an
application interacted with Gabble, which is not even an upper bound on the last
time the user interacted with the client application: Empathy sets your presence
automatically when your screensaver kicks in, at which point the time since the
last activity would be erroneously reset to 0.

There's no way for a Telepathy UI to retrieve the last activity time of a
contact.  In any case, this XEP requires clients to *poll* for the idle time of
their contacts, which is ridiculous.  A better model would be to include the
last activity time (not seconds since last activity) in your presence.  Then, we
could define an interface on org.freedesktop.Telepathy.Connection — IdleTimes,
say — with something resembling the following API:

 » method SetIdle(uint64: last_activity_unix_time) → nothing
   - your presence applet would call this with the time of your last interaction
     — with it or with your entire desktop, per user preference — after you've
     been idle for n minutes (say, 5) or when your screensaver kicks in or
     something
 » method Back() → nothing
 » signal BecameIdle(uint32: handle, uint64: last_activity_unix_time)
   - raised when a contact becomes idle.
 » signal CameBack(uint32: handle)
   - raised when a contact came back.

I don't really see the use of implementing this XEP: its design is fundamentally
wrong.  I'm amazed that I can't find a XEP proposing to do idleness as a
presence attribute; we should write one.  :-)

Wow. I'm not proud of the comment above. The entire second half is completely wrong: the patch did indeed only update the last-activity time when the user actually sent a message, rather than on every stanza sent (as I had assumed at the time). And the tone is not exactly friendly. I apologize, and have updated the patch to apply to current master.

Created attachment 48478 [details] [review]
Support XEP-0012: Last Activity

Created attachment 48479 [details] [review]
MucChannel: fix coding style violations

I noticed these while updating the patch.

Review of attachment 48478 [details] [review]:

++

Review of attachment 48479 [details] [review]:

++

Merged to master for 0.13.4.