After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 606947 - support Contact Search API
support Contact Search API
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: General
2.29.x
Other Linux
: Normal enhancement
: ---
Assigned To: empathy-maint
empathy-maint
: 607858 (view as bug list)
Depends on:
Blocks: 640628
 
 
Reported: 2010-01-14 11:58 UTC by Danielle Madeley
Modified: 2011-08-29 10:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposal 1 (448.85 KB, image/jpeg)
2010-11-29 12:03 UTC, Emilio Pozuelo Monfort
  Details
proposal 2 (227.26 KB, image/jpeg)
2010-11-29 12:05 UTC, Emilio Pozuelo Monfort
  Details
diff (49.76 KB, patch)
2011-01-18 08:32 UTC, Guillaume Desmottes
reviewed Details | Review

Comment 1 Jonny Lamb 2010-01-23 13:16:33 UTC
*** Bug 607858 has been marked as a duplicate of this bug. ***
Comment 2 Danielle Madeley 2010-06-04 07:14:10 UTC
Worth recording my WIP branch: http://git.collabora.co.uk/?p=user/danni/empathy.git;a=shortlog;h=refs/heads/contact-search

This is stalled because Collabora's server never seems to return search results.
Comment 3 Danielle Madeley 2010-11-24 22:12:18 UTC
It's worth noting that this has been undrafted now:

http://telepathy.freedesktop.org/spec/Channel_Type_Contact_Search.html
Comment 4 Guillaume Desmottes 2010-11-25 09:22:57 UTC
Ideally we should have a TpChannel subclass in tp-glib and rely on it.
Comment 5 Danielle Madeley 2010-11-25 10:43:47 UTC
This branch includes an EmpathyTpContactSearch, that I had thought to move into tp-glib once it seemed sensible enough.

The ContactSearch API is actually not that fun to implement on the client side. I don't think it should be channel-based (even though it uses a channel internally), that's not how clients want to write a search dialog at all. They want something they can make queries too, and get results back, and not care about how many channels it creates in the meantime.
Comment 6 Emilio Pozuelo Monfort 2010-11-29 12:00:25 UTC
I'm working on this bug on top of Danielle's branch. I'm thinking about how the UI should be, and I've designed a couple of quick'n'dirty mockups. Please note that UX is not my strongest point, i.e. they probably suck :)
Comment 7 Emilio Pozuelo Monfort 2010-11-29 12:03:08 UTC
Created attachment 175456 [details]
proposal 1

This mockup has the search fields and the results in the same window. Clicking on Search updates the search results (i.e. performs a new search with the current fields). I'm not sure about how to add a contact, be it a button on the bottom or on each row. There could also be a button next to the Search one to clean all the fields.
Comment 8 Emilio Pozuelo Monfort 2010-11-29 12:05:13 UTC
Created attachment 175457 [details]
proposal 2

This second design only has the search fields in the window. When you hit Search, the results are displayed as in the image below. When you hit New Search there, the search fields are shown again.

Comments?
Comment 9 Guillaume Desmottes 2010-11-29 13:00:06 UTC
I think I prefer the first one as I'm not a big fan of windows explosion. On the hand, maybe that could be annoying on small screens (netbook) especially if there are a lot of search fields.

Keep in mind that adding to the roster is not the only think you could want to do with the contact. For example you may want to send him a message, a file or call him (if the protocol allows you to perform such operation on contacts which are not in your roster).
Comment 10 Emilio Pozuelo Monfort 2010-11-29 13:54:41 UTC
(In reply to comment #9)
> I think I prefer the first one as I'm not a big fan of windows explosion. On
> the hand, maybe that could be annoying on small screens (netbook) especially if
> there are a lot of search fields.

Yeah, so... which one? :) The first one can grow too much if we start adding fields indeed.

> Keep in mind that adding to the roster is not the only think you could want to
> do with the contact. For example you may want to send him a message, a file or
> call him (if the protocol allows you to perform such operation on contacts
> which are not in your roster).

OK, I'll try to think about some pretty UI for less common actions (like a drop down menu or something)
Comment 11 Danielle Madeley 2010-11-29 21:52:54 UTC
I opt for the first one. If there are lots of fields, the less interesting fields could be hidden behind a GtkExpander.
Comment 12 Emilio Pozuelo Monfort 2010-12-01 16:35:15 UTC
I have a working implementation, but the UI is not quite pretty. I've created a wiki page and I'll ask some UX designers to look at it: http://live.gnome.org/Empathy/ContactSearch
Comment 13 Emilio Pozuelo Monfort 2010-12-02 16:30:06 UTC
I've just reported https://bugs.freedesktop.org/show_bug.cgi?id=32053 to get the TpContactSearch proxy object in tp-glib.
Comment 14 Emilio Pozuelo Monfort 2011-01-17 15:32:22 UTC
I'm kinda happy with http://git.collabora.co.uk/?p=user/pochu/empathy.git;a=shortlog;h=refs/heads/contact-search-2-606947

The UI can probably be tweaked a bit to look better.

As Sjoerd pointed out on IRC, we want to merge this and the "Add Contact" dialog. For that, we'd need this dialog to have an entry where you can type in the contact id directly.
Comment 15 Guillaume Desmottes 2011-01-18 08:32:43 UTC
Created attachment 178591 [details] [review]
diff
Comment 16 Guillaume Desmottes 2011-01-18 08:59:27 UTC
Review of attachment 178591 [details] [review]:

The "Add" button should be insensitive while there is no contact selected. If not, hitting it would raise:
** CRITICAL **: add_selected_contact: assertion `sel == TRUE' failed

You should add some padding. See http://library.gnome.org/devel/hig-book/stable/design-window.html

I think we should have a Close button as well.

The dialog should have a title.

I tried searching with gabble. The "Search" button stay unsensitive and nothing happens.
Here is the log:


(empathy:14555): empathy-DEBUG: builder_get_file_valist: Loading file /home/cassidy/gnome/empathy/libempathy-gtk/empathy-individual-widget.ui
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_poll_features: 0x1c05800: request 0x20bc960 prepared
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_prepare_request_finish: 0x20bc960
(empathy:14555): empathy-DEBUG: _account_chooser_changed: The server supports cs|limit|server: yes|no|yes
(empathy:14555): empathy-DEBUG: _account_chooser_changed: New account is /org/freedesktop/Telepathy/Account/gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0
(empathy:14555): tp-glib/channel-DEBUG: tp_contact_search_open_new_channel: Requesting new search channel
(empathy:14555): tp-glib/client-DEBUG: tp_base_client_register: request name org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n1
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_borrow_interface_by_id: 0x7f6844017cb0: org.freedesktop.Telepathy.ChannelDispatcher DBusGProxy is 0x1f40160
(empathy:14555): tp-glib/client-DEBUG: acr_request_cb: Got ChannelRequest: /com/nokia/MissionControl/requests/r1
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_borrow_interface_by_id: 0x7f6844017c20: org.freedesktop.Telepathy.ChannelRequest DBusGProxy is 0x1f3fce0
(empathy:14555): tp-glib/client-DEBUG: acr_request_cb: Calling ChannelRequest.Proceed()
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_dispose: 0x7f6844017cb0
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_invalidate: 0x7f6844017cb0: Proxy unreferenced
(empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_finalize: 0x7f6844017cb0
(empathy:14555): tp-glib/client-DEBUG: acr_channel_request_proceed_cb: Proceed succeeded; waiting for the channel to be handled


in Mission Control:


mcd-DEBUG: 18/01/2011 09:46:37.336869: _mcd_client_registry_found_name: Registering client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
mcd-DEBUG: 18/01/2011 09:46:37.337013: mcd_client_proxy_constructed: org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
mcd-DEBUG: 18/01/2011 09:46:37.337167: mcd_client_proxy_introspect: No .client file for org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2. Ask on D-Bus.
mcd-DEBUG: 18/01/2011 09:46:37.338057: dispatcher_channel_request_acl_start: start /org/freedesktop/Telepathy/Account/gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0.org.freedesktop.Telepathy.Connection.Interface.Requests.CreateChannel acl (0x9f6240)
mcd-DEBUG: 18/01/2011 09:46:37.338109: dispatcher_channel_request_acl_success: complete acl (0x9f6240)
mcd-DEBUG: 18/01/2011 09:46:37.338191: _mcd_request_init: 0xa2f380
mcd-DEBUG: 18/01/2011 09:46:37.338258: _mcd_request_new: 0xa2f380 (for 0xa4c7e0)
mcd-DEBUG: 18/01/2011 09:46:37.338301: _mcd_channel_set_status: 0xa5fde0, 1
mcd-DEBUG: 18/01/2011 09:46:37.338412: _mcd_dispatcher_add_request: Default handler org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 for request /com/nokia/MissionControl/requests/r2 doesn't want AddRequest
mcd-DEBUG: 18/01/2011 09:46:37.338458: dispatcher_channel_request_acl_cleanup: cleanup acl (0x9f6240)
mcd-DEBUG: 18/01/2011 09:46:37.343722: _mcd_client_proxy_add_interfaces: org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2: org.freedesktop.Telepathy.Client.Handler
mcd-DEBUG: 18/01/2011 09:46:37.343779: _mcd_client_proxy_get_interfaces_cb: Client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
mcd-DEBUG: 18/01/2011 09:46:37.343825: _mcd_client_proxy_get_interfaces_cb: org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 is a Handler
mcd-DEBUG: 18/01/2011 09:46:37.344829: ready_to_request_cb: Starting online request
mcd-DEBUG: 18/01/2011 09:46:37.344878: _mcd_account_online_request: connection status for gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0 is 0
mcd-DEBUG: 18/01/2011 09:46:37.344923: _mcd_account_online_request: gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0 is already connected
mcd-DEBUG: 18/01/2011 09:46:37.344965: online_request_cb: called
mcd-DEBUG: 18/01/2011 09:46:37.345011: _mcd_mission_set_parent: child = 0xa5fde0, parent = 0xa348c0
mcd-DEBUG: 18/01/2011 09:46:37.345194: _mcd_channel_set_status: 0xa5fde0, 2
mcd-DEBUG: 18/01/2011 09:46:37.346725: _mcd_client_proxy_handler_get_all_cb: org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 has 0 HandlerChannelFilter entries
mcd-DEBUG: 18/01/2011 09:46:37.346780: _mcd_client_proxy_handler_get_all_cb: org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 has BypassApproval=T
mcd-DEBUG: 18/01/2011 09:46:37.346832: _mcd_connection_update_client_caps: Sending client caps to connection
mcd-DEBUG: 18/01/2011 09:46:37.346940: mcd_client_registry_ready_cb: org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2

::: extensions/Channel_Type_Contact_Search.xml
@@ +19,3 @@
+Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.</p>
+  </tp:license>
+  <interface name="org.freedesktop.Telepathy.Channel.Type.ContactSearch">

This interface is stable, why do you need generating this as en extension? Can't you just use tp-glib?

::: libempathy-gtk/empathy-contact-search-dialog.c
@@ +61,3 @@
+  GtkWidget *find_button;
+  GtkWidget *search_entry;
+  //GtkWidget *server_entry;

No C++ comment please, just remove the line if we don't need it.

@@ +126,3 @@
+    EmpathyContactManager *manager = empathy_contact_manager_dup_singleton ();
+
+    if (error)

error != NULL

@@ +251,3 @@
+
+  priv->searcher = tp_contact_search_new_finish (result, &error);
+  if (error)

if (error != NULL)

@@ +356,3 @@
+  hbox = gtk_hbox_new (FALSE, 6);
+
+  label = gtk_label_new (_("Enter search criteria"));

Wouldn't "Search: " just be enough ? And I think I'd put it on the same line as the entry.
Comment 17 Guillaume Desmottes 2011-01-18 10:30:35 UTC
Oh, as a note, we shouldn't merge this branch until the tp-glib branch has been merged and released. Then we shouldn't forget to update configure.ac, the wiki and jhbuild.
Comment 18 Emilio Pozuelo Monfort 2011-01-19 14:56:01 UTC
(In reply to comment #16)
> Review of attachment 178591 [details] [review]:
> 
> The "Add" button should be insensitive while there is no contact selected. If
> not, hitting it would raise:
> ** CRITICAL **: add_selected_contact: assertion `sel == TRUE' failed

Right. Fixed.

> You should add some padding. See
> http://library.gnome.org/devel/hig-book/stable/design-window.html

I've tried to do this, but I'm not sure the results are so good :(

> I think we should have a Close button as well.

Done.

> The dialog should have a title.

Done.

> I tried searching with gabble. The "Search" button stay unsensitive and nothing
> happens.
> Here is the log:
> 
> 
> (empathy:14555): empathy-DEBUG: builder_get_file_valist: Loading file
> /home/cassidy/gnome/empathy/libempathy-gtk/empathy-individual-widget.ui
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_poll_features: 0x1c05800:
> request 0x20bc960 prepared
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_prepare_request_finish:
> 0x20bc960
> (empathy:14555): empathy-DEBUG: _account_chooser_changed: The server supports
> cs|limit|server: yes|no|yes
> (empathy:14555): empathy-DEBUG: _account_chooser_changed: New account is
> /org/freedesktop/Telepathy/Account/gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0
> (empathy:14555): tp-glib/channel-DEBUG: tp_contact_search_open_new_channel:
> Requesting new search channel
> (empathy:14555): tp-glib/client-DEBUG: tp_base_client_register: request name
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n1
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_borrow_interface_by_id:
> 0x7f6844017cb0: org.freedesktop.Telepathy.ChannelDispatcher DBusGProxy is
> 0x1f40160
> (empathy:14555): tp-glib/client-DEBUG: acr_request_cb: Got ChannelRequest:
> /com/nokia/MissionControl/requests/r1
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_borrow_interface_by_id:
> 0x7f6844017c20: org.freedesktop.Telepathy.ChannelRequest DBusGProxy is
> 0x1f3fce0
> (empathy:14555): tp-glib/client-DEBUG: acr_request_cb: Calling
> ChannelRequest.Proceed()
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_dispose: 0x7f6844017cb0
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_invalidate: 0x7f6844017cb0:
> Proxy unreferenced
> (empathy:14555): tp-glib/proxy-DEBUG: tp_proxy_finalize: 0x7f6844017cb0
> (empathy:14555): tp-glib/client-DEBUG: acr_channel_request_proceed_cb: Proceed
> succeeded; waiting for the channel to be handled
> 
> 
> in Mission Control:
> 
> 
> mcd-DEBUG: 18/01/2011 09:46:37.336869: _mcd_client_registry_found_name:
> Registering client
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
> mcd-DEBUG: 18/01/2011 09:46:37.337013: mcd_client_proxy_constructed:
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
> mcd-DEBUG: 18/01/2011 09:46:37.337167: mcd_client_proxy_introspect: No .client
> file for org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2.
> Ask on D-Bus.
> mcd-DEBUG: 18/01/2011 09:46:37.338057: dispatcher_channel_request_acl_start:
> start
> /org/freedesktop/Telepathy/Account/gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0.org.freedesktop.Telepathy.Connection.Interface.Requests.CreateChannel
> acl (0x9f6240)
> mcd-DEBUG: 18/01/2011 09:46:37.338109: dispatcher_channel_request_acl_success:
> complete acl (0x9f6240)
> mcd-DEBUG: 18/01/2011 09:46:37.338191: _mcd_request_init: 0xa2f380
> mcd-DEBUG: 18/01/2011 09:46:37.338258: _mcd_request_new: 0xa2f380 (for
> 0xa4c7e0)
> mcd-DEBUG: 18/01/2011 09:46:37.338301: _mcd_channel_set_status: 0xa5fde0, 1
> mcd-DEBUG: 18/01/2011 09:46:37.338412: _mcd_dispatcher_add_request: Default
> handler org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
> for request /com/nokia/MissionControl/requests/r2 doesn't want AddRequest
> mcd-DEBUG: 18/01/2011 09:46:37.338458: dispatcher_channel_request_acl_cleanup:
> cleanup acl (0x9f6240)
> mcd-DEBUG: 18/01/2011 09:46:37.343722: _mcd_client_proxy_add_interfaces:
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2:
> org.freedesktop.Telepathy.Client.Handler
> mcd-DEBUG: 18/01/2011 09:46:37.343779: _mcd_client_proxy_get_interfaces_cb:
> Client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2
> mcd-DEBUG: 18/01/2011 09:46:37.343825: _mcd_client_proxy_get_interfaces_cb:
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 is a
> Handler
> mcd-DEBUG: 18/01/2011 09:46:37.344829: ready_to_request_cb: Starting online
> request
> mcd-DEBUG: 18/01/2011 09:46:37.344878: _mcd_account_online_request: connection
> status for gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0 is 0
> mcd-DEBUG: 18/01/2011 09:46:37.344923: _mcd_account_online_request:
> gabble/jabber/cassidy_2dtest1_40jabber_2ebelnet_2ebe0 is already connected
> mcd-DEBUG: 18/01/2011 09:46:37.344965: online_request_cb: called
> mcd-DEBUG: 18/01/2011 09:46:37.345011: _mcd_mission_set_parent: child =
> 0xa5fde0, parent = 0xa348c0
> mcd-DEBUG: 18/01/2011 09:46:37.345194: _mcd_channel_set_status: 0xa5fde0, 2
> mcd-DEBUG: 18/01/2011 09:46:37.346725: _mcd_client_proxy_handler_get_all_cb:
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 has 0
> HandlerChannelFilter entries
> mcd-DEBUG: 18/01/2011 09:46:37.346780: _mcd_client_proxy_handler_get_all_cb:
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2 has
> BypassApproval=T
> mcd-DEBUG: 18/01/2011 09:46:37.346832: _mcd_connection_update_client_caps:
> Sending client caps to connection
> mcd-DEBUG: 18/01/2011 09:46:37.346940: mcd_client_registry_ready_cb:
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e255.n2

The gabble logs would be useful.

> ::: extensions/Channel_Type_Contact_Search.xml
> @@ +19,3 @@
> +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
> USA.</p>
> +  </tp:license>
> +  <interface name="org.freedesktop.Telepathy.Channel.Type.ContactSearch">
> 
> This interface is stable, why do you need generating this as en extension?
> Can't you just use tp-glib?

Right, done. Danni added it when it was still a draft and I just updated it, didn't know I could use it from tp-glib.

> ::: libempathy-gtk/empathy-contact-search-dialog.c
> @@ +61,3 @@
> +  GtkWidget *find_button;
> +  GtkWidget *search_entry;
> +  //GtkWidget *server_entry;
> 
> No C++ comment please, just remove the line if we don't need it.

I've changed the comments to C ones. That is something we probably want at some point, another text entry to specify the server, in case you want to search in a different one (for distributed networks such as XMPP). I've left it out for now though, since that's not required for the minimal support (search on your own server), but should be working if we remove the comments for that code.

> @@ +126,3 @@
> +    EmpathyContactManager *manager = empathy_contact_manager_dup_singleton ();
> +
> +    if (error)
> 
> error != NULL
> 
> @@ +251,3 @@
> +
> +  priv->searcher = tp_contact_search_new_finish (result, &error);
> +  if (error)
> 
> if (error != NULL)

All done.

> @@ +356,3 @@
> +  hbox = gtk_hbox_new (FALSE, 6);
> +
> +  label = gtk_label_new (_("Enter search criteria"));
> 
> Wouldn't "Search: " just be enough ? And I think I'd put it on the same line as
> the entry.

Done.

We probably should move the spinner to somewhere else. Not sure where though. Perhaps at the right of the find button, hidden when stopped. Suggestions?
Comment 19 Guillaume Desmottes 2011-01-20 08:26:14 UTC
Here is the gabble logs (0.11.4):

wocky-DEBUG: 20/01/2011 09:20:12.783911: stanza_iq_handler_new: wocky-porter.c:253: Failed to normalise stanza recipient ''
wocky-DEBUG: 20/01/2011 09:20:12.783997: _write_node_tree: Serializing tree:
* iq xmlns='jabber:client' type='get' to='' id='258123783897'
    * query xmlns='jabber:iq:search'
gabble/connection-DEBUG: 20/01/2011 09:20:12.784418: gabble_connection_update_capabilities: enter
gabble/media-channel-DEBUG: 20/01/2011 09:20:12.784478: gabble_media_factory_add_caps: Client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e276.n2 media capabilities:
gabble/connection-DEBUG: 20/01/2011 09:20:12.784529: gabble_connection_update_capabilities: client org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e276.n2 has no interesting capabilities
gabble/presence-DEBUG: 20/01/2011 09:20:12.784601: gabble_presence_set_capabilities: about to add caps to resource ceb47283 with serial 5
gabble/presence-DEBUG: 20/01/2011 09:20:12.784647: gabble_presence_set_capabilities: found resource ceb47283
gabble/presence-DEBUG: 20/01/2011 09:20:12.784693: gabble_presence_set_capabilities: new serial 5, old 4, clearing caps
gabble/presence-DEBUG: 20/01/2011 09:20:12.784740: gabble_presence_set_capabilities: updating caps for resource ceb47283
gabble/connection-DEBUG: 20/01/2011 09:20:12.784791: gabble_connection_refresh_capabilities: nothing to do
wocky-DEBUG: 20/01/2011 09:20:12.802275: _end_element_ns: Received stanza
* iq xmlns='jabber:client' from='cassidy-test1@jabber.belnet.be' to='cassidy-test1@jabber.belnet.be/ceb47283' type='error' id='258123783897'
    * query xmlns='jabber:iq:search'
    * error code='503' type='cancel'
        * service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'
wocky-DEBUG: 20/01/2011 09:20:12.802382: check_spoofing: wocky-porter.c:946: 'cassidy-test1@jabber.belnet.be' (normal: 'cassidy-test1@jabber.belnet.be') attempts to spoof an IQ reply from ''
wocky-DEBUG: 20/01/2011 09:20:12.802433: check_spoofing: wocky-porter.c:948: Our full JID is 'cassidy-test1@jabber.belnet.be/ceb47283' and our bare JID is 'cassidy-test1@jabber.belnet.be'
gabble/connection-DEBUG: 20/01/2011 09:20:12.802505: connection_iq_unknown_cb: got unknown iq:
* iq xmlns='jabber:client' from='cassidy-test1@jabber.belnet.be' to='cassidy-test1@jabber.belnet.be/ceb47283' type='error' id='258123783897'
    * query xmlns='jabber:iq:search'
    * error code='503' type='cancel'
        * service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'
Comment 20 Guillaume Desmottes 2011-01-20 08:50:35 UTC
(In reply to comment #18)
> > You should add some padding. See
> > http://library.gnome.org/devel/hig-book/stable/design-window.html
> 
> I've tried to do this, but I'm not sure the results are so good :(

The treeview doesn't seem to be aligned with the other widgets.

> > ::: extensions/Channel_Type_Contact_Search.xml
> > @@ +19,3 @@
> > +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
> > USA.</p>
> > +  </tp:license>
> > +  <interface name="org.freedesktop.Telepathy.Channel.Type.ContactSearch">
> > 
> > This interface is stable, why do you need generating this as en extension?
> > Can't you just use tp-glib?
> 
> Right, done. Danni added it when it was still a draft and I just updated it,
> didn't know I could use it from tp-glib.

That's one of the point of tp-glib, providing generated API for all our stable
API.
extensions/ is used only to make test implementation of WIP API.

> > ::: libempathy-gtk/empathy-contact-search-dialog.c
> > @@ +61,3 @@
> > +  GtkWidget *find_button;
> > +  GtkWidget *search_entry;
> > +  //GtkWidget *server_entry;
> > 
> > No C++ comment please, just remove the line if we don't need it.
> 
> I've changed the comments to C ones. That is something we probably want at some
> point, another text entry to specify the server, in case you want to search in
> a different one (for distributed networks such as XMPP). I've left it out for
> now though, since that's not required for the minimal support (search on your
> own server), but should be working if we remove the comments for that code.

I'm wondering if searching fails because my server (jabber.belnet.be) doesn't
support search. We should handle properly this case.

> We probably should move the spinner to somewhere else. Not sure where though.
> Perhaps at the right of the find button, hidden when stopped. Suggestions?

Yeah that would be better I think.
Comment 21 Guillaume Desmottes 2011-01-20 08:55:11 UTC
Humm I tested using my main user. The dialog claims that search is supported on my Salut and Facebook account which is obvioulsy false.

Searching doesn't work on my collabora account either.
Comment 22 Emilio Pozuelo Monfort 2011-01-20 10:56:32 UTC
(In reply to comment #19)
> Here is the gabble logs (0.11.4):
> 
> wocky-DEBUG: 20/01/2011 09:20:12.783911: stanza_iq_handler_new:
> wocky-porter.c:253: Failed to normalise stanza recipient ''
> wocky-DEBUG: 20/01/2011 09:20:12.783997: _write_node_tree: Serializing tree:
> * iq xmlns='jabber:client' type='get' to='' id='258123783897'
>     * query xmlns='jabber:iq:search'
> gabble/connection-DEBUG: 20/01/2011 09:20:12.784418:
> gabble_connection_update_capabilities: enter
> gabble/media-channel-DEBUG: 20/01/2011 09:20:12.784478:
> gabble_media_factory_add_caps: Client
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e276.n2 media
> capabilities:
> gabble/connection-DEBUG: 20/01/2011 09:20:12.784529:
> gabble_connection_update_capabilities: client
> org.freedesktop.Telepathy.Client.TpGLibRequestAndHandle._3a1_2e276.n2 has no
> interesting capabilities
> gabble/presence-DEBUG: 20/01/2011 09:20:12.784601:
> gabble_presence_set_capabilities: about to add caps to resource ceb47283 with
> serial 5
> gabble/presence-DEBUG: 20/01/2011 09:20:12.784647:
> gabble_presence_set_capabilities: found resource ceb47283
> gabble/presence-DEBUG: 20/01/2011 09:20:12.784693:
> gabble_presence_set_capabilities: new serial 5, old 4, clearing caps
> gabble/presence-DEBUG: 20/01/2011 09:20:12.784740:
> gabble_presence_set_capabilities: updating caps for resource ceb47283
> gabble/connection-DEBUG: 20/01/2011 09:20:12.784791:
> gabble_connection_refresh_capabilities: nothing to do
> wocky-DEBUG: 20/01/2011 09:20:12.802275: _end_element_ns: Received stanza
> * iq xmlns='jabber:client' from='cassidy-test1@jabber.belnet.be'
> to='cassidy-test1@jabber.belnet.be/ceb47283' type='error' id='258123783897'
>     * query xmlns='jabber:iq:search'
>     * error code='503' type='cancel'
>         * service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'
> wocky-DEBUG: 20/01/2011 09:20:12.802382: check_spoofing: wocky-porter.c:946:
> 'cassidy-test1@jabber.belnet.be' (normal: 'cassidy-test1@jabber.belnet.be')
> attempts to spoof an IQ reply from ''
> wocky-DEBUG: 20/01/2011 09:20:12.802433: check_spoofing: wocky-porter.c:948:
> Our full JID is 'cassidy-test1@jabber.belnet.be/ceb47283' and our bare JID is
> 'cassidy-test1@jabber.belnet.be'
> gabble/connection-DEBUG: 20/01/2011 09:20:12.802505: connection_iq_unknown_cb:
> got unknown iq:
   ^^^^^^^^^^^^^

> * iq xmlns='jabber:client' from='cassidy-test1@jabber.belnet.be'
> to='cassidy-test1@jabber.belnet.be/ceb47283' type='error' id='258123783897'
>     * query xmlns='jabber:iq:search'
>     * error code='503' type='cancel'
>         * service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'

Unknown iq. I don't know why you're getting that. Do you get the same thing with you salut, facebook and collabora accounts? My collabora and gmail ones return the same error, but I don't get an unknown iq from gabble. I have gabble 0.11.5, but the NEWS doesn't seem relevant.
Comment 23 Emilio Pozuelo Monfort 2011-01-20 11:07:05 UTC
(In reply to comment #22)
> Unknown iq. I don't know why you're getting that. Do you get the same thing
> with you salut, facebook and collabora accounts? My collabora and gmail ones
> return the same error, but I don't get an unknown iq from gabble. I have gabble
> 0.11.5, but the NEWS doesn't seem relevant.

False alarm, I actually get that too. However it works fine for me, the logic to detect if the server supports contact search or not works for me and the search button is greyed out in those cases, including my collabora address.
Comment 24 Guillaume Desmottes 2011-01-20 11:17:25 UTC
Well, in that case we should filter out those accounts from the account chooser.

And I'm pretty sure Salut doesn't implement ContactSearch, so it should be filtered out as well.
Comment 25 Emilio Pozuelo Monfort 2011-01-20 11:30:27 UTC
(In reply to comment #24)
> Well, in that case we should filter out those accounts from the account
> chooser.

OK, will do.

> And I'm pretty sure Salut doesn't implement ContactSearch, so it should be
> filtered out as well.

Do you get a greyed out Find button for it? If so the logic already works fine, I'll just hide the account instead of greying the button.
Comment 26 Guillaume Desmottes 2011-01-20 11:45:31 UTC
You should pass a custom filter to empathy_account_chooser_set_filter() and grey out accounts not supporting search.
Comment 27 Emilio Pozuelo Monfort 2011-01-20 15:10:08 UTC
Accounts not supporting contact search greyed out.

Spinner moved between the search entry and the find button (it looks better to me there) and hidden when not spinning.

Set the server to NULL instead of "" when creating / resetting a TpContactSearch, since gabble can't cope with the latter. See https://bugs.freedesktop.org/show_bug.cgi?id=32390 (I didn't remember this).

Added padding to the treeview.

There is still a problem here, my gmail account advertises contact-search support, but the TpContactSearch object isn't created... presumably a gabble or a gmail bug.
Comment 28 Guillaume Desmottes 2011-01-21 09:49:15 UTC
I can now search on jabber.belnet.be \o/

I think it would be nice displaying "No contact found" or something when we don't get any result (like we do with live search). It's a bit confusing atm if the search is done really fast, you are not sure if it actually searched something or not.

Searching on the Collabora server still doesn't work though. I guess that's because it doesn't have a search server. Maybe tp_capabilities_supports_contact_search() should return us the default search server as well? So if there is none, we could grey out the account (as we don't have any UI to change it atm).
And I think it would be interesting to display it as default once we'll have a server text entry.


I wouldn't put empathy_account_chooser_filter_supports_contact_search() in the account-chooser file. Just let it in the dialog one.
Comment 29 Emilio Pozuelo Monfort 2011-01-21 11:11:41 UTC
(In reply to comment #28)
> I can now search on jabber.belnet.be \o/

Woo!

> I think it would be nice displaying "No contact found" or something when we
> don't get any result (like we do with live search). It's a bit confusing atm if
> the search is done really fast, you are not sure if it actually searched
> something or not.

Nice idea, done.

> Searching on the Collabora server still doesn't work though. I guess that's
> because it doesn't have a search server. Maybe
> tp_capabilities_supports_contact_search() should return us the default search
> server as well? So if there is none, we could grey out the account (as we don't
> have any UI to change it atm).
> And I think it would be interesting to display it as default once we'll have a
> server text entry.

Err, if there is no search server, it shouldn't be advertising contact search support at all, so the current implementation should be fine...

The problem here is that AFAIK the search server is a different one. I'm not sure how/if telepathy advertises this, but if it does, we should just set the server to that when creating the TpContactSearch.

> I wouldn't put empathy_account_chooser_filter_supports_contact_search() in the
> account-chooser file. Just let it in the dialog one.

Done.

This is looking quite good with all the UI improvements :-)
Comment 30 Guillaume Desmottes 2011-01-21 11:59:03 UTC
(In reply to comment #29)
> > Searching on the Collabora server still doesn't work though. I guess that's
> > because it doesn't have a search server. Maybe
> > tp_capabilities_supports_contact_search() should return us the default search
> > server as well? So if there is none, we could grey out the account (as we don't
> > have any UI to change it atm).
> > And I think it would be interesting to display it as default once we'll have a
> > server text entry.
> 
> Err, if there is no search server, it shouldn't be advertising contact search
> support at all, so the current implementation should be fine...
> 
> The problem here is that AFAIK the search server is a different one. I'm not
> sure how/if telepathy advertises this, but if it does, we should just set the
> server to that when creating the TpContactSearch.

Humm, we have to distinguish these cases:

- The CM doesn't support contact search at all, all the accounts associated with this CM should be greyed out.

- The CM supports contact search and the XMPP server of the connection has a default search server (like jabber.belnet.be). Your current UI can search on those; all good.

- The CM supports contact search but the XMPP server doesn't have a default search server. In that case we could use contact search but will have to specify another search server to use. With your current UI, we shouldn't claim to support those accounts and so they should greyed out as well. If we don't have D-Bus API to know the default search server of a Connection, maybe we should consider adding one.

- The CM supports contact search but there is no XMPP server AND server doesn't server-to-server connectivity, so we can't make any search. In that case, the Connection shouldn't claim supporting ContactSearch. The main example of this is of course Facebook. That's kinda related to https://bugs.freedesktop.org/show_bug.cgi?id=26823 which proposes to disable features not supported by FB.
Comment 31 Emilio Pozuelo Monfort 2011-01-21 15:44:11 UTC
(In reply to comment #30)
> - The CM supports contact search but the XMPP server doesn't have a default
> search server. In that case we could use contact search but will have to
> specify another search server to use. With your current UI, we shouldn't claim
> to support those accounts and so they should greyed out as well. If we don't
> have D-Bus API to know the default search server of a Connection, maybe we
> should consider adding one.

I can't find a way to know that... I can open a bug though. I guess we could push what we have and fix this afterwards, to meet the UI freeze.
Comment 32 Emilio Pozuelo Monfort 2011-01-21 15:45:48 UTC
s/UI freeze/feature freeze/
Comment 33 Guillaume Desmottes 2011-01-24 09:24:32 UTC
(In reply to comment #31)
> (In reply to comment #30)
> > - The CM supports contact search but the XMPP server doesn't have a default
> > search server. In that case we could use contact search but will have to
> > specify another search server to use. With your current UI, we shouldn't claim
> > to support those accounts and so they should greyed out as well. If we don't
> > have D-Bus API to know the default search server of a Connection, maybe we
> > should consider adding one.
> 
> I can't find a way to know that... I can open a bug though. I guess we could
> push what we have and fix this afterwards, to meet the UI freeze.

Please check with the author of the ContactSearch spec and open a bug if needed.
Yeah that couldn't block this branch but it would be good to think about it before merging the tp-glib API.
Comment 34 Guillaume Desmottes 2011-01-26 10:58:54 UTC
If you have some time, it would be nice to do bug #640628 once this has been merged.
Comment 35 Emilio Pozuelo Monfort 2011-01-26 18:31:27 UTC
TpContactSearch merged.

OK to go?
Comment 36 Guillaume Desmottes 2011-01-27 09:39:01 UTC
We should first have a tp-glib release, update http://live.gnome.org/TwoPointNinetyone/ExternalDependencies and the moduleset in jhbuild.

Also, did you find a solution for the "no default server" problem?
Comment 37 Emilio Pozuelo Monfort 2011-01-28 00:54:02 UTC
(In reply to comment #34)
> If you have some time, it would be nice to do bug #640628 once this has been
> merged.

Sounds cool! Not sure if I'll have time soon, and not sure if we can get this in after Feature Freeze... so if you want to cook a quick patch we can probably get it in :)

(In reply to comment #36)
> We should first have a tp-glib release, update
> http://live.gnome.org/TwoPointNinetyone/ExternalDependencies and the moduleset
> in jhbuild.

Jonny did the release, and I've updated the wiki and jhbuild.

Merged! \o/

> Also, did you find a solution for the "no default server" problem?

Not really. We can open a new bug for that.
Comment 38 Guillaume Desmottes 2011-01-28 09:15:24 UTC
(In reply to comment #37)
> > Also, did you find a solution for the "no default server" problem?
> 
> Not really. We can open a new bug for that.

Please do, I'd really like to solve that.
Comment 39 Emilio Pozuelo Monfort 2011-01-28 10:25:26 UTC
(In reply to comment #38)
> (In reply to comment #37)
> > > Also, did you find a solution for the "no default server" problem?
> > 
> > Not really. We can open a new bug for that.
> 
> Please do, I'd really like to solve that.

Opened bug 640806