582669 - Make separate keyboard access shortcuts for pinned tabs and regular tabs

Status: NEW

(Firefox :: Tabbed Browser, enhancement, P5)

Description

[imradyurrad]

User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100727 Minefield/4.0b3pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; rv:2.0b3pre) Gecko/20100727 Minefield/4.0b3pre

Ctrl+0 shortcut currently doesn't do anything. Ctrl+1 should switch to the first non-apptab tab 

Reproducible: Always

Comment 1

[Kevin Brosnan]

Ctrl + 0 is used to reset the zoom level.

Comment 2

[imradyurrad]

(In reply to comment #1)
> Ctrl + 0 is used to reset the zoom level.

Oops. Forgot about that. So how about this: If the zoom level is at default, ctrl+0 should behave as described

Comment 3

[Deniz Sarikaya]

(In reply to comment #2)
> (In reply to comment #1)
> > Ctrl + 0 is used to reset the zoom level.
> 
> Oops. Forgot about that. So how about this: If the zoom level is at default,
> ctrl+0 should behave as described

Bad idea. What if someone has zoomed in but wants to switch to the first App Tab without resetting the zoom level?

Comment 4

[imradyurrad]

(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Ctrl + 0 is used to reset the zoom level.
> > 
> > Oops. Forgot about that. So how about this: If the zoom level is at default,
> > ctrl+0 should behave as described
> 
> Bad idea. What if someone has zoomed in but wants to switch to the first App
> Tab without resetting the zoom level?


Ctrl+shift+0?

Comment 5

[imradyurrad]

At the least, apptabs should be excluded from ctrl + 1-9

Comment 6

[d.a.]

Maybe switch between app tabs with ctrl/cmd + alt + 1-9? 

It would make some sense on OS X at least as the alt/option key is very often used to access more options using the keyboard.

Comment 7

[Simon Montagu :smontagu]

Generalizing the summary (or hijacking the bug, depending on how you look at it ;-) -- but "Switch to App Tab with Ctrl+0/⌘+0" is WONTFIX for reasons already given in comment 1). I find the current behaviour of using Ctrl/Alt/Cmd-<1..9> for both app tabs and regular tabs inconvenient and counter-intuitive. Something like comment 6 is the way to go IMHO.

Comment 8

[imradyurrad]

Eh, no probs. Progress is progress after all.

Comment 9

[Sindre Dammann]

Attachment: Proposed patch + test

In this patch accel+alt+num selects an apptab on XP_GNOME, alt+num selects an apptab on all other platforms. On Windows 7 (or on my computer at least) accel+alt+num only fires when using the numpad, so I reckon we can't use that.

The patch changes the function selectTabAtIndex to only select normal tabs, and adds a new selectAppTabAtIndex function which only selects app tabs.

What about the ctrl+tab, ctrl+shift+tab, etc. keyboard shortcuts? Should their behavior be changed as well? And if so what would their desired behavior be?

Comment 10

[Kevin Brosnan]

You will need to request review of this patch to get it in the tree. https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch#Getting_Reviews and use [e-mail]

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 468293 [details] [diff] [review]
Proposed patch + test

> #else
> #define NUM_SELECT_TAB_MODIFIER accel
>+#define NUM_SELECT_APPTAB_MODIFIER alt
> #endif

This doesn't seem right, as Alt is used for OS-level shortcuts on Windows. I'd suggest shift+accel instead.

Also, I'm not sure I like the proposed change at all. Please get the patch ui-reviewed.

Comment 12

[Sindre Dammann]

Comment on attachment 468293 [details] [diff] [review]
Proposed patch + test

(In reply to comment #11)
> This doesn't seem right, as Alt is used for OS-level shortcuts on Windows. I'd
> suggest shift+accel instead.

I tried accel+shift+num, but that didn't work on my machine (had no effect).

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 468293 [details] [diff] [review]
Proposed patch + test

Moving the review over to limi since he is driving the app tab design

Comment 14

[Alex Limi (:limi) — Firefox UX Team]

Comment on attachment 468293 [details] [diff] [review]
Proposed patch + test

I can't tell exactly what's in this patch, but if it's Shift-(Cmd|Ctrl)-<number> to reach the app tabs, then I'm OK with this. Agree with Dao that Alt/Option isn't the right qualifier to use.

Comment 15

[Sindre Dammann]

Attachment: Attempt at Ctrl+Shift Patch

Here's a patch that uses Ctrl+Shift. However it does not work. I have no idea why, it works with "alt", but when I change it to "accel,shift" the command never executes.

Maybe somebody else knows, or suspects, what could be causing this? I'm all out of ideas.

Comment 16

[Dão Gottwald [:dao]]

It might fail because shift+1 isn't 1 anymore...

Comment 17

[Sindre Dammann]

Oh, right.

Then what would be the best way to implement this? Replacing 1, 2, 3, etc. with ! @, #, etc. doesn't seem like a very good option since the character produced will vary depending on the keyboard layout. Using addEventListener("keydown",...) with e.keyCode seems to work, but implementing a few of the shortcuts using a completely different method seems rather inconsistent.

Is there a better way?

Comment 19

[Dão Gottwald [:dao]]

(In reply to comment #17)
> Oh, right.
> 
> Then what would be the best way to implement this? Replacing 1, 2, 3, etc. with
> ! @, #, etc. doesn't seem like a very good option since the character produced
> will vary depending on the keyboard layout. Using
> addEventListener("keydown",...) with e.keyCode seems to work, but implementing
> a few of the shortcuts using a completely different method seems rather
> inconsistent.

Maybe remove the current <key>s and implement everything using addEventListener?

Comment 20

[Wyatt Childers]

Why bother with 0 can't we just leave this be and use 1-9?