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 599650 - Ctrl-TAB should jump to the next tab with unread/highlighted message
Ctrl-TAB should jump to the next tab with unread/highlighted message
Status: RESOLVED OBSOLETE
Product: empathy
Classification: Core
Component: Chat
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: empathy-maint
empathy-maint
: 621402 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-10-26 12:43 UTC by Matěj Cepl
Modified: 2018-05-22 13:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Progress (3.05 KB, patch)
2010-03-20 04:10 UTC, Aaron Brown
reviewed Details | Review
Proposed patch (6.24 KB, patch)
2010-03-26 06:10 UTC, Aaron Brown
reviewed Details | Review

Description Matěj Cepl 2009-10-26 12:43:59 UTC
basically %subj% says it all (and yes, it should cycle, but that's another matter, which is apparently to flammable to be mentioned here).
Comment 1 Will Thompson 2009-10-26 12:46:40 UTC
(By "too flammable to be mentioned here", Matej means "already filed as bug #589263".)
Comment 2 Guillaume Desmottes 2009-10-26 15:43:49 UTC
Why Ctrl+TAB? Is there other applications using this shortcute for a similar use?
Comment 3 Matěj Cepl 2009-10-26 16:39:31 UTC
To be honest, I have just copied what I use in gajim. Well, I was also trying to keep Ctrl+[PgUp/PgDown] preserved for the original jumping from one tab to other. Also Firefox uses Ctrl+TAB for switching between the tabs. But of course, Empathy should be following The Holy Writ of HIG :)
Comment 4 Will Thompson 2009-10-26 21:22:38 UTC
(In reply to comment #2)
> Why Ctrl+TAB? Is there other applications using this shortcute for a similar
> use?

Pidgin uses Ctrl-Tab for this, and switches to the most active window as follows:

• If there's a window with an unread message that highlighted us, prefer it;
• If there's a window with an unread message, prefer it;
• If there's a window with activity (join/part, etc), prefer it;
• Else go to the next window.

This follows the algorithm used by Alt-A in irssi.
Comment 5 Sumana Harihareswara 2009-10-29 11:19:08 UTC
http://library.gnome.org/devel/hig-book/stable/input-keyboard.html.en#standard-shortcuts suggests (Table 10-13):

Ctrl+Tab, Shift+Ctrl+Tab: Moves keyboard focus out of enclosing widget to next/previous control, in those situations where Tab alone has another function (e.g. GtkTextView)
Comment 6 Guillaume Desmottes 2010-01-07 21:21:29 UTC
Thanks Sumana. This confirm that we should use another set of keys for this.
Comment 7 Will Thompson 2010-01-08 15:24:00 UTC
Perhaps:

Tabs -> Previous Tab with Activity   Ctrl-P
Tabs -> Next Tab with Activity       Ctrl-N
Comment 8 David Siegel 2010-01-11 20:36:11 UTC
Ctrl+Tab is a backup to handle inconsistent behavior of Tab? Weak! Oh well. Will's suggestion seems reasonable. I will remove the hundredpapercuts task because if we cannot implement a standard shortcut here, then the feature becomes esoteric.
Comment 9 Guillaume Desmottes 2010-01-12 11:03:00 UTC
Hey, using Ctrl+Tab to jump to a tab is *not* a standard shortcut.
Comment 10 Aaron Brown 2010-03-17 00:52:08 UTC
The status of this bug is still UNCONFIRMED?

Ctrl+N/P seems logical but the keys are a fair distance apart on the keyboard, how about  Ctrl+< for previous and Ctrl+> for next?  Is there documentation outlining current keyboard shortcuts?
Comment 11 Matěj Cepl 2010-03-17 07:34:02 UTC
(In reply to comment #10)
> The status of this bug is still UNCONFIRMED?
> 
> Ctrl+N/P seems logical but the keys are a fair distance apart on the keyboard,
> how about  Ctrl+< for previous and Ctrl+> for next?  Is there documentation
> outlining current keyboard shortcuts?

NOOOOOO!

Sorry, more politely, please NO!

< > keys are going to be death to i18n. You cannot imagine where these end in non-US keyboard layouts (e.g., Ctrl-< is <Left-Ctrl>+<Right-Alt>+<,> on cs_CZ keyboard and I have no idea, what it actually emits). Talking about keys being a distance apart.

I would prefer some more qualifiers to Ctrl-TAB (Ctrl-Alt-TAB, anyone?), but Ctrl-P/N or Ctrl-,. would do as well.
Comment 12 Aaron Brown 2010-03-18 09:04:39 UTC
Thats true, I hadn't thought about other layouts.

I'm working on a patch using Ctrl+Shift+PgUp/Dn but haven't been able to control the direction/wrap around as yet...
Comment 13 Aaron Brown 2010-03-20 04:10:56 UTC
Created attachment 156605 [details] [review]
Progress

Here's what I've got so far, there's no control over direction/wrap_around yet
Comment 14 Guillaume Desmottes 2010-03-22 09:49:14 UTC
Review of attachment 156605 [details] [review]:

Thanks a lot for your patch.
You should set better commit msg, "Progress on bug#599650" isn't really useful.

::: src/empathy-chat-window.c
@@ +1088,3 @@
+
+	priv	= GET_PRIV (window);
+	l	= priv->chats;

No need to align '='

@@ +1090,3 @@
+	l	= priv->chats;
+
+	for (; l != NULL; l = g_list_next (l)) {

init 'l' here.

@@ +1892,3 @@
 				       "menu_tabs_next", &priv->menu_tabs_next,
 				       "menu_tabs_prev", &priv->menu_tabs_prev,
+				       "menu_tabs_prev_unread", &priv->menu_tabs_prev_unread,

You forgot to commit the UI change introducing those.
Comment 15 Aaron Brown 2010-03-26 06:10:54 UTC
Created attachment 157142 [details] [review]
Proposed patch

My apologies, here is a revised patch.
Comment 16 Guillaume Desmottes 2010-03-26 13:22:27 UTC
Review of attachment 157142 [details] [review]:

Thanks for your patch.

You should include the number of the bug in your commit message to be credited in release notes. Adding "(#599650)" will do it.

::: src/empathy-chat-window.c
@@ +139,3 @@
 static void chat_window_update (EmpathyChatWindow *window);
 
+static guint get_all_unread_messages (EmpathyChatWindowPriv *priv);

You can easily avoid this declaration by defining the function earlier in the file.
I'd rename it get_nb_all_unread_messages.

@@ +351,3 @@
 	gtk_action_set_sensitive (priv->menu_tabs_next, TRUE);
 	gtk_action_set_sensitive (priv->menu_tabs_prev, TRUE);
+	gtk_action_set_sensitive (priv->menu_tabs_next_unread, unread_msgs);

should be unread_msgs != 0
I'm wondering if we shouldn't keep those sensitive and just fallback to the next tab if there is no unread message. See wjt's comment about how Pidgin implements this.

::: src/empathy-chat-window.ui
@@ +117,3 @@
+            <property name="label" translatable="yes">Previous Unread Message</property>
+          </object>
+          <accelerator key="Page_Up" modifiers="GDK_CONTROL_MASK|GDK_SHIFT_MASK"/>

Maybe Ctrl+P/N would be easier to type. Thoughts?
Comment 17 Will Thompson 2010-03-26 14:34:23 UTC
(In reply to comment #16)
> +static guint get_all_unread_messages (EmpathyChatWindowPriv *priv);
> 
> You can easily avoid this declaration by defining the function earlier in the
> file.
> I'd rename it get_nb_all_unread_messages.

count_unread_messages() ?

> ::: src/empathy-chat-window.ui
> @@ +117,3 @@
> +            <property name="label" translatable="yes">Previous Unread
> Message</property>
> +          </object>
> +          <accelerator key="Page_Up"
> modifiers="GDK_CONTROL_MASK|GDK_SHIFT_MASK"/>
> 
> Maybe Ctrl+P/N would be easier to type. Thoughts?

Ctrl-Shift-PgDown means "move the current tab one to the right" in at least gnome-terminal and Chromium.
Comment 18 Aaron Brown 2010-03-27 06:00:30 UTC
(In reply to comment #16)
> +static guint get_all_unread_messages (EmpathyChatWindowPriv *priv);
> 
> You can easily avoid this declaration by defining the function earlier in the
> file.
This function was already defined so Instead of moving it higher in the file I forward declared it, however if the correct way is to move the definition I'll change it and remember this for the future.

> I'd rename it get_nb_all_unread_messages.
Again, this wasn't my code.  Should I rename it to conform to coding style empathy/telepathy naming styles and change all calls, open another bug for this, or just leave it as is?

> @@ +351,3 @@
>      gtk_action_set_sensitive (priv->menu_tabs_next, TRUE);
>      gtk_action_set_sensitive (priv->menu_tabs_prev, TRUE);
> +    gtk_action_set_sensitive (priv->menu_tabs_next_unread, unread_msgs);
> 
> should be unread_msgs != 0
unread_msgs <= 0 will be interpretted as FALSE, unread_msgs >= 1 as TRUE, but I'll change this anyway.

> I'm wondering if we shouldn't keep those sensitive and just fallback to the
> next tab if there is no unread message. See wjt's comment about how Pidgin
> implements this.
Okay, missed the part about falling back on next/previous if there's no unread/highlighted messages, I'll implement the same sensitivity as previous/next tabs (which will be dependant on gtk-keynav-wrap-around after the gnome-2.30 code freeze)

> ::: src/empathy-chat-window.ui
> @@ +117,3 @@
> +            <property name="label" translatable="yes">Previous Unread
> Message</property>
> +          </object>
> +          <accelerator key="Page_Up"
> modifiers="GDK_CONTROL_MASK|GDK_SHIFT_MASK"/>
> 
> Maybe Ctrl+P/N would be easier to type. Thoughts?
If everyone is happy with Ctrl+P/N I'll change it to that.

(In reply to comment #17)
> Ctrl-Shift-PgDown means "move the current tab one to the right" in at least
> gnome-terminal and Chromium.
Shall I also implement this, or open another bug?
Comment 19 Will Thompson 2010-03-28 23:20:08 UTC
(In reply to comment #18)
> > Ctrl-Shift-PgDown means "move the current tab one to the right" in at least
> > gnome-terminal and Chromium.
> Shall I also implement this, or open another bug?

Oh, I was just using that as a counter-example for why those keys might not be ideal for "next unread", not necessarily saying "THIS MUST BE DONE" :)
Comment 20 Aaron Brown 2010-03-29 03:48:41 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > > Ctrl-Shift-PgDown means "move the current tab one to the right" in at least
> > > gnome-terminal and Chromium.
> > Shall I also implement this, or open another bug?
> 
> Oh, I was just using that as a counter-example for why those keys might not be
> ideal for "next unread", not necessarily saying "THIS MUST BE DONE" :)

I opened bug #614155 with a patch for implementing this. Two lines of code isn't really a big deal :)
Comment 21 Guillaume Desmottes 2010-06-16 12:20:19 UTC
*** Bug 621402 has been marked as a duplicate of this bug. ***
Comment 22 Jean-François Fortin Tam 2012-08-26 03:59:45 UTC
Hi Aaron (or empathy devs), please forward-port this patch or close the report as incomplete?
Comment 23 GNOME Infrastructure Team 2018-05-22 13:48:48 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/empathy/issues/134.