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 547658 - Add client type support
Add client type support
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Contact List
unspecified
Other All
: Normal enhancement
: ---
Assigned To: Emilio Pozuelo Monfort
: 592287 592764 593350 600991 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-08-13 19:30 UTC by Brian
Modified: 2011-08-29 10:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
attaching diff for easiest reviewing (29.74 KB, patch)
2010-08-02 10:53 UTC, Guillaume Desmottes
reviewed Details | Review
bcurtiswx mockup (4.91 KB, image/png)
2010-08-02 18:47 UTC, Brian Curtis
  Details
all branch diff against master (23.51 KB, patch)
2010-11-17 13:43 UTC, Emilio Pozuelo Monfort
reviewed Details | Review

Description Brian 2008-08-13 19:30:52 UTC
A lot of my friends sign off, and on the normal clients and Pidgin, it shows them as "Mobile". It's helpful to know the difference because I don't wanna send an IM and have it text my friend at midnight if she's asleep, but if she's on, it's different. So please, show a Cellphone icon instead of just the regular Online symbol, or some other distinguishing mark.

Other information:
Comment 1 Guillaume Desmottes 2008-08-15 09:23:51 UTC
That's not something we can do without meta-contacts support (#460647). The CM is not enough to determine if a contact is a mobile phone or not (except if it's gnome-phone-manager's CM maybe). The right way to do that is with meta-contact where then we can know if the number is a mobile phone or not.
Comment 2 Brian 2008-08-15 17:25:48 UTC
...meta-contacts? How does that have anything to do with a status?
AIM doesn't give you their phone number, it just says they're mobile, and when you IM it automatically forwards it on to their phone. Same with Yahoo!.
Comment 3 Guillaume Desmottes 2008-08-15 17:51:01 UTC
Oh sorry, I misunderstood your bug then. Will ask to Haze dev what they think about this.
Comment 4 Will Thompson 2008-08-15 18:16:16 UTC
We need to extend the Telepathy specification to support this kind of status metadata – what kind of device the contact is on, their mood, what they are listening to, etc. – before Empathy can show it to you.  Right now, telepathy-haze cannot tell Empathy that your friend is online from their mobile even though libpurple knows.

I've filed a bug over at https://bugs.freedesktop.org/show_bug.cgi?id=17157 .
Comment 5 Guillaume Desmottes 2009-08-19 09:09:25 UTC
*** Bug 592287 has been marked as a duplicate of this bug. ***
Comment 6 Guillaume Desmottes 2009-08-24 13:24:13 UTC
*** Bug 592764 has been marked as a duplicate of this bug. ***
Comment 7 Pierre-Luc Beaudoin 2009-08-28 04:03:36 UTC
*** Bug 593350 has been marked as a duplicate of this bug. ***
Comment 8 Frederic Peters 2009-11-06 20:22:11 UTC
*** Bug 600991 has been marked as a duplicate of this bug. ***
Comment 9 DonB 2010-05-12 00:48:41 UTC
Has there been any progress on this? 

I have multiple friends without messaging plans with their cellular service who seem to have unknowingly opted into receiving mobile messages via aim so each time I message them I am costing them money. Currently I have stopped initiating chats with my friends as I have no idea if they are mobile or actually online/available.
Comment 10 kxra 2010-05-13 18:43:47 UTC
I just want to add that this shouldn't just cover mobile users

Pidgin supports icons for mobile users, blocked this users, users that can recieve SMS text msgs, users that have a musictracker plugin installed or is playing music on Windows and has 'Now Playing' enabled to show to their contact, users logged in using a web interface to chat, and users that are bots
http://developer.pidgin.im/wiki/Using%20Pidgin#Whatdotheiconsnexttomybuddymean
Comment 11 Jonny Lamb 2010-07-29 17:34:11 UTC
Check it out.

http://people.collabora.co.uk/~jonny/itsaphone.png

omomomgomgomgomgogmogmgomgomgomgomgogmomg

Here is my git branch. It's probably better to review the big diff as I put it all in the wrong place to start so reviewing diff by diff is a bit of a trap.

http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=shortlog;h=refs/heads/client-types

and here's my gabble branch:

http://git.collabora.co.uk/?p=user/jonny/telepathy-gabble.git;a=shortlog;h=refs/heads/client-types
Comment 13 Guillaume Desmottes 2010-08-02 10:53:40 UTC
Created attachment 166961 [details] [review]
attaching diff for easiest reviewing
Comment 14 Guillaume Desmottes 2010-08-02 12:13:23 UTC
Review of attachment 166961 [details] [review]:

I think putting the phone icon there is really confusing. It's next to the webcam button which is activatable while this one is not. I'm sure most users will think that's the button to use to make audio call.
But I'm not sure of the best place to put it instead? Maybe in the corner of the avatar or something?

Adding more code to EmpathyContact makes me cry, especially after we spend so much time moving its features to TpContact. Any chance to fast track the spec and implement it in TpContact instead?

::: .gitignore
@@ +16,3 @@
 *.loT
 .semanticache
+.\#*

emacs magic?

::: help/.gitignore
@@ +14,3 @@
 ru/*.xml
 sv/*.xml
+zh_CN/*.xml

This has been done in master I think.

::: libempathy/empathy-contact.c
@@ +496,3 @@
+  contact_connect_to_client_types_updated (contact);
+
+  array = g_array_new (FALSE, FALSE, sizeof (TpHandle));

g_array_sized_new() ?
Comment 15 Brian Curtis 2010-08-02 18:47:43 UTC
Created attachment 167001 [details]
bcurtiswx mockup

I think a better place for protocol icons and this phone icon would be next to the status message (not the icon but the text).  This mockup shows an example of the way I see best for placement of the icon.
Comment 16 Jonny Lamb 2010-08-02 21:50:43 UTC
(In reply to comment #14)
> I think putting the phone icon there is really confusing. It's next to the
> webcam button which is activatable while this one is not. I'm sure most users
> will think that's the button to use to make audio call.
> But I'm not sure of the best place to put it instead? Maybe in the corner of
> the avatar or something?

Corner of the avatar sounds like a terrible option, tbh. :-)

> Adding more code to EmpathyContact makes me cry, especially after we spend so
> much time moving its features to TpContact. Any chance to fast track the spec
> and implement it in TpContact instead?

I'll try, but it's unavoidable because of the way our draft interfaces work. We can put it straight into tp-glib when undrafted though. Just be patient. :-)

> emacs magic?

Yes,

http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=commitdiff;h=58372c

> This has been done in master I think.

Okay, well git will sort that for me when merged.

> g_array_sized_new() ?

Okay, changed.
Comment 17 Guillaume Desmottes 2010-08-03 08:07:05 UTC
(In reply to comment #16)
> (In reply to comment #14)
> > I think putting the phone icon there is really confusing. It's next to the
> > webcam button which is activatable while this one is not. I'm sure most users
> > will think that's the button to use to make audio call.
> > But I'm not sure of the best place to put it instead? Maybe in the corner of
> > the avatar or something?
> 
> Corner of the avatar sounds like a terrible option, tbh. :-)

What about Brian's suggestion to display it next to the status message?

> > This has been done in master I think.
> 
> Okay, well git will sort that for me when merged.

Just drop the commit, there is no point to create conflicts/merge if we can easily avoid them.
Comment 18 Jonny Lamb 2010-08-03 15:40:53 UTC
(In reply to comment #17)
> What about Brian's suggestion to display it next to the status message?

It looks fine. I don't know how I'd do that in GTK though. :-)

> Just drop the commit, there is no point to create conflicts/merge if we can
> easily avoid them.

Yes, that's what I meant. git will sort everything out for me.
Comment 19 Jonny Lamb 2010-08-05 21:22:30 UTC
So, like, it'd be great to get this merged before folks changes  absolutely everything and I have to keep rewriting my patch.

I understand that you're not happy about having additions to EmpathyContact, but surely you can see that there is no way around it. If you think that you can convince smcv to suddenly say "yeah why not let's undraft after a few days as a draft" then I applaud you. :-)

So, how about it? UI changes are cheap later on but there are loads of contact changes at the moment. Also, I'll buy you a beer or something.
Comment 20 Guillaume Desmottes 2010-08-06 07:47:41 UTC
Ok, what about this: you make the UI change (because the current one is terribly confusing) and once it's done we merge the branch.
Meanwhile I'll try to convince smcv to undraft :)
Comment 21 Jonny Lamb 2010-09-30 17:29:18 UTC
Right, so I've not done any UI changes, but here's a much nicer branch that uses the upcoming client-type features in TpContact, which will appear in 0.13.1.

The code is nicer, but I'm a bit crap on the UI front. :-)

http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=shortlog;h=refs/heads/client-types
Comment 22 Guillaume Desmottes 2010-10-01 08:32:58 UTC
How does this branch interact with bug #630188. Shouldn't you depend of it instead?

Branch looks good but I still think this UI is horribly confusing and we should display the phone logo next to the status message as suggested in the mockup.
Comment 23 Jonny Lamb 2010-10-01 15:00:57 UTC
Well, if you ignore the first commit to port the folks stuff to their new names, then although it won't work without my folks client types branch being merged, the code in no way depends on it, so this could easily be merged before empathy is ported to folks 0.3.

In fact... should we not be depending on folks to give our TpContacts the client type feature? It would make the empathy code more complex etc., but hmm. Thoughts?
Comment 24 Jonny Lamb 2010-10-06 08:25:46 UTC
FYI, Travis said it was fine to rely on the client types feature being there.

So, this is ready to merge except if you want to change the UI. Feel free, Guillaume!
Comment 25 Guillaume Desmottes 2010-10-11 13:26:55 UTC
I got this crash when testing your branch:


Lib-GObject-WARNING **: invalid cast from `FolksBackendsKfPersona' to `FolksPresence'
aborting...

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007fffefec7c88 in g_logv (log_domain=0x7ffff07f0190 "GLib-GObject", log_level=G_LOG_LEVEL_WARNING, format=0x7ffff07f18c8 "invalid cast from `%s' to `%s'", 
    args1=0x7fffffffc790) at gmessages.c:553
553			G_BREAKPOINT ();
(gdb) bt
  • #0 g_logv
    at gmessages.c line 553
  • #1 g_log
    at gmessages.c line 577
  • #2 g_type_check_instance_cast
    at gtype.c line 3982
  • #3 individual_get_client_types
    at empathy-individual-store.c line 167
  • #4 add_individual_to_store
    at empathy-individual-store.c line 197
  • #5 individual_store_add_individual
    at empathy-individual-store.c line 467
  • #6 individual_store_add_individual_and_connect
    at empathy-individual-store.c line 914
  • #7 individual_store_members_changed_cb
    at empathy-individual-store.c line 967
  • #8 _empathy_marshal_VOID__STRING_OBJECT_OBJECT_UINT
    at empathy-marshal.c line 443
  • #9 g_closure_invoke
    at gclosure.c line 766
  • #10 signal_emit_unlocked_R
    at gsignal.c line 3252
  • #11 g_signal_emit_valist
    at gsignal.c line 2983
  • #12 g_signal_emit
    at gsignal.c line 3040
  • #13 aggregator_individuals_changed_cb
    at empathy-individual-manager.c line 211
  • #14 g_cclosure_user_marshal_VOID__POINTER_POINTER_STRING_OBJECT_ENUM
    at individual-aggregator.c line 2566
  • #15 g_closure_invoke
    at gclosure.c line 766
  • #16 signal_emit_unlocked_R
    at gsignal.c line 3252
  • #17 g_signal_emit_valist
    at gsignal.c line 2983
  • #18 g_signal_emit_by_name
    at gsignal.c line 3077
  • #19 folks_individual_aggregator_personas_changed_cb
    at individual-aggregator.c line 1416
  • #20 _folks_individual_aggregator_personas_changed_cb_folks_persona_store_personas_changed
    at individual-aggregator.c line 814
  • #21 g_cclosure_user_marshal_VOID__POINTER_POINTER_STRING_OBJECT_ENUM
    at persona-store.c line 564
  • #22 g_closure_invoke
  • #23 signal_emit_unlocked_R
    at gsignal.c line 3252
  • #24 g_signal_emit_valist
    at gsignal.c line 2983
  • #25 g_signal_emit_by_name
    at gsignal.c line 3077
  • #26 tpf_persona_store_add_new_personas_from_contacts
    at tpf-persona-store.c line 2747
  • #27 tpf_persona_store_create_personas_from_channel_handles_async_co
    at tpf-persona-store.c line 2406
  • #28 tpf_persona_store_create_personas_from_channel_handles_async_ready
    at tpf-persona-store.c line 2305
  • #29 g_simple_async_result_complete
    at gsimpleasyncresult.c line 692
  • #30 get_contacts_by_handle_cb
    at tp-lowlevel.c line 161
  • #31 contacts_context_continue
    at contact.c line 1350
  • #32 contacts_get_avatar_data
    at contact.c line 2375
  • #33 contacts_context_continue
    at contact.c line 1375
  • #34 contacts_got_attributes
    at contact.c line 3108
  • #35 connection_got_contact_attributes
    at connection-handles.c line 731
  • #36 _tp_cli_connection_interface_contacts_invoke_callback_get_contact_attributes
    at _gen/tp-cli-connection-body.h line 11031
  • #37 tp_proxy_pending_call_idle_invoke
    at proxy-methods.c line 153
  • #38 g_idle_dispatch
    at gmain.c line 4254
  • #39 g_main_dispatch
    at gmain.c line 2149
  • #40 g_main_context_dispatch
    at gmain.c line 2702
  • #41 g_main_context_iterate
    at gmain.c line 2780
  • #42 g_main_loop_run
    at gmain.c line 2988
  • #43 gtk_main
    at gtkmain.c line 1320
  • #44 main
    at empathy.c line 550

Comment 26 Jonny Lamb 2010-10-18 12:17:28 UTC
(In reply to comment #25)
> I got this crash when testing your branch:
> 
> Lib-GObject-WARNING **: invalid cast from `FolksBackendsKfPersona' to
> `FolksPresence'

Ah yeah sorry about that. I updated my branch to contain a fix for it but haven't been able to test it due to GTK+3 rubbishness. Anyway, that should sort it. Give it a go!
Comment 27 Guillaume Desmottes 2010-10-18 13:18:21 UTC
I have this crash now :(


Program received signal SIGSEGV, Segmentation fault.
0x00007ffff1da88aa in folks_presence_get_presence_type (self=0x9c7530) at presence.c:186
186		return FOLKS_PRESENCE_GET_INTERFACE (self)->get_presence_type (self);
(gdb) bt
  • #0 folks_presence_get_presence_type
    at presence.c line 186
  • #1 update_weak_contact
    at empathy-individual-widget.c line 227
  • #2 client_types_update
    at empathy-individual-widget.c line 827
  • #3 empathy_individual_widget_set_individual
    at empathy-individual-widget.c line 2152
  • #4 individual_view_query_tooltip_cb
    at empathy-individual-view.c line 221
  • #5 _gtk_marshal_BOOLEAN__INT_INT_BOOLEAN_OBJECT
    at gtkmarshalers.c line 836
  • #6 g_closure_invoke
    at gclosure.c line 766
  • #7 signal_emit_unlocked_R
    at gsignal.c line 3252
  • #8 g_signal_emit_valist
    at gsignal.c line 2993
  • #9 g_signal_emit_by_name
    at gsignal.c line 3077
  • #10 gtk_tooltip_run_requery
    at gtktooltip.c line 921
  • #11 _gtk_tooltip_handle_event
    at gtktooltip.c line 1579
  • #12 gtk_main_do_event
    at gtkmain.c line 1798
  • #13 gdk_event_source_dispatch
    at gdkeventsource.c line 303
  • #14 g_main_dispatch
    at gmain.c line 2149
  • #15 g_main_context_dispatch
    at gmain.c line 2702
  • #16 g_main_context_iterate
    at gmain.c line 2780
  • #17 g_main_loop_run
    at gmain.c line 2988
  • #18 gtk_main
    at gtkmain.c line 1321
  • #19 gtk_application_default_run
    at gtkapplication.c line 126
  • #20 g_application_run
    at gapplication.c line 886
  • #21 gtk_application_run
    at gtkapplication.c line 491
  • #22 main
    at empathy.c line 699

Comment 28 Emilio Pozuelo Monfort 2010-11-16 11:51:51 UTC
I've rebased Jonny's branch on top of master and fixed that crash.

The UI changes are more complicated. It's not possible to do it with the GTK+ API, so they would need some gdk/cairo hacks to get everything into a GtkCellRenderer or a GdkPixbuf, or so it seems to me.

Other alternatives would be to put the icon between the status icon and the name and status, but that wouldn't probably look quite pretty, or to put it on the bottom right corner of the status icon, but then it may be difficult to see or to know what it is.

Thoughts?
Comment 29 Emilio Pozuelo Monfort 2010-11-16 11:55:45 UTC
There's also the question of where to display it in compact mode. If we put it in the bottom right corner in the status icon there will be no problem (except that it will be even smaller), otherwise we could put it between the status icon and the contact name.
Comment 30 Emilio Pozuelo Monfort 2010-11-17 13:43:39 UTC
Created attachment 174680 [details] [review]
all branch diff against master

Here's my branch: http://git.collabora.co.uk/?p=user/pochu/empathy.git;a=commitdiff;h=refs/heads/bug-547658

And here's the patch diffing my branch against master.
Comment 31 Guillaume Desmottes 2010-11-17 14:19:07 UTC
Review of attachment 174680 [details] [review]:

Look's all good to me. Please remove the FIXME and feel free to merge to master.

::: libempathy-gtk/empathy-cell-renderer-text.c
@@ +359,3 @@
 
+		if (!priv->is_group && priv->types != NULL && g_strv_length (priv->types) > 0
+		    // FIXME: why don't we check the whole array?

Because atm Empathy doesn't manage ressources so we display the "best" one.
Comment 32 Emilio Pozuelo Monfort 2010-11-17 14:30:49 UTC
Thanks, comment removed and branch merged.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.