GNOME Bugzilla – Bug 703487
Show typing icon against composing members of a MUC
Last modified: 2013-08-14 12:24:46 UTC
<glassrose> why can't I see the members of a MUC "composing" as they type? * glassrose asks herself <rishi> That might depend on whether the protocol supports it. <glassrose> No I tried a MUC with jabber accounts and couldn't produce the typing icons adjacent to the typing members of the MUC. Something has gone wrong since I implemented it. This feature was first introduced by me in commit 1acf899870 and was lost in commit 4f470e26bafd. Why not bring it back.
It is not easy to see how one would go for it. The IndividualStore deals with complete individuals while compositors are EmpathyContacts. Guillaume, can you explain how one would proceed to make it possible?
I didn't touch this code since a while, but you could fetch the TpContact from the individual and check the typing status from there using Telepathy API directly I think.
I have "contact-chat-state-changed" signal connected to in EmpathyIndividualStoreChannel to track changes in chat states and have update the status-icon pixbuf in the store accordingly. Patch follows.
Created attachment 250527 [details] [review] Show typing icon against composing members of a MUC
The above patch didn't chain up to the parent's constructed method. Revised patch follows.
Created attachment 250929 [details] [review] Show typing icon against composing members of a MUC
Review of attachment 250929 [details] [review]: ::: libempathy-gtk/empathy-individual-store-channel.c @@ +262,3 @@ +individual_store_channel_contact_chat_state_changed (TpTextChannel *channel, + TpContact *tp_contact, + if (typing_state == TP_CHANNEL_CHAT_STATE_COMPOSING) TpChannelChatState @@ +263,3 @@ + TpContact *tp_contact, + guint state, + if (typing_state == TP_CHANNEL_CHAT_STATE_COMPOSING) You can use 'EmpathyIndividualStoreChannel *self' directly. @@ +271,3 @@ + if (! (state == TP_CHANNEL_CHAT_STATE_COMPOSING || + state == TP_CHANNEL_CHAT_STATE_PAUSED || + GTK_ICON_SIZE_MENU); you can simplify this logic expression: if (! (a || b || c)) <=> if (!a && !b && !c) @@ +272,3 @@ + state == TP_CHANNEL_CHAT_STATE_PAUSED || + state == TP_CHANNEL_CHAT_STATE_ACTIVE)) + Actually shouldn't we always reset the pixbuf if the state is != COMPOSING? @@ +277,3 @@ + if (empathy_contact_is_user (contact)) + { + g_free (icon_filename); contact is not urefed if we don't enter in this block. @@ +287,3 @@ + self = EMPATHY_INDIVIDUAL_STORE_CHANNEL (user_data); + individual = g_hash_table_lookup (self->priv->individuals, tp_contact); + EMPATHY_INDIVIDUAL_STORE (model), individual); I'd use a g_warning() instead to be a bit safer from crashes. @@ +292,3 @@ + GUINT_TO_POINTER (state)); + + gtk_tree_store_set (GTK_TREE_STORE (model), iter, Couldn't you use individual_store_find_contact (make it public) and so save you from this foreach function and set_data()? @@ +308,3 @@ + chain_up (object); + +static void Use tp_g_signal_connect_object @@ +310,3 @@ + g_signal_connect (self->priv->channel, "contact-chat-state-changed", + G_CALLBACK (individual_store_channel_contact_chat_state_changed), +individual_store_channel_contact_chat_state_changed (TpTextChannel *channel, Can't we do that in individual_store_channel_set_individual_channel ?
(In reply to comment #7) > Review of attachment 250929 [details] [review]: > > ::: libempathy-gtk/empathy-individual-store-channel.c > @@ +262,3 @@ > +individual_store_channel_contact_chat_state_changed (TpTextChannel *channel, > + TpContact *tp_contact, > + if (typing_state == TP_CHANNEL_CHAT_STATE_COMPOSING) > > TpChannelChatState > > @@ +263,3 @@ > + TpContact *tp_contact, > + guint state, > + if (typing_state == TP_CHANNEL_CHAT_STATE_COMPOSING) > > You can use 'EmpathyIndividualStoreChannel *self' directly. > > @@ +271,3 @@ > + if (! (state == TP_CHANNEL_CHAT_STATE_COMPOSING || > + state == TP_CHANNEL_CHAT_STATE_PAUSED || > + GTK_ICON_SIZE_MENU); > > you can simplify this logic expression: > if (! (a || b || c)) <=> if (!a && !b && !c) > Done! > @@ +272,3 @@ > + state == TP_CHANNEL_CHAT_STATE_PAUSED || > + state == TP_CHANNEL_CHAT_STATE_ACTIVE)) > + > > Actually shouldn't we always reset the pixbuf if the state is != COMPOSING? > We don't want to cause any updation on GONE or INACTIVE events uselessly. We just bother about when a user stops typing (PAUSED) or enters text (ACTIVE). Still I have removed the filter. :) > @@ +277,3 @@ > + if (empathy_contact_is_user (contact)) > + { > + g_free (icon_filename); > > contact is not urefed if we don't enter in this block. > corrected! > @@ +287,3 @@ > + self = EMPATHY_INDIVIDUAL_STORE_CHANNEL (user_data); > + individual = g_hash_table_lookup (self->priv->individuals, tp_contact); > + EMPATHY_INDIVIDUAL_STORE (model), individual); > > I'd use a g_warning() instead to be a bit safer from crashes. > Done. > @@ +292,3 @@ > + GUINT_TO_POINTER (state)); > + > + gtk_tree_store_set (GTK_TREE_STORE (model), iter, > > Couldn't you use individual_store_find_contact (make it public) and so save you > from this foreach function and set_data()? > Done. > @@ +308,3 @@ > + chain_up (object); > + > +static void > > Use tp_g_signal_connect_object > > @@ +310,3 @@ > + g_signal_connect (self->priv->channel, "contact-chat-state-changed", > + G_CALLBACK (individual_store_channel_contact_chat_state_changed), > +individual_store_channel_contact_chat_state_changed (TpTextChannel *channel, > > Can't we do that in individual_store_channel_set_individual_channel ? Done! Patches follow.
Created attachment 251577 [details] [review] Make individual_store_find_contact and free_iters public
Created attachment 251578 [details] [review] Show typing icon against composing members of a MUC
Review of attachment 251577 [details] [review]: Feel free to merge right away with these changes. ::: libempathy-gtk/empathy-individual-store.c @@ +200,3 @@ } +GList * Please add a comment: /* (transfer full) free with empathy_individual_store_free_iters() */ ::: libempathy-gtk/empathy-individual-store.h @@ -144,2 +144,3 @@ FolksIndividual *individual); +GList *empathy_individual_store_find_contact (EmpathyIndividualStore *self, Please put those below the '/* protected */' commment, only sub-classes should use those functions.
Review of attachment 251578 [details] [review]: ::: libempathy-gtk/empathy-individual-store-channel.c @@ +159,3 @@ + for (l = iters; l != NULL; l = l->next) + { + } you can move the if/else block outside of the for loop so it will be done just once. The store will keep a ref each time we call _set() so it's fine re-using the same pixbuf more than once I think (but please test).
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.