The code in ctrlNumberTabSelection() is for switching between tabs in Firefox. The marked block needs to be configurable via a preference, either a new one or one of the existing ones ("ui.key.chromeAccess", "ui.key.menuAccessKey" etc) AFAICT, this tab switching feature is not yet present in SeaMonkey, but it would be nice to have it someday and we should consider what that means for the shortcuts under the Window menu (CTRL+1 to open Navigator and so forth).

AFAICT ui.key.accelKey is pretty much what was intended in that place. This patch additionally makes sure not to accept Accel+Shift+<#> and other modifier combinations. NB: For easier modifier handling, it would be nice to have a .modifiers property on key events which returns a mask of all keys pressed. This would simplify handling multi-modifier combos (as e.g. for bug 340902) and also checking for exactly one modifier (as in this patch).

Comment on attachment 230631 [details] [diff] [review] use accelKey for ctrlNumberTabSelection Simon, AFAIK I don't have superreview rights on this code. I think we shouldn't do the event.shiftKey test in this case, so that Accel+Shift+NUMPAD_1 etc will also work. with that, r=mats

+ if (event.ctrlKey && (event.altKey || event.metaKey) || + event.altKey && event.metaKey) You need some parenthesis here to make it clear what the last && and || apply to.

(In reply to comment #4) > You need some parenthesis here to make it clear what the last && and || apply > to. Seems like I got too used to operator precedence in this case. Would you mind adding rules as to when parentheses should be applied to the style guide at http://developer.mozilla.org/en/docs/JavaScript_style_guide ?

Comment on attachment 230631 [details] [diff] [review] use accelKey for ctrlNumberTabSelection As per bug 366084 comment#2, not ignoring Shift state for tab selection might be the better thing to do. Advantage: <accel>+Shift+<number> can be used for something else. Disadvantage: we'll have to make sure that <accel>+<number> indeed works on all platforms (so we don't get another version of bug 357101).

Mats: Can I leave the waiting for superreview and doing the check-in to you?

Sure, no problem.

Comment on attachment 232778 [details] [diff] [review] patch ignoring the Shift key So, this nets out to changing the modifier from Alt to Ctrl on Linux, which is something we explicitly changed to match other apps (gedit, gnome terminal, etc) Probably justifies its own pref at this point, the patch looks okay otherwise.