While investigating bug 616136, I found that setting noautofocus="true" doesn't stop popup notification panels from receiving focus when they are opened, but that seems to only happen when a chrome element has focus at the time the panel is opened. I'm not familiar with the way focus works, so this is a guess by me. Nevertheless, we need a way to prevent a panel from getting focus when it opens.

Is there some steps I can use to test this? Which notifications are causing this problem?

This problem used to apply to popup notifications until the noautofocus="true" was removed in bug 594586. We removed that so that these notifications will just always get focus for the sake of consistency. You can test this by reverting the change from that bug. I think we would like to put back the noautofocus="true" if we can get it to work as expected.

I tested this and don't see any bug, and I can't follow the discussion in bug 594586 enough to understand what prompted this bug to be filed.

(In reply to Neil Deakin from comment #3) > I tested this and don't see any bug, and I can't follow the discussion in > bug 594586 enough to understand what prompted this bug to be filed. Neil, the problem is that, as soon as you get a doorhanger notification, keyboard focus is no longer inside the currently loaded page. If you press Tab after such a password prompt (for example) appears, you are thrown into this panel rather than staying in the context of the currently loaded page. In Firefox 3.6 and earlier, where we had the simpler notification popup, focus would always stay within the page even when the notification bar appeared. This is no longer the case. So to reproduce: 1. Provoke a doorhanger, like a password remembrance prompt. 2. Press tab once this appears. Expected: You'd go to the first focusable item on the newly loaded page. Actual: You land in the doorhanger panel on the "Save password" button menu. And there, you're bitten, incidentally, by bug 653226.

(In reply to Marco Zehe (:MarcoZ) from comment #4) > 1. Provoke a doorhanger, like a password remembrance prompt. > 2. Press tab once this appears. > > Expected: You'd go to the first focusable item on the newly loaded page. Wouldn't this break keyboard accessibility of the doorhanger? How would you access the doorhanger using the keyboard otherwise?

(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #5) > Wouldn't this break keyboard accessibility of the doorhanger? How would you > access the doorhanger using the keyboard otherwise? With the Firefox 3.6 notification bar, the save password button had a mnemonic like Alt+P or something (I don't recall). This mnemonic is now also present inside the doorhanger, and because it is an accessibility alert type, the mnemonic key is spoken, too. Alternatively, if the panel is present, it could be included in the F6 cycling order, directly preceeding the document, so that if a doorhanger appears, and the user *chooses* to interact with it, they can press Shift+F6 once to get there and then tab through the buttons or choices. Of course, pressing the mnemonic key from inside the document would still work.

Note that, in order to move this forward, I've collected all information and a proposed solution on this wiki page, using the doorhanger as the main example for this: https://wiki.mozilla.org/Accessibility/Doorhanger_Keyboard_Navigation

Note that this patch should adds panels with focusable element to the F6 document tab order. It doesn't change any other behaviour. Probably noautofocus should be set on the panel.

Comment on attachment 591826 [details] [diff] [review] Patch, put panels in the document tab order This feels right to me. Focus is still circling around the password remembrance doorhanger, but I suspect that's a problem in the widget that needs to be solved separately.

This fixes some tests that failed with the previous patch because the starting content was null.

Comment on attachment 596746 [details] [diff] [review] Improved patch and add a test Coding style in general is: if (expr) { stmt; } >+ if (aType == MOVEFOCUS_FORWARDDOC || aType == MOVEFOCUS_BACKWARDDOC) { >+ // When moving between documents, make sure to get the right >+ // starting content in a descendant. >+ nsCOMPtr<nsPIDOMWindow> focusedWindow; >+ startContent = GetFocusedDescendant(aWindow, true, getter_AddRefs(focusedWindow)); >+ } >+ else { } else { >+nsIContent* >+nsFocusManager::GetNextTabbableDocument(nsIContent* aStartContent, bool aForward) >+{ >+ // If currentPopup is set, then the starting content is in a panel. >+ nsIFrame* currentPopup = nsnull; >+ nsCOMPtr<nsIDocument> doc; > nsCOMPtr<nsIDocShellTreeItem> startItem; >- if (mFocusedWindow) { >+ >+ if (aStartContent) { >+ doc = aStartContent->GetCurrentDoc(); >+ if (doc) { >+ startItem = do_QueryInterface(doc->GetWindow()->GetDocShell()); >+ } >+ >+ // Check if the starting content is inside a panel. Document navigation >+ // must start from this panel instead of the document root. >+ nsIContent* content = aStartContent; >+ while (content) { >+ if (content->Tag() == nsGkAtoms::panel) { You want to check namespace here. Please ask review also from some a11y developer.

Comment on attachment 596746 [details] [diff] [review] Improved patch and add a test asking feedback from Marco per comment #12

Comment on attachment 596746 [details] [diff] [review] Improved patch and add a test I'm going to test this patch on a local build, but a question beforehand: Does it make sense to test tab key behavior here as well? Esp the focus behavor when the panel goes away, where focus goes, and where tab and shift+tab takes the user in such a situation. An example is the pressingof "space" on the "Close this message" button of the password doorhanger. Focus should be returned to the document, then. I think it would be good to test that focus does return to a defined position/element when a panel with a previously focused item goes away.

Comment on attachment 596746 [details] [diff] [review] Improved patch and add a test So, the bug makes the focus handling more consistent when there are focusable items in panels, but it does not address the original problem this bug was filed for, that the doorhanger panel automatically receives focus whenever it appears. This still happens even with the patch. So, not sure if I should f+ or f- this, since it definitely improves things, but does not fix the original bug yet.

That just requires someone to add noautofocus="true" to the panel you're testing. (as the original bug describes)

Neil, I guess Olli didn't meant review actually but feedback from a11y guy. You have one from Marco, that should be enough.

Well, ok, feedback+ should be enough :)

Comment on attachment 596746 [details] [diff] [review] Improved patch and add a test canceling review request then :)

I think the fix for this bug introduced a regression in BlueGriffon: if I launch BlueGriffon 1.4 and create a blank document, type a few chars and open the CSS Properties panel (that is a XUL floating panel), using the left arrow to move the caret works fine in the editor. If I do the same with BlueGriffon 1.4.1 (fix for the current bug is in), then the first menulist in my CSS Properties panel acquires the focus when the left arrow key is pressed and the editor loses the focus! I have a lot of users' feedback about this. Should the code in this fix check if a panel is floating? We have a rather strong behavioural difference here between floating panels and contextual popups.

(In reply to Daniel Glazman from comment #21) > I think the fix for this bug introduced a regression in BlueGriffon: Nevermind. I finally found the guilty commit and this is not the current bug but the fix for bug 669026. Sorry for the spam.

(In reply to Marco Zehe (:MarcoZ) from comment #15) > [The patch] does not address the original problem this > bug was filed for, that the doorhanger panel automatically receives focus > whenever it appears. (In reply to Neil Deakin from comment #16) > That just requires someone to add noautofocus="true" to the panel you're > testing. (as the original bug describes) Marco, was a bug ever filed to have this done for doorhangers? This still doesn't appear to be fixed. If not, I'll file one. :)

Jamie, feel free to file it! :)

As i understand it, this patch was supposed to put doorhangers in the f6 tab order (for want of a better term). However, I can't f6 or shift+f6 to an "update saved password" doorhanger. Am I misunderstanding the purpose of this patch or has it regressed?

(In reply to James Teh [:Jamie] from comment #25) > I can't f6 or shift+f6 to an > "update saved password" doorhanger. Filed bug 972117.