Not as either native / safari's tabs, the new tab widget is not focusable when "full keyboard access" is on. Try the following in Safari, Camino 0.8.x branch and recent Camino trunk builds: 1. Turn on "Full Keyboard Access" is System Preferences->Keyboard 2. Try to tab to the tabbar (from the searchbar or something) and move between tabs using the left/right arrows. It should be pretty easy to draw a focus ring around the tab rect (at least in Carbon ; ) )

There are a couple of other open issues with tabbing/focus issues that may or may not be related: bug 152987 and bug 198153.

tough call on whether or not to block the release for this. I think it'll take a while to get it fixed (the fix might not be bad, but finding someone to do it will be). For now I don't think we should hold up the release, but we should definitely get this into a minor update after 0.9 it it doesn't make it into 0.9.

Taking

This would be great to have for 1.0, but is it practical? How hard is the fix going to be? I say move to 1.1.

This shouldn't be too hard, should it?

-> 1.1

-> hwaara, per big SoC meeting

Mass un-setting milestone per 1.6 roadmap. Filter on RemoveRedonkulousBuglist to remove bugspam. Developers: if you have a patch in hand for one of these bugs, you may pull the bug back to 1.6 *at that point*.

Key view loop support for the tab bar.

Comment on attachment 335084 [details] [diff] [review] Patch This functions well in my testing, even without the nib change for bug 152987, which I gather is needed for making the full loop work. (That is, this patch does successfully insert the tabs, their close buttons, the scroll arrows, and the All Tabs menu successfully into our current dysfunctional loop, and the focus/key loop works as expected in the tab bar within the context of that known dysfunction.)

Comment on attachment 335084 [details] [diff] [review] Patch We chatted about this at the meeting, and the patch does require and should be tested with the nib on bug 152987, but it doesn't require the patch from 152987 (that is, the nib lands with whichever bug of this, bug 441828, or bug 152987 to land first).

Comment on attachment 335084 [details] [diff] [review] Patch >+ NSView* mOriginalNextKeyView; // The next key view after the tab bar, as set from within Interface Builder. Same name change here as in the bookmark bar bug. >+ TabButtonView* firstTabButton = [(BrowserTabViewItem*)[mTabView tabViewItemAtIndex:0] buttonView]; It can happen that we get here in initial setup with firstTabButton being nil. That used to be okay because we'd call this method later even when the tab bar is hidden and it would be fixed up, but with my changes in bug 454030 that's no longer the case. You'll need a // If we don't have a tab button yet, just keep the current nextKeyView. if (!firstTabButton) return; here. >+ for (int currentButtonIndex = 0; currentButtonIndex < numberOfTabs; currentButtonIndex++) { > [...] >+ if (currentButtonIndex == (numberOfTabs - 1)) { > [...] >+ } >+ else { Rather than constantly checking to see if you are at the end, loop to one before the end and move the whole contents of the if block outside the loop It's weird that the tab bar is reaching into the tab view's subviews to hook up the tab chain, but I can see where it could be messy to do the other way (and that's hardly the worst encapsulation problem in our tab system), so we can call this good for now. >+ if ([[theEvent characters] isEqualToString:@" "] && >+ (firstResponder == mOverflowLeftButton || >+ firstResponder == mOverflowRightButton)) >+ { >+ [super keyDown:theEvent]; >+ if ([firstResponder acceptsFirstResponder]) >+ [[self window] makeFirstResponder:firstResponder]; >+ } >+ else { >+ [super keyDown:theEvent]; >+ } Move the [super keyDown:] to just above the |if| (since you are testing against the saved firstResponder, that will still work) and get rid of the else block. Also, as a minor nit: right now the #pragma mark separates the drag-and-drop handling in this file from everything else, so put your new methods above to maintain that (otherwise the mark is really random). With all the various patches brought together and the one fix above, the behavior looks great!

Updated to address review comments.

Comment on attachment 338648 [details] [diff] [review] Patch, v2 sr=smorgan

Landed on cvs trunk along with bug 152987 and bug 441828; thanks Sean! If you see a regression or, more likely, a case where the new loop breaks, please file a follow-up bug.