GNOME Bugzilla – Bug 636613
Become a server SASL handler
Last modified: 2010-12-07 11:56:58 UTC
All this branch does for now is handle SASL channels with mechanism X-TELEPATHY-PASSWORD (haze git master). Saving passwords is currently not implemented as it is blocked on a spec bug (yet to be filed). I'll fix up these things when the spec has these things fixed, but there's really no reason why this can't be merged now. http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=shortlog;h=refs/heads/sasl Give it a go! Have some fun!;
You can probably make the "make passwords optional" bug depend on this one.
Created attachment 175994 [details] [review] diff Attaching diff for review.
Review of attachment 175994 [details] [review]: The branch looks pretty. Code is clear and commits are nicely separated. Good work! I just have few general questions and spotted some coding style errors. Does the branch pass "make check"? Shouldn't EmpathyServerSASLHandler be a TpChannel subclass? I know it's not doable atm because we can't define our own features (vote for fdo #31583 !) but that's the idea right? Isn't http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=commitdiff;h=ee8f480f660963c7d26872a27ba9889bbed6a000 a bit overkill? Why not just use gtk_widget_grab_focus() like we always do? ::: libempathy/empathy-server-sasl-handler.c @@ +58,3 @@ + G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, async_initable_iface_init)); + +#define GET_PRIV(obj) EMPATHY_GET_PRIV (obj, EmpathyServerSASLHandler); I'm trying to get rid of this pattern in new code. Please use self->priv @@ +194,3 @@ + priv = GET_PRIV (object); + + tp_cli_channel_interface_sasl_authentication_connect_to_sasl_status_changed (priv->channel, line is too long @@ +407,3 @@ + + tp_cli_channel_interface_sasl_authentication_call_start_mechanism_with_data ( + priv->channel, -1, "X-TELEPATHY-PASSWORD", array, start_mechanism_with_data_cb, This line is too long.
(In reply to comment #3) > The branch looks pretty. Code is clear and commits are nicely separated. Good > work! Woo, thanks! > Does the branch pass "make check"? It does now. :-) > Shouldn't EmpathyServerSASLHandler be a TpChannel subclass? I know it's not > doable atm because we can't define our own features (vote for fdo #31583 !) but > that's the idea right? I'm not sure. The handler is quite a bit more than just a proxy to a DBus object but it completely handles the SASL mechanism as well as poking the password in and out of the keyring. Hmm. > Isn't > http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=commitdiff;h=ee8f480f660963c7d26872a27ba9889bbed6a000 > a bit overkill? Why not just use gtk_widget_grab_focus() like we always do? Oh this is quite different. This doesn't just grab the focus when it first appears (which is done by gtk_widget_grab_focus at the end of constructed) but it ensures that you can't type anything without it appearing in the password dialog. I really like this feature in seahorse as it means I don't end up typing my password/passphrase into an IRC window because I moved my mouse (focus follows mouse). Am I making sense? > I'm trying to get rid of this pattern in new code. Please use self->priv Oh awesome! I felt a little bad using it! Done. > line is too long Fixed. > This line is too long. Fixed. Okay I updated my branch with a few commits. It's in the same place. Thanks for your review!
(In reply to comment #4) > > Shouldn't EmpathyServerSASLHandler be a TpChannel subclass? I know it's not > > doable atm because we can't define our own features (vote for fdo #31583 !) but > > that's the idea right? > > I'm not sure. The handler is quite a bit more than just a proxy to a DBus > object but it completely handles the SASL mechanism as well as poking the > password in and out of the keyring. Hmm. Maybe yeah, anyway, that can wait. > > Isn't > > http://git.collabora.co.uk/?p=user/jonny/empathy.git;a=commitdiff;h=ee8f480f660963c7d26872a27ba9889bbed6a000 > > a bit overkill? Why not just use gtk_widget_grab_focus() like we always do? > > Oh this is quite different. This doesn't just grab the focus when it first > appears (which is done by gtk_widget_grab_focus at the end of constructed) but > it ensures that you can't type anything without it appearing in the password > dialog. I really like this feature in seahorse as it means I don't end up > typing my password/passphrase into an IRC window because I moved my mouse > (focus follows mouse). Am I making sense? I see, thanks for the explanation. Go go merge !
Woo, thanks!