[Bug 20768] Support more ways to be invisible (XEP-0186, maybe XEP-0126, maybe whatever Google does)

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Apr 23 20:55:28 CEST 2010


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

Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review-
         AssignedTo|will.thompson at collabora.co. |eitan.isaacson at collabora.co
                   |uk                          |.uk

--- Comment #11 from Will Thompson <will.thompson at collabora.co.uk> 2010-04-23 11:55:27 PDT ---
_gabble_connection_create_invisible_privacy_list() and
_gabble_connection_invisible_privacy_list_set_active() would be a lot more
readable if they used wocky_stanza_build()! Ditto later functions that build
stanzas.

In both of those functions, you're sending the IQ to the server, and ignoring
the reply. (_gabble_connection_send() only returns FALSE if we're not
connected; any other errors are reported asynchronously.) Is that intentional?
I feel like we should probably not silently ignore setting yourself as
invisible failing asynchronously.

Also... we should probably listen for privacy list pushes to detect someone
deleting our invisible list. (I'm not really sure what we should do if that
happens... log a debug message? Try to re-create it?)

Does this branch do the right thing if I set myself to invisible before calling
Connect(), or do we get a brief flash of Available first? Ah, I see you have
tests for this case... I'd be more sure it worked if they actually tested that
we only send presence after turning on invisibility.

+      (conn->features & GABBLE_CONNECTION_FEATURES_PRESENCE_INVISIBLE ||
+          GABBLE_CONNECTION_FEATURES_PRIVACY == 0))

You mean |, not ||.

The second patch partially reverting the first makes it hard to review what's
actually changed. Could you squash them together? And then the third patch
moves all that code to a different file, and breaks some indentation:

-      if ((self->features & GABBLE_CONNECTION_FEATURES_PRIVACY) != 0)
-        lm_message_node_set_attribute (lm_message_get_node (message),
-            "type", "unavailable");
+            if ((self->features & GABBLE_CONNECTION_FEATURES_PRIVACY) != 0)
+       lm_message_node_set_attribute (lm_message_get_node (message),
+           "type", "unavailable");

Broadcasting presence type=unavailable to MUCs is a bad idea: that's how you
leave a MUC! <http://xmpp.org/extensions/xep-0045.html#exit> I think when
sending presence to a MUC, we should map invisible to Busy: you can't be
invisible in a MUC.

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



More information about the telepathy-bugs mailing list