as in Mozilla, Netscape and IE, we should be able to tab btwn the urlbar and the web page content. this is not the case for chimera --unless there's another keyboard shortcut for doing this. once i'm in the urlbar (or anywhere in the toolbar, since the buttons are focusable), i cannot get back to the page content unless i use the mouse: 1. make sure a page is loaded (not about:blank), http://kith.org/ 2. go to the urlbar: either hit cmd+L or just click there with the mouse. 3. hit tab to return to the web page content. results: tabbing/shift+tabbing only cycles through the toolbar buttons and the urlbar --never goes to the page content. likewise, if i'm in the page content, tabbing/shift+tabbing will only cycle through focusable content, and will not go to the toolbar/urlbar: 1. make sure a page is loaded (not about:blank), http://kith.org/ and that focus is not in the toolbar. 2. hit tab to cycle through. results: focus remains in the page content --never goes to the urlbar/toolbar.

yeah, need to figure out tabbing interaction with cocoa

just use cmd-l in the interim. that's all we do in mozilla.

doesn't cmd-tab work in mozilla?

haven't tried cmd+tab yet.. but the problem with cmd+L is that it gets you to the urlbar, but once there, how do you use the keyboard to get back to the page content? (in mozilla you could hit tab from there to return to the content.)

Biggie for me, this one. I also think that Mac users will clamour for an option for tabbing between editable form elements only, rather than every link (IE has this). We should be able to do this with stylesheets, right?

cmd+tab goes through the Dock icons (open apps); according to the AHIG, that's expected. i tried ctrl+tab (nothing happens) and option+tab (just put in tab whitespace), to no avail. sfraser sez he'll take this, for the time being...

For consistency with other Mac browsers, you should probably be able to tab from the urlbar to the page content simply by pressing TAB (takes you to the first focussable element, which depending on your preferences could be the first form field or the first link) or SHIFT-TAB (takes you to the LAST focussable element). Similarly, to be consistent with other Mac browsers, you should be able to tab from the page content to the urlbar by pressing TAB when on the last focussable element on the page, or by pressing SHIFT-TAB when on the first.

The way you set up tabs in Cocoa is by setting an object's nextKeyView to the view you want to tab to. You can do this using the setNextKeyView: method in NSResponder (which NSView inherits from). You can also set this with Interface Builder. This has to do with fabled Responder Chain in Cocoa: http://developer.apple.com/techpubs/macosx/Cocoa/TasksAndConcepts/ProgrammingTopics/BasicEventHandling/Concepts/AboutRespChain.html Hitting tab twice makes the browser view active. I think this means that there's another view in between the toolbar and the content view that's getting firstResponder status first. Unfortunately, I have no idea which view that is. I thought it was the bookmarks toolbar, but even when it's hidden and it can't accept first responder (which I set using the -(BOOL)acceptsFirstResponder method), it still takes two tabs. Any thoughts on what other view might be getting in the way?

BTW, just to be clear, when I said "set up tabs," I meant "tabbing interaction," not tabs as UI elements.

the behavior for this has changed (2002.08.08.05 nb) --i can now tab from the urlbar to the page content, but there's an intervening "dead spot": 1. start with focus in urlbar via cmd+L. 2. hit tab once --focus moves out of the urlbar (no more highlighting), but it's not in the page content either 9per prachi's comment 8, 2nd paragraph), as tabbing at this point does nothing. 3. hit tab a 2nd time: now you can scroll thru the page content. i can then hit tab (or shift-tab) to cycle thru the focusable items in the content. the issue of not cycling back to the urlbar remains an issue, however... anyhow, has something changed (perhaps the checkin for bug 151039?) that now allows tabbing from the urlbar to the content (in spite of the deadspot)?

*** Bug 168130 has been marked as a duplicate of this bug. ***

re comment #2 - mike - Cmd-L takes you to the URLbar but doesn't bring you back out. Maybe if you hit Cmd-L, and then hit ESC (usually mapped to cancel in dialogs) you can go back?

drat, i cannot seem to repro comment 10, where i was able to reach the content area via tabbing. tested on 10.2.2 with 2002.11.26.04. what i see now: 1. load a page, eg, http://google.com/ --focus will initially be in the content area, as expected. 2. go to the urlbar, cmd-L. 3. start hitting the tab key. results: i end up cycling through all of the enabled items (buttons/field) in that toolbar. at the end of the cycle, though, there's still a "dead spot" before tabbing goes to the next enabled item. as mentioned earlier, this isn't a problem on IE... just recently looked at OmniWeb, and i'm also able to move focus from the toolbar (eg, the urlbar field) to the content area via the tab key.

You have keyboard accessibility on, eh?

I think that's correct: I don't have full Keyboard Access turned on, and I have never been able to tab out of the URL bar (to any location) on any build.

i have full keyboard access turned on --i even have "highlight any control" selected. (fwiw, i use letter keys instead of function or custom keys for the control-based shortcuts.) upon reading comment 15, looks like this bug occurs whether or not full keybd acces is on, though...

I'm glad to see this is on the meta blockers list .. I keep running into this bug and it's frustrating to go back to the mouse.

Comment #8 seems to indicate the way to proceed, and it would be very easy to implement a basic tab or shift-tab means of moving back and forth between location bar and page content, if it were not for the fact that Chimera implements a tabbed browsing (and here I mean multiple views appear in the same window!). As such, every time the selected view changes, the responder chain has to change as well. Not sure how this would interact with Full Keyboard access, though. Sairuh, what happens when you hit tab in IE or Mozilla (etc) with Full Keyboard access activated?

Wincent, I may be displaying my ignorance here since I haven't looked at the nibs ;-) but if there is a single view that wraps the tabbed viewing area, then the selection can go to the wrapper view which presumably knows which of the tabs is foremost.

I can't find a way to set up the tab order between the toolbar and the content area. The issue here is that the toolbar is in its own special world; it's not in the window content, and also, the toolbar items are not NSViews, and so can't have their nextKeyView set. We could hack things so that a tab key in the location text field focusses the content, but that breaks 'hilight any control' mode. However, this is what Mail.app appears to do for its search field.

a bit more info (or perhaps a quasi-workaround): recipe a -------- 0. make sure keybd prefs are set to highlight any control (eg, i use the letter controls). 1. load a page. 2. go to the urlbar, cmd-L. 3. rather than hitting tab as in comment 13, hit ctrl-T. 4. the first item in the bookmarks toolbar is in focus. now, keep hitting the tab key till you reach the last bookmarks toolbar item. 5. hit tab again. results: focus will *finally* go to the content area (rather long-winded way to go). recipe b -------- 0. make sure keybd pref is the same as in (a). 1. load a page. 2. select something (highlight) in the page, or start tabbing around till a focusable item (link, image-link) gets focus (focus ring). 3. hit control-T, which focuses the first item in the navigation toolbar. 4. [optional] tab around the items in the navigation toolbar. 5. hit control-T again. results: focus is returned to the content area --either at the selection or the focused item, depending on what you did in step 2. recipe c -------- 1. repeat steps 0-2 in recipe (b) above. 2. bring the urlbar in focus, cmd-L. 3. repeat steps 3-5 in recipe (a) above. results: focus will *finally* go to the content area (again, a rather long-winded way to go).

*** Bug 191747 has been marked as a duplicate of this bug. ***

It's a two-line fix in AutoCompleteTextField.mm in the delegate method for handling key events, where the insertTab: method is being handled, if the mPupupWin is NOT visible (i.e., the autocomplete popup is not showing) then I set the firstResponder to the window. This defaults tabbing back to the regular browser view. Hit tab again and you will being to cycle through the elements on the web page. I'm not totally satisfied because I still think that nextKeyView /should/ work but I played around with it a lot and couldn't get it to work. This is a workaround, but I think there's probably something more fundamentally wrong with how firstResponders are being handled (and it might have something to do with the fieldEditor, although I played with that too). In any case this patch allows to move from the URL bar to the browser view with TWO tabs. If you just do ONE tab then the key view is apparently nothing because it just beeps (which is Cocoa's default when there's nothing to accept the key events). This bug was such a pain ...

Comment on attachment 115733 [details] [diff] [review] two-line fix two line fix.

looks reasonable to me. simon, do you want to land it or should i?

Be my guest

landed. thanks!

cool! i'll test this with tomorrow's nightly...

tested with 2003.03.14.12 on 10.2.4 --i have added "accessibility.tabfocus" pref set to 7 (all widgets, see bug 160387) and the Keyboard sys pref set to tab to all widgets as well. 1. start with focus in urlbar (eg, cmd+L). 2. start hitting the tab key... results: * you need to hit tab twice to get out of the urlbar * after that, focus moves to each item in the Bookmarks toolbar --whether the BM tb is hidden or displayed-- * then, finally, focus goes to the content area. * caveat: tabbing goes through all focusable content, then cycles back up to the top of the web page content, but not cycling back up to the urlbar. not like mozilla behavior --but at least you can hit cmd+L to get there. :)

As far as cycling back to the url bar, camino needs an impl of nsIWebBrowserChromeFocus. See http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsIWebBrowserChromeFocus.idl When tabbing through content elements, at some point Gecko will get a tab key after it has visited the last focusable item. At this time, it calls up to the embedding app using nsIWebBrowserChromeFocus::FocusNextElement(). When this happens, the embedding app can set the focus on the next item after the web browser.

Comment on attachment 115733 [details] [diff] [review] two-line fix We have a new Camino request flag which can be set multiple times for a patch. Moving old review requests to the new flag. Sorry for the spam.

This doesn't work any more.

Ping. Since this is on the 1.0 list, I'd thought I'd do a quick ping to see if anyone has done work on it. For accessibility reasons, it seem quite important for Camino.

Can someone confirm that this is suddenly working again on the branch--if you don't have the Seach field in your toolbar? (You'll need to open a new window after you've removed the Search field from the toolbar; the first window seems to get confused.) It still (as always) takes two tabs to get out of the URL bar and into the content area, but you can tab there now in some cases, which wasn't possible at all before. You still can't tab all the way back to the URL bar (or shift-tab to go right back to it), though.

(In reply to comment #34) > Can someone confirm that this is suddenly working again on the branch--if you > don't have the Seach field in your toolbar? Yup, WFM 2005091409 (v1.0a1)

I can tab *into* the content area from the location bar, but not out of it using 2005092804. Can you tab back out of it?

(In reply to comment #36) > Can you tab back out of it? "You still can't tab all the way back to the URL bar (or shift-tab to go right back to it), though."

Is there any way this can get fixed by 1.0?

We're getting mighty close to 1.0, can we please please have this fixed? It seems like such a trivial issue but it's triviality makes it a basic necessity. I think we should have a choice between tabbing through control elements and through every link. For example, start with address bar and tab once to the search bar. Tab again to the content area so we can have page navigation (up/down, left right), tab again to the first control element and so on and cycle back to the address bar. On the second option, the next tab after content area takes you to the first link. on beta 1, tabbing from search bar just cycles through the toolbar.

This won't make 1.0 unless someone can figure out what's going on. For now... moving off the 1.0 list.

Comment 34 won't work with the splitview location bar/search field, so the only place this even *partially* works anymore is on the dead-end 1.8.0 branch.

some further testing... if you follow the steps in mentioned in bug 185385 comment 6, you can essentially get the BM toolbar to activate. that is, with the 1.8.0 builds. start loading a page, and hide camino before it can finish loading. wait for the load to complete, then activate camino and notice that the first item in the BM toolbar has an aqua outline around it. pressing shift-tab allows you to immediately select the last text field in the document. alternatively, press just tab repeatedly until you go through all the items in the toolbar, after which the first text field in the page will be selected. can someone test this on the new 1.8.1 builds? full keyboard access needs to be on. i'm testing on 10.3.9.

some further testing... if you follow the (slightly modified) steps I mentioned in bug 185385 comment 6, you can essentially get the BM toolbar to activate. that is, with the 1.8.0 builds. start loading a page, and hide camino before it can finish loading. wait for the load to complete, then activate camino and notice that the first item in the BM toolbar has an aqua outline around it. pressing shift-tab allows you to immediately select the last text field in the document. alternatively, press just tab repeatedly until you go through all the items in the toolbar, after which the first text field in the page will be selected. can someone test this on the new 1.8.1 builds? full keyboard access needs to be on. i'm testing on 10.3.9.

This almost works; it does all the hard work. All that is left is making the content view have a proper next keyview (and previous keyview).

Håkan, I'm going to reassign this to you since you have it mostly working.

Since Håkan is not actively working on these right now, -> 1.2

There is currently, AFIK, no way to make a NSToolbar or NSToolbarItem the window's firstResponder (they are not NSView subclasses). That is going to make this bug kind of difficult. Håkan's patch gets us on the right track, as it knows when focus should leave the browser content area. We need a way to determine the nextValidKeyView and if it happens to be in the toolbar, make it the first responder. This isn't a new problem, see Simon's thread on Cocoa-Dev back in 02: http://www.cocoabuilder.com/archive/message/cocoa/2002/12/3/53713. A real hack which can almost* accomplish this (meant as an example only): NSArray *toolbarItems = [[[self window] toolbar] visibleItems]; NSEnumerator *toolbarItemEnumerator = [toolbarItems objectEnumerator]; id toolbarItem; while ((toolbarItem = [toolbarItemEnumerator nextObject])) { if ([toolbarItem isEnabled]) { [[self window] makeFirstResponder:[toolbarItem view]]; break; } } * The problem with the above is that only toolbar items with a custom view explicitly set will return anything for [toolbarItem view]. An AppKit private method [toolbarItem _view] returns an NSView in all cases, but is not suitable for use. I've been experimenting with the key view loop by way of bug 346803. A patch here is almost necessary for it.

Responding to Sean Murphy's latest comments: It seems the hangup here is in trying to provide a generic solution for any toolbar item that can accept respnoder status. But realistically isn't it only the "URL+Search" toolabr item that needs to be focused, in order to satisfy the expected key loop? Customize Toolbar doesn't show any other obvious candidates for first responder status. So my suggestion is to identify the "URL+Search" toolbar item by identifier in the while loop above, and when it's discovered (i.e. if it's in the toolbar), to simply hook the known NSView for it into the chain. if ([[toolbarItem identifier] isEqualToString:kURLSearchBoxItem]) { [[self window] makeFirstResponder:**savedURLSearchView**]; } This is a bit kludgy and lacks flexibility for future responder-items in the toolbar, but then again, isn't the current failure also a bit kludgy? Fixing this would remove what seems to be a persistent embarrassment to Camino, and especially likely to turn off picky first-time users. Daniel

(In reply to comment #48) > So my suggestion is to identify the "URL+Search" toolbar item by identifier in > the while loop above, and when it's discovered (i.e. if it's in the toolbar), > to simply hook the known NSView for it into the chain. > > if ([[toolbarItem identifier] isEqualToString:kURLSearchBoxItem]) { > [[self window] makeFirstResponder:**savedURLSearchView**]; > } > > This is a bit kludgy and lacks flexibility for future responder-items in the > toolbar, but then again, isn't the current failure also a bit kludgy? Fixing > this would remove what seems to be a persistent embarrassment to Camino, and > especially likely to turn off picky first-time users. How does that affect users who have Full Keyboard Access turned on (or does it not change how FKA works)? cl

> How does that affect users who have Full Keyboard Access turned on (or does it > not change how FKA works)? I'm not 100% sure, but I think full keyboard access is sort of "magical" in that Apple just inserts items that are not normally part of the keychain into the keychain as designated by developers. But my suggestion was mostly meant as a way to possibly break the stand-still that seems to have come from comment #47. Assuming everything else in the patch is good, then my suggestion may overcome the limitation described there. Summary: the bug seems to be stalled on "finding the right NSView to make next key view," and my suggestion is to simply store the reference to the one toolbar item view we know we want to include in the chain.

(In reply to comment #50) > Summary: the bug seems to be stalled on "finding the right NSView to make next > key view," and my suggestion is to simply store the reference to the one > toolbar item view we know we want to include in the chain. I am going to heed Daniel's suggestion and try a few things here. He's right, without losing some flexibility in regard to which toolbar item will receive focus when tabbing around, this bug is unlikely to make any progress. The plan of attack I recommend is to first just get some behavior in place, that is, tabbing out of the browser content to at least jump to the toolbar's location field. For now, this action is far better than doing nothing. I think then we should just file another bug report for increasing the flexibility and allowing focus on the first eligible view (which, with FKA on, probably isn't the location field - see below). (In reply to comment #49) > How does that affect users who have Full Keyboard Access turned on (or does it > not change how FKA works)? This is the one noticeable problem. With FKA on and the default toolbar set, tabbing out of the browser content should select the back toolbar button when focus jumps around to the top. So, coercing the focus to the location field essentially ignores FKA settings and always behaves as if FKA is turned off. There are keyboard shortcuts assigned for just about every toolbar button though, so tabbing to them is probably the slowest and least desirable way of performing their action. The most common sequence of actions on the user's part will likely be tabbing four extra times to get to the location field. Again, we do absolutely nothing now, so it's not like we abide FKA in this regard currently either.

This patch is based upon Håkan's start and inspired by Daniel's behavioral suggestion. It turns out Simon first mentioned taking this path back in comment #20: > I can't find a way to set up the tab order between the toolbar and the content > area. The issue here is that the toolbar is in its own special world; it's not > in the window content, and also, the toolbar items are not NSViews, and so can't > have their nextKeyView set. > > We could hack things so that a tab key in the location text field focusses the > content, but that breaks 'hilight any control' mode. However, this is what > Mail.app appears to do for its search field. Since it's five years after that comment was made, and no better way has surfaced, I agree that we should just go with it. Also, what he describes about Mail.app back on '02 is still true - which might indicate we're not the only ones who've run into such a problem.

Comment on attachment 267834 [details] [diff] [review] patch Håkan, will you have any time soon to have a look at this patch?

Comment on attachment 267834 [details] [diff] [review] patch Since the patch is mostly the same as my old one, I mostly want to know the behaviour of this. 1. Does this mean that tabbing in the location bar doesn't move to the search bar? IMO, that needs to work. 2. Just a sanity-check: tabbing between elements in the browser still works, but once it hits the first/last item it goes back to the location bar?

(In reply to comment #54) > > 1. Does this mean that tabbing in the location bar doesn't move to the search > bar? IMO, that needs to work. The patch only affects tabbing in the browser (web page) content area. Currently, the browser's content is essentially a trap as far as the key-view loop is concerned, meaning when you reach the last focusable element on a page it loops around to the beginning of the browser content. What the patch does is allow BrowserWrapper to be informed when tabbing has reached either the beginning or the end of the content's key-view loop, and when that event happens it can step in and take an appropriate action (to break the endless closed loop that would normally take place; for now that means moving focus up to the URL bar). After focus is outside of the browser content the key-view loop is not affected in any way; So tabbing from the location bar will still correctly move focus over to the search field. You actually did most of the heavy lifting for this patch and set up the underlying behavior and probably knew most of this, but I wanted to explain it in detail so everyone knows what's going on here. So now, you can start with focus in the location bar, tab all the way through the search field, bookmark toolbar, every web page form element, and back up to the location field, and so on. > 2. Just a sanity-check: tabbing between elements in the browser still works, > but once it hits the first/last item it goes back to the location bar? Yes, the only time tabbing inside the browser content is affected is when tabbing reaches either the first or last focusable element. All other locations in the content's key-view loop are not touched.

Thanks for the answers Murph. I tested the patch, it works great. It's so nice to be able to get out of the content view using the keyboard! One thought (maybe it has already been answered in the plethory of the discussion): can't we handle the "shift-tab" case when going backwards out of the content view by just getting and focusing the BrowserWrapper's previousKeyView or something? If we're lucky, that'd mean going back to the bookmarks bar or search bar, which would rock.

Comment on attachment 267834 [details] [diff] [review] patch This is a big step forward. >+- (void)tabOutOfBrowser:(BOOL)tabbingForward; >+{ >+ // For now, since there doesn't appear to be a good way to transfer focus up to a >+ // NSToolbarItem, we're arbitrarily choosing to focus the location field. This is >+ // done regardless of which direction the user is tabbing out of the browser. >+ >+ if ([[[self window] windowController] respondsToSelector:@selector(focusURLBar)]) >+ [[[self window] windowController] focusURLBar]; >+} >+ We should probably focus the search bar when tabbing backward ([aBWC focusSearchBar]) - I believe this'll make the behavior with FKA off work correctly. >+// Implementation of nsIWebBrowserChromeFocus >+/* void focusNextElement(); */ Nit - mixed comment styling is ugly.

I hope this patch wasn't sitting around just because I didn't explicitly stamp it review-. I was waiting for my comment 56 to be replied to...

(In reply to comment #56) > One thought (maybe it has already been answered in the plethory of the > discussion): can't we handle the "shift-tab" case when going backwards out of > the content view by just getting and focusing the BrowserWrapper's > previousKeyView or something? > > If we're lucky, that'd mean going back to the bookmarks bar or search bar, > which would rock. Håkan, I apologize for not getting to this comment. I must have become sidetracked with other work at the time and forgot to address it. Anyway, using previousKeyView is definitely the preferred approach and correct way to go about this, but like forward tabbing using nextKeyView, it doesn't work out. There are at least two scenarios where it breaks down: When the bookmark bar is visible, BrowserWrapper's previousValidKeyView is properly set to the last BookmarkButton, so reverse tabbing out of the browser area will correctly move keyboard focus into the bookmark bar. Continued reverse tabbing along the bookmark bar, however, eventually runs into the same NSToolbarItem key view problem. This means that a shift-tab on the first BookmarkButton will not move focus onto the search or location field like it should and instead loop back down and avoid the toolbar all-together. The other scenario is when the bookmark bar is hidden, and in this case a shift-tab out of the browser area will again (because of the inability to make a NSToolbarItem firstResponder) just loop around inside the browser content. Maybe we could set the first BookmarkButton's previousKeyView to be the search bar to get this approach working. Then, we would still need to account for a hidden bookmark bar. Maintaining a manual key-view loop in an application where views can be shown/hidden programmatically at any time is tough work. Plus, the toolbar seems to be magically placed into the loop by Apple. For now, I just updated the current approach with Froodian's comments. It's better than nothing, but definitely not ideal. -- We are left with a problem though, and I didn't fix it under this bug since it'll probably warrant a separate report. Calling |focusURLBar| or |focusSearchBar| without the combined location/search toolbar item visible makes the browser view inactive while focus, at the same time, actually remains inside of it (since it never left and went up to the toolbar). Then, at least one side-affect of setting the browser inactive in this situation seems to be that you cannot interact with input text fields until transferring focus off to an outside view. So, I guess we should not |setBrowserActive:NO| unless we're sure focus will in fact leave.

Comment on attachment 279944 [details] [diff] [review] patch v2 r=me

(In reply to comment #59) > The other scenario is when the bookmark bar is hidden, and in this case a > shift-tab out of the browser area will again (because of the inability to make > a NSToolbarItem firstResponder) just loop around inside the browser content. > > Maybe we could set the first BookmarkButton's previousKeyView to be the search > bar to get this approach working. Then, we would still need to account for a > hidden bookmark bar. Maintaining a manual key-view loop in an application > where views can be shown/hidden programmatically at any time is tough work. > Plus, the toolbar seems to be magically placed into the loop by Apple. > > For now, I just updated the current approach with Froodian's comments. It's > better than nothing, but definitely not ideal. > > -- > > We are left with a problem though, and I didn't fix it under this bug since > it'll probably warrant a separate report. Calling |focusURLBar| or > |focusSearchBar| without the combined location/search toolbar item visible > makes the browser view inactive while focus, at the same time, actually remains > inside of it (since it never left and went up to the toolbar). Then, at least > one side-affect of setting the browser inactive in this situation seems to be > that you cannot interact with input text fields until transferring focus off to > an outside view. So, I guess we should not |setBrowserActive:NO| unless we're > sure focus will in fact leave. I think setting the next/previousKeyView for things inside views that may be hidden is OK. When a view is hidden, nothing inside it can get first responder, so the responder chain should ignore it and pick the next best item.

(In reply to comment #61) Yes, good point - that's what {next,previous}ValidKeyView are for. So, maybe what we could do is set up the key view loop within Interface Builder as much as possible, and then do some more fine-grained control in code, where necessary... For instance, regarding the BookmarkToolbar, if you add a new BookmarkButton you'll notice that this new button is not in the key view loop and is skipped over. Only after opening a new window does Cocoa automatically calculate the loop and account for all buttons. (sort-of bug 338557). Ideally, we could just rely on NSWindow's |recalculateKeyViewLoop| and |setAutorecaculateKeyViewLoop:|, but these methods are 10.4+ only. When maintaining a manual loop, NSToolbarItems are only inserted into a closed nextKeyView loop, meaning the last item must have nextKeyView set to jump around to the beginning. These items seem to always be inserted at the top (coordinate-wise) location of the closed loop. Another thing that bugs me is when you have a superview, such as the BookmarkBar, containing a bunch of subviews, you cannot just set the superview as a nextKeyView, but rather you need to dive in and hook up each subview into the outside loop. Does it make more sense to follow what you suggested in comment 56 combined with re-working the key-view loop as I described above? We will probably still need the |tabOutOfBrowser:| code, but instead of arbitrarily directing focus we could rely on BrowserWrapper's {next,previous}KeyView outlets to tell us where to go. Where it might get a little difficult is that each BrowserWrapper is not instantiated within IB, so the most we can get at through there is BrowserTabViewItem, I believe. I'd rather do this the right way, but doing so would depend upon BrowserWindow have a fully working key-view loop. I'll keep fooling around and see where I can get with this approach.

Comment on attachment 279944 [details] [diff] [review] patch v2 > We are left with a problem though, and I didn't fix it under this bug since > it'll probably warrant a separate report. Calling |focusURLBar| or > |focusSearchBar| without the combined location/search toolbar item visible > makes the browser view inactive while focus, at the same time, actually remains > inside of it (since it never left and went up to the toolbar). Then, at least > one side-affect of setting the browser inactive in this situation seems to be > that you cannot interact with input text fields until transferring focus off to > an outside view. That's a serious usability regression that would result from this change; that's not something for another bug, it's a problem with this patch. If your other strategy doesn't work, you should have this forward to a method in BWC that can make intelligent decisions based on toolbar state, and place the focus accordingly rather than sending it off into the abyss.

(In reply to comment #63) > That's a serious usability regression that would result from this change; > that's not something for another bug, it's a problem with this patch. I understand. I wasn't trying to be lazy; My thinking was that since |focusURLBar| is offered as part of BrowserWindowController's public API, it should take its own steps internally to determine if the URL bar is in fact visible before doing anything. Clients of the method should not have to know that the implementation winds up setting the browser view as inactive. Unless there's something documented in |focusURLBar|, I'd expect it to just take a safe approach and do nothing if there's no toolbar visible. The reason I brought up another bug is that calling |focusURLBar| without a toolbar and running into the problem is not exclusive to this particular usage, it could happen to any caller of that public method. Anyway, I've really been working on my other strategy to manually establish a fully working key view loop throughout the entire browser window, and then integrate the browser content's tabbing behavior into that loop. One key aspect, which is making it tough, is that BrowserWrapper's nextKeyView must be set to its corresponding CHBrowserView; otherwise the browser content will never receive keyboard focus. The NSToolbar continues to be a thorn in my side. I have found its magical insertion by Apple into the key view loop to be inconsistent during reverse (shift) tabbing. You cannot reverse tab into a toolbar, and reverse tabbing when focus is inside the toolbar will throw you back out onto a view never explicitly entered into the normal loop. I attached a quick test app in case anyone wants to visually see what I mean: (Note: turn on full keyboard access before testing) On launch, button 1 will be the key view. Pressing tab would normally go to the next key view (which is button 2), but based on geometric order the toolbar items have been inserted before it and they will receive focus instead, so they're in the loop and that's working fine. With focus outside the toolbar and on one of the buttons, continue pressing shift-tab and reverse around the loop. You'll notice that the toolbar never comes into play this time, even though the expected focus location should be unchanged from that of forward tabbing. Next, forward tab around until you're up in the toolbar, and with one of the toolbar items highlighted, continue to reverse tab until transferred out of the toolbar. You'll notice that the key view is now one completely outside of the loop, which was chosen based solely on geometric order. I don't want to get too off-topic, but since the toolbar is an essential part of our browser window's key view loop, I wanted to fully explain the issues at hand. I'll keep working at this approach to determine if it's feasible, and if not we can just fall back to the aforementioned patch.

Pushing this out to 2.0; it's too late to take it now without having seen a patch yet, but Sean's got some work done, so we really want to fix this for 2.0.

Works for me, on OS X 10.5.3, 3.0 and 3.0.1pre

Since this is a Camino bug, behavior in Firefox 3 isn't relevant.

I've had some more time to look at this issue again. Here's an update about where I'm at with it: Basically, maintaining a manual key-view loop, I can get forward tabbing to work correctly throughout the entire browser window. Tabbing past the last focusable browser element will properly focus the first eligible toolbar item. The only problem I still have to tackle is reverse tabbing into the toolbar. I've actually talked with Apple about the "magic" behind-the-scenes key-view loop maintenance handled by AppKit, specifically the insertion of the toolbar into the loop. To ensure the toolbar is properly integrated at the correct location, the key view loop must be a closed one, meaning the last key view must jump around and point to the first. Also, there must be no views which canBecomeKeyView outside of the loop for AppKit to properly detect where the toolbar belongs. This last point is our problem, as the ChildView Gecko content subviews of CHBrowserView are not present in the loop. Apple recommended that I recursively loop over all CHBrowserView subviews when page loading is complete and set a next key view on them. This approach does work and allows the entire window's loop to work soundly forward and backward, but not in all situations. ChildView's can be created dynamically, after the page loading process is complete (on Google.com for instance, clicking "more" to display a menu of additional searches prompts the creation of another subview). Furthermore, with 10.4+ support we have available NSWindow's setAutorecalculatesKeyViewLoop and recalculateKeyViewLoop, but they do not assign any nextKeyView on any ChildViews. So, again, without incorporating the ChildViews into the loop, forward tabbing still does work correctly throughout the entire window. Reverse (shift) tabbing also works correctly except the toolbar is skipped. I'll continue to take a look at this, and, it is indeed the best approach, I'll try to determine a way to ensure ChildViews are always present in this loop. Feel free to share any ideas.

This patch hooks the browser content into the key view loop. As we talked about in #camino-mtg, the only catch is that reverse tabbing will skip the toolbar without introducing any hacks to integrate all ChildViews into the key view loop. The fixes for the find bar (bug 408534) and the popup blocker (bug 346803) were so tightly integrated with this code, that I thought it made sense to include fixes for them in here as well. If this will get in the way of bug tracking/research (through CVS history), I can break them apart if need be.

To go along with the latest patch, attached is a new BrowserWindow.nib, wherein I set the window's initialFirstResponder (to inform AppKit we're managing the loop ourselves) and set up a simple key view loop cycling between the BookmarkToolbar, BrowserTabBarView (not yet focusable, I'll get to that soon), and the BrowserTabView. Also included are nibs for the FindBar and PopupBlockedView, where I merely needed to set the |nextKeyView| of the actual FindBarView and PopupBlockedView to their first control subview. Again, I didn't like attaching these here, since there are separate bugs, but the supporting code for them is tightly integrated with the patch.

Comment on attachment 331847 [details] [diff] [review] Patch Håkan, since you've worked with this sort of code (this very code in a couple of cases) before, can you review Sean's patch?

Comment on attachment 331847 [details] [diff] [review] Patch I would've liked to, but I'm sorry: I won't have time for this. (Got too many things to do, and this patch is huge so I'll just be honest right away.)

Comment on attachment 331847 [details] [diff] [review] Patch This is one of our priorities for 2.0 so it needs to be in someone's queue, and if Håkan can't review it, realistically that means it ends up in Stuart's queue :(

Comment on attachment 331847 [details] [diff] [review] Patch Three behavior problems I'm seeing with this patch: - The serious one that needs to be fixed is that my bookmark bar becomes a black hole; once I tab into it, I can't tab past the first item. I assume we need to set that up manually now that we don't have an automatic key loop being generated? - A minor issue that I'm okay with, but sure why it's happening: forward tabbing out of content takes me to the toolbar, but forward-tabbing out of the find bar takes me to the content area.

(In reply to comment #75) > (From update of attachment 331847 [details] [diff] [review]) > Three behavior problems I'm seeing with this patch: > - The serious one that needs to be fixed is that my bookmark bar becomes a > black hole; once I tab into it, I can't tab past the first item. I assume we > need to set that up manually now that we don't have an automatic key loop being > generated? > - A minor issue that I'm okay with, but sure why it's happening: forward > tabbing out of content takes me to the toolbar, but forward-tabbing out of the > find bar takes me to the content area. You know what, both of the issues you describe are expected without my BookmarkToolbar key loop patch on bug 441828. I should have indicated the dependency. That last point is caused, without the BookmarkToolbar patch, because when the find bar appears its last key view (the match case button) is set to the BookmarkToolbar. Since the toolbar is skipped, the next key view found by AppKit winds up back at the CHBrowserView. So, again, all of these problems are solved in the patch on bug 441828.

Comment on attachment 331847 [details] [diff] [review] Patch Putting back in my queue, since with all the patches combined the behavior is looking solid.

Comment on attachment 331847 [details] [diff] [review] Patch >+// Sets |aNextKeyView| to follow the last key view at the end of our internal subview loop. >+// Explanation: The BrowserWrapper behaves similarly to NSTabView in that it manages an internal key >+// view loop among its subviews, encapsulating them from external users. |setNextKeyView:| is not used >+// here because -[BrowserWrapper nextKeyView] actually points to the first subview of the wrapper This comment is a bit confusing in that the implementation does use setNextKeyView:; I think what you really mean is that *clients* use this instead of setNextKeyView:. Reword to make that clearer (and axe "Explanation:", since that's clear from context). r=me with that change and the understanding that the blocking bugs need to land before/with this change.

(In reply to comment #78) > (From update of attachment 331847 [details] [diff] [review]) > >+// Sets |aNextKeyView| to follow the last key view at the end of our internal subview loop. > >+// Explanation: The BrowserWrapper behaves similarly to NSTabView in that it manages an internal key > >+// view loop among its subviews, encapsulating them from external users. |setNextKeyView:| is not used > >+// here because -[BrowserWrapper nextKeyView] actually points to the first subview of the wrapper > > This comment is a bit confusing in that the implementation does use > setNextKeyView:; I think what you really mean is that *clients* use this > instead of setNextKeyView:. Reword to make that clearer (and axe > "Explanation:", since that's clear from context). Updated that comment.

Comment on attachment 338650 [details] [diff] [review] Patch, v1.1 sr=pink

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