GNOME Bugzilla – Bug 680449
Add UOA app plugin
Last modified: 2012-07-30 07:00:08 UTC
Such plugin will allow us to change personnal settings from the UOA UI.
Created attachment 219817 [details] [review] EmpathyContactWidget: Stop using EmpathyAvatarChooser Changing avatar is only used for editing user's information, and it will soon be replaced by a dedicated widget. This is done to make easier to change EmpathyAvatarChooser's API before introducing the new widget.
Created attachment 219818 [details] [review] EmpathyAvatarChooser: rebase on TpAccount API It can now edit the avatar of a TpAccount passed at construct time. The image is taken from the TpAccount directly instead of asking the user to set one. This is much more self contained code.
Created attachment 219819 [details] [review] EmpathyCalendarButton: Make it work without gtk_widget_show_all
Created attachment 219820 [details] [review] EmpathyUserInfo: New widget to edit personal information This is a much simplified version of EmpathyContactWidget
Created attachment 219821 [details] [review] EmpathAccountsDialog: user the new EmpathyUserInfo widget
Created attachment 219822 [details] [review] UOA: fix empathy.application XML indentation
Created attachment 219823 [details] [review] UOA accounts plugin widget: tweak styling of the info bar
Created attachment 219824 [details] [review] add empathy-app-plugin
Created attachment 219825 [details] [review] add empathy-app-plugin-widget
Created attachment 219826 [details] [review] UOA cc plugin: Support editing existing accounts
Created attachment 219829 [details] [review] EmpathyContactWidget: remove unused flags and dead code This widget is used only to add a new contact, all its flexibility isn't needed anymore.
Created attachment 219830 [details] [review] EmpathyContactWidget: remove all remaining flags It can now be used only in one scenario: add a new contact dialog
Created attachment 219831 [details] [review] EmpathyUserInfo: New widget to edit personal information This is a much simplified version of EmpathyContactWidget
Review of attachment 219817 [details] [review]: ++
Review of attachment 219818 [details] [review]: ::: libempathy-gtk/empathy-avatar-chooser.c @@ +132,3 @@ + if (self->priv->account == NULL) + { + GAsyncResult *result, How can this happen? We are reffing self while the call is running. @@ +174,3 @@ + + tp_account_get_avatar_async (self->priv->account, + } TpWeakRef maybe? @@ +185,3 @@ + get_avatar_cb, g_object_ref (self)); + + } Open a tp-glib bug and link it in this comment. @@ +190,3 @@ + NULL); + + pixbuf = empathy_pixbuf_from_data_and_mime ((gchar *) avatar->data, constructed is supposed to be chain-up first thing in the function.
Review of attachment 219819 [details] [review]: ++
Review of attachment 219821 [details] [review]: ++
Review of attachment 219822 [details] [review]: ++
Review of attachment 219823 [details] [review]: ++
Review of attachment 219824 [details] [review]: ++
Review of attachment 219825 [details] [review]: ++ ::: ubuntu-online-accounts/cc-plugins/app-plugin/empathy-app-plugin-widget.c @@ +50,3 @@ +static guint signals[LAST_SIGNAL]; + + double \n @@ +184,3 @@ + GError *error = NULL; + + if (!tp_account_manager_prepare_all_finish (manager, result, &error)) prepare_all? Is that new API? @@ +215,3 @@ + accounts = g_list_delete_link (accounts, accounts); + } + g_list_free (accounts); do we still need this as you are destroying the list while iterating over it? @@ +259,3 @@ + TP_CONTACT_FEATURE_PRESENCE, + TP_CONTACT_FEATURE_LOCATION, + TP_CONTACT_FEATURE_INVALID, Do we really need Presence and Location?
Review of attachment 219826 [details] [review]: ::: ubuntu-online-accounts/cc-plugins/empathy-accounts-plugin-widget.c @@ +315,3 @@ + accounts = g_list_delete_link (accounts, accounts); + } + { need to free?
Review of attachment 219829 [details] [review]: ++
Review of attachment 219830 [details] [review]: ::: libempathy-gtk/empathy-individual-dialogs.c @@ +135,3 @@ contact = empathy_contact_dup_from_folks_individual (individual); + contact_widget = empathy_contact_widget_new (contact); can we still set the alias when adding a new contact?
Review of attachment 219831 [details] [review]: No need to add it to po/POTFILES.in?
(In reply to comment #15) > Review of attachment 219818 [details] [review]: > > ::: libempathy-gtk/empathy-avatar-chooser.c > @@ +132,3 @@ > + if (self->priv->account == NULL) > + { > + GAsyncResult *result, > > How can this happen? We are reffing self while the call is running. This happens if the callback arrives after the widget as been destroyed. gtk_widget_destroy() calls dispose on it even if we are still holding a ref. Of course it won't be finalized until we unref, so it's still safe to dereference self->priv. > @@ +174,3 @@ > + > + tp_account_get_avatar_async (self->priv->account, > + } > > TpWeakRef maybe? Good point, did that. It also makes the above stull more obvious :-) > @@ +185,3 @@ > + get_avatar_cb, g_object_ref (self)); > + > + } > > Open a tp-glib bug and link it in this comment. done: https://bugs.freedesktop.org/show_bug.cgi?id=52938 > @@ +190,3 @@ > + NULL); > + > + pixbuf = empathy_pixbuf_from_data_and_mime ((gchar *) avatar->data, > > constructed is supposed to be chain-up first thing in the function. fixed.
(In reply to comment #22) > Review of attachment 219825 [details] [review]: > > ++ > > ::: ubuntu-online-accounts/cc-plugins/app-plugin/empathy-app-plugin-widget.c > @@ +50,3 @@ > +static guint signals[LAST_SIGNAL]; > + > + > > double \n Fixed > @@ +184,3 @@ > + GError *error = NULL; > + > + if (!tp_account_manager_prepare_all_finish (manager, result, &error)) > > prepare_all? Is that new API? See https://bugs.freedesktop.org/show_bug.cgi?id=52531, and please review it as well :D > @@ +215,3 @@ > + accounts = g_list_delete_link (accounts, accounts); > + } > + g_list_free (accounts); > > do we still need this as you are destroying the list while iterating over it? Yes, if it hit the beak it still need to free the rest of the list. > @@ +259,3 @@ > + TP_CONTACT_FEATURE_PRESENCE, > + TP_CONTACT_FEATURE_LOCATION, > + TP_CONTACT_FEATURE_INVALID, > > Do we really need Presence and Location? no, removed.
(In reply to comment #23) > Review of attachment 219826 [details] [review]: > > ::: ubuntu-online-accounts/cc-plugins/empathy-accounts-plugin-widget.c > @@ +315,3 @@ > + accounts = g_list_delete_link (accounts, accounts); > + } > + { > > need to free? Same as above: in case of break.
(In reply to comment #25) > Review of attachment 219830 [details] [review]: > > ::: libempathy-gtk/empathy-individual-dialogs.c > @@ +135,3 @@ > contact = empathy_contact_dup_from_folks_individual (individual); > > + contact_widget = empathy_contact_widget_new (contact); > > can we still set the alias when adding a new contact? Sure, why not? I removed the flag because EmpathyContactWidget now always make it editable.
(In reply to comment #26) > Review of attachment 219831 [details] [review]: > > No need to add it to po/POTFILES.in? Good point, empathy-app-plugin-widget.c and empathy-accounts-plugin-widget.c as well. btw how does it know where to find empathy's translations when the code is running as a plugin for another app?
(In reply to comment #31) > (In reply to comment #26) > > Review of attachment 219831 [details] [review] [details]: > > > > No need to add it to po/POTFILES.in? > > Good point, empathy-app-plugin-widget.c and empathy-accounts-plugin-widget.c as > well. Actually only empathy-app-plugin-widget.c was missing. > btw how does it know where to find empathy's translations when the code is > running as a plugin for another app? Replying to myself it seems we just have to include gi18n-lib.h, but it seems some of libempathy/ and libempathy-gtk/ still includes gi18n.h, I'll fix. I've merged all the commits with the fixes, thanks!