GNOME Bugzilla – Bug 626560
Unusual tabbing behaviour in Facebook account creation dialog
Last modified: 2010-11-22 05:54:13 UTC
In most account creation and login dialogs, one can enter a username, then press Tab and enter a password. For the Facebook account creation dialog, there is a label in between the username and password fields with a link, adding an extra item to tab through between the username and password fields. This is a little surprising. It may be worthwhile to change that label's tab index so people can interact with the form as usual.
Indeed. This could be done using gtk_container_set_focus_chain()
Created attachment 174465 [details] [review] link is unfocussed Patch attached. Please review.
Review of attachment 174465 [details] [review]: ::: libempathy-gtk/empathy-account-widget.c @@ +1426,3 @@ + + /* Removing the label form list of focusable widgets */ + parent = GTK_CONTAINER (gtk_widget_get_parent (label_example_fb)); You leak the returned list; you have to free it once you're done with it.
Created attachment 174638 [details] [review] Incremental patch Here is my next commit diff for this bug.
Review of attachment 174638 [details] [review]: ::: libempathy-gtk/empathy-account-widget.c @@ +1450,3 @@ children = g_list_remove (children, label_example_fb); gtk_container_set_focus_chain (parent, children); + g_object_unref (children); Did you test this code? A GList is not an object, you can't unref it. Use g_list_free().
Also, you didn't updated your remote branch.
oops! corrected. Also, I have 3 patches now in a series and I have pushed the branch here: http://gitorious.org/glassrose-gnome/empathy/commits/link_unfocussed_626560
Created attachment 174847 [details] [review] patch 1
Created attachment 174848 [details] [review] Patch 2
Created attachment 174849 [details] [review] Patch 3
Review of attachment 174847 [details] [review]: You should squash your commits together here, especially because the second one is wrong and unrequired. You can do this with `git rebase -i master` (interactive rebase). Furthermore, when attaching a series of patches, please make them one attachment, not 3. You can create a single file of patches using `git format-patch --stdout <branchpoint> > my.diff` ::: libempathy-gtk/empathy-account-widget.c @@ +1325,3 @@ GtkWidget *entry_id; + GtkContainer *parent; + GList *children; Please move these into the block in which they are used.
Fixed up on IRC, committed as http://git.gnome.org/browse/empathy/commit/?id=38f4477ad6d36bc000db7f7388266007b194c0dd