Using Ctrl+Tab/Ctrl+Shift+Tab for tab switching is broken on the trunk, for Mac. Mano suspected it may be a regression from the cocoa widgets switch. I'm pretty surprised that I would be the first person to report this, but I can't find a dupe, and neither Mano nor mconnor think this was an intentional change (in fact it was re-added for 2.0 in bug 264787), so here we are.

Thanks for marking my bug as duplicate. Is it possible to switch to Hardware field from "PC" to "Mac"? I didn't think to use the combination of "OS X" OS and "PC" hardware in my searching.

Hardware="PC" and OS="Mac OS X" is what Bugzilla prefills for Intel Macs, by default. You should just ignore the Hardware field when looking for bugs by operating system.

Ctrl+Tab for switching (non-browser) tabs is broken in seamonkey xpfe builds as well.

Stefan: What do you mean by "non-browser" tabs? (Sorry if I'm being dense.) Everyone: Do Ctrl-PgUp and Ctrl-PgDn also not work on Macs? IIUC, they are supposed to also walk tabs. At least, they work on my Linux system, where the (kde) window manager intercepts Ctrl-Tab and Ctrl-Shift-Tab, both of which therefore never reach any non-pure-text app.

(In reply to comment #5) > Stefan: What do you mean by "non-browser" tabs? (Sorry if I'm being dense.) Tabs in, for example, Page Info

(In reply to comment #6) > (In reply to comment #5) > > Stefan: What do you mean by "non-browser" tabs? (Sorry if I'm being dense.) > > Tabs in, for example, Page Info > Have you tried Ctrl-PgDn (or Cmd-PgDn) there? Does it work if you do?

Ok, regression window is 2006-09-28 to 2006-09-29, this points a finger at bug 326469. Josh, gavin said you might know what the problem was?

I confirm that for regular web pages tabs, on MacBook Pro with Gran Paradiso a6, Ctrl-PgDn and Ctrl-PgUp do work for regular web page tabs. Ctrl-tab and Not Cmd-PgDn don't work. For Preferences and Page info, there is no way to switch tabs from the keyboard.

Well, the good news is that this WFM on 10.5 9a466 :) Bad news, still seeing it on 10.4. Adjusting the summary appropriately.

The little test app on bug 398514 makes it easy to see the problem here. On 10.4 Cocoa doesn't send an NSKeyDown event for control-tab, as that is special for first responder control. On 10.5 we get a performKeyEquivalent: message with an NSKeyDown event for control-tab, which is why this bug doesn't show up on 10.5. So, on 10.4 we just don't get an NSKeyDown control-tab event from cocoa. We might be able to do a crazy appshell hack to find the event and post it somehow, but I'm not sure that'll work or what it would do to embedding, and if it did work it would be messy. Probably the best thing to do here is we get the NSKeyUp event from cocoa for control-tab send an NS_KEY_DOWN event into gecko before the NS_KEY_UP event gets sent into gecko. The only consequence of that is that tabs will switch on key up, not key down. This will be cleaner if we do it along with or after the changes in bug 398514.

(In reply to comment #12) > Probably the best thing to do here is we get the NSKeyUp event from cocoa for > control-tab send an NS_KEY_DOWN event into gecko before the NS_KEY_UP event > gets sent into gecko. The only consequence of that is that tabs will switch on > key up, not key down. This is a deal breaker for bug 395980.

Why is that a deal breaker? Nothing would be different from Gecko's POV.

Does "on key up" relate to Ctrl or to Tab? Ctrl would mean that you could never bring that panel up, Tab would mean that you couldn't scroll quickly through the tabs (which would be tenable I think).

It would make the latter impossible, scrolling through a bunch of tabs without ever bringing the key up. However, that seems like an unlikely behavior and not worth a huge effort to work around this bug in a better way, sorry.

Agreed; I was assuming the former.

There is probably some code consolidation that can be done in our key handling, but since we're about to make more changes soon I think it is best not to prematurely optimize and stick with clarity for now. Once we know more about what our final key handling structure is going to be we can consolidate some code into shared functions.

Josh: Why don't you call keyDown simply?

keyDown: does stuff we don't need to do. And if the first responder changes in response to the key down event we don't want to just return we want to call keyUp: on the new first responder.

Comment on attachment 285316 [details] [diff] [review] fix v1.0 o.k.

This updated patch fixes two bugs in the original. First of all if an NS_KEY_DOWN event sent from keyUp: changed focus the call to keyUp: in the new first responder would send another NS_KEY_DOWN event. Secondly, if keyUp: called keyUp: after a first responder change, the original keyUp: call would continue on to send another keyUp: event.

Comment on attachment 285499 [details] [diff] [review] fix v1.1 I haven't tested this. But I can't find any logical errors ... so it seems fine to me.

Oops, forgot to re-diff after my last change. This includes the second fix I described for v1.1.

Comment on attachment 285515 [details] [diff] [review] fix v1.2 Looks fine to me.

+ NSResponder* resp = [[self window] firstResponder]; + if (resp != (NSResponder*)self) { Does this not-focused path get taken if the key-down event tears down the window? + nsKeyEvent geckoEvent(PR_TRUE, NS_KEY_PRESS, nsnull); + [self convertCocoaKeyEvent:nativeKeyDownEvent toGeckoEvent:&geckoEvent]; What if this event tears down the window? + if ((!geckoEvent.isChar || geckoEvent.isControl) && + !nsTSMManager::IsComposing()) { How can isControl be false here, or geckoEvent.isChar be true here?

Add additional safety checks for window/view destruction and don't do the isChar/isControl checks when we already know which value they must have.

+ if (!mGeckoChild) + return; How do we know that 'this' is still alive? Is someone holding a strong ref to it? + // if this is a non-letter keypress, or the control key is down, + // dispatch the keydown to gecko, so that we trap delete, + // control-letter combinations etc before Cocoa tries to use + // them for keybindings. Fix comment

Yes, it is still alive via strong ref (autorelease pool strong ref or explicit).

Comment on attachment 285611 [details] [diff] [review] fix v1.3 assuming comment fixed

landed on trunk, this is the exact patch I landed

I've verified that: * ctrl + shift + tab cycles backwards from the currently selected tab * ctrl + tab cycles forward from the currently selected tab works as expected in both: PPC: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a9pre) Gecko/2007102404 Minefield/3.0a9pre Intel: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007102404 Minefield/3.0a9pre Verified FIXED

Hmm, so on Tiger, Ctrl+Tab will activate on key up from now on? That's unfortunate for perceived performance.