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 601693 - Shouldn't cycle tab if gtk-keynav-wrap-around is 0
Shouldn't cycle tab if gtk-keynav-wrap-around is 0
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Chat
2.29.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
Depends on:
Blocks:
 
 
Reported: 2009-11-12 13:48 UTC by Guillaume Desmottes
Modified: 2010-03-17 09:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (2.36 KB, patch)
2010-03-16 10:32 UTC, Aaron Brown
reviewed Details | Review
corrected patch (1.89 KB, patch)
2010-03-16 22:34 UTC, Aaron Brown
committed Details | Review

Description Guillaume Desmottes 2009-11-12 13:48:47 UTC
As said on bug #589263, we shouldn't cycle tabs if this option is disabled.
Comment 1 Guillaume Desmottes 2009-11-12 13:49:32 UTC
This is a perfect bug for a new contributor.
Comment 2 Aaron Brown 2010-03-15 09:57:12 UTC
I'd love to give it a go, would you have any tips on where to start looking?

Were menu_tabs_[next, prev] along the right lines?
Comment 3 Guillaume Desmottes 2010-03-15 11:14:04 UTC
Hi Aaron,

Code is in src/empathy-chat-window.c in 2 functions: chat_window_tabs_next_activate_cb and chat_window_tabs_previous_activate_cb.

You should use GtkSettings API to check if the gtk-keynav-wrap-around is 0 and don't cycle in that case.
Comment 4 Aaron Brown 2010-03-16 10:32:16 UTC
Created attachment 156256 [details] [review]
Proposed patch

Patch of src/empathy-chat-window.c only (no ChangeLog entry, will repatch including one if requested)
Comment 5 Guillaume Desmottes 2010-03-16 11:28:17 UTC
Review of attachment 156256 [details] [review]:

Thanks a lot for your patch. Don't worry about ChangeLog entry, we don't use it anymore, it's automatically generated from the git history.

I inlined some minor style comments.

::: src/empathy-chat-window.c
@@ +1028,3 @@
 	EmpathyChat           *chat;
 	gint                  index_, numPages;
+	gboolean              *wrapAround;

We use this_style for variables.

@@ +1032,3 @@
 	priv = GET_PRIV (window);
 
+	g_object_get (gtk_settings_get_default (), "gtk-keynav-wrap-around", &wrapAround, NULL);

This line is longer than 80 chars, you should wrap it.

@@ +1053,3 @@
 	EmpathyChat           *chat;
 	gint                  index_, numPages;
+        gboolean              wrap_around;

this one is not tab-aligned

@@ +1057,3 @@
 	priv = GET_PRIV (window);
 
+	g_object_get (gtk_settings_get_default (), "gtk-keynav-wrap-around", &wrap_around, NULL);

same problem about the length.

@@ +1082,3 @@
 	priv = GET_PRIV (window);
 
+	g_object_get (gtk_settings_get_default (), "gtk-keynav-wrap-around", &wrap_around, NULL);

here too.
Comment 6 Aaron Brown 2010-03-16 22:34:28 UTC
Created attachment 156308 [details] [review]
corrected patch
Comment 7 Aaron Brown 2010-03-16 22:34:45 UTC
I'm glad you picked up on those mistakes, I was adamant I had changed all my wrapAround variables to wrap_around which highlighted the fact that I'd unnecessarily changed chat_window_tabs_left_activate_cb. I'll submit a corrected patch.
Comment 8 Guillaume Desmottes 2010-03-17 09:21:02 UTC
Merged, thanks a lot for your patch!