User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a4pre) Gecko/20070327 Minefield/3.0a4pre Build Identifier: Frame has state focusable and fires focus events. No other GNOME application does this, frame is never the keyboard input focus itself. See wiki addressing issue: http://live.gnome.org/LSR/FocusEvents Reproducible: Always Steps to Reproduce: 1. 2. 3.

A browser is different than an editor that way. Anything which is potentially scrollable needs to be focusable. Those items are actually focusable. In addition, this is redundant with bug 375746, which talks about extra focus events but only gives the document frame as an example. Any reason not to mark this INVALID?

Again, this is talking about the window frame, not the document frame. The window frame in Firefox is never scrollable. Therefore, it should not get focus or focusable, or fire those events.

Thanks. I was confused because a frame in Mozilla-terminology is a <frame>, which is a subdocument.

It's correct behavior in MSAA then, but not in ATK.

(In reply to comment #3) > Thanks. I was confused because a frame in Mozilla-terminology is a <frame>, > which is a subdocument. For our future edification, what's the term Mozilla uses for the top-level window frame? (In reply to comment #4) > It's correct behavior in MSAA then, but not in ATK. Thanks for clarifying. In AT-SPI, we rely on window:activate events from the window manager to know when a window comes to the foreground. No need for you to do anything for that event. Focus, to us, means the user's keyboard input is going to affect the accessible that fired the event.

We call it the root accessible, or the top level window. ATK/AT-SPI terminology is fine as long as you indicate that's what's being used, e.g. ATK_ROLE_FRAME would make it clear.

This also fixes bug 375746.

This fixes bug 375746, bug 375747 and bug 375748. However, event skew fix from bug 374790 is necessary for it to work on Windows. One thing it does not fix for menu focus events is when Alt or F10 is pressed to focus the menu. That is a regression from bug 341462, and I opened bug 376640 for it.

Comment on attachment 260753 [details] [diff] [review] Fixes mising focus events when exiting menus as well > nsDocAccessible::GetState(PRUint32 *aState, PRUint32 *aExtraState) > { > // nsAccessible::GetState() always fail for document accessible. > nsAccessible::GetState(aState, aExtraState); > >- *aState |= nsIAccessibleStates::STATE_FOCUSABLE; >+ nsCOMPtr<nsIXULDocument> xulDoc(do_QueryInterface(mDocument)); >+ if (!xulDoc) { >+ *aState |= nsIAccessibleStates::STATE_FOCUSABLE; >+ } Every XUL document implements nsIXULDocument, therefore if some XUL document is in xul:iframe then it can't be focusable. > nsRootAccessible::GetState(PRUint32 *aState, PRUint32 *aExtraState) > { > nsresult rv = nsDocAccessibleWrap::GetState(aState, aExtraState); > NS_ENSURE_SUCCESS(rv, rv); > >- NS_ASSERTION(mDocument, "mDocument should not be null unless mDOMNode is"); >- if (gLastFocusedNode) { >- nsCOMPtr<nsIDOMDocument> rootAccessibleDoc(do_QueryInterface(mDocument)); >- nsCOMPtr<nsIDOMDocument> focusedDoc; >- gLastFocusedNode->GetOwnerDocument(getter_AddRefs(focusedDoc)); >- if (rootAccessibleDoc == focusedDoc) { >- *aState |= nsIAccessibleStates::STATE_FOCUSED; >- } >- } You said everyting that is scrollable can be focusable. Why can't root accessible focusable? >- if (accessible == this) { >- // Top level window focus events already automatically fired by MSAA >- // based on HWND activities. Don't fire the extra focus event. >- NS_IF_RELEASE(gLastFocusedNode); >- gLastFocusedNode = mDOMNode; >- NS_IF_ADDREF(gLastFocusedNode); >- return NS_OK; >- } I don't get why this MSAA hack can be removed.

Comment on attachment 260753 [details] [diff] [review] Fixes mising focus events when exiting menus as well cancelling review request until comments are answered and fixed if needed.

> Every XUL document implements nsIXULDocument, therefore if some XUL document > is in xul:iframe then it can't be focusable. > You said everyting that is scrollable can be focusable. Why can't root > accessible focusable? The normal method of using frame->IsFocusable() I tried a number of ways to find a frame that returns PR_TRUE for that, in the HTML case, and couldn't find one. To be honest I'm not sure how to do this right, to generally find out whether a document is focusable. Thanks for bringing up the issue. For now, this is good enough to get 99% of the cases and unblock the LSR team. I've filed a follow-up bug 376803 to fix this correctly. > >- if (accessible == this) { > >- // Top level window focus events already automatically fired by MSAA > >- // based on HWND activities. Don't fire the extra focus event. > >- NS_IF_RELEASE(gLastFocusedNode); > >- gLastFocusedNode = mDOMNode; > >- NS_IF_ADDREF(gLastFocusedNode); > >- return NS_OK; > >- } > > I don't get why this MSAA hack can be removed. We don't fire any focus events anymore with this patch, on any nsDocAccessible/nsRootAccessible unless it actually gets focused. This is now prevented at the end of FireAccessibleFocusEvent(). The DOM will fire a focus event for the document even though a control inside of it wants focus. This is not desirable in MSAA or ATK. Since there is no longer any extra event to prevent, we don't need that code.

Comment on attachment 260753 [details] [diff] [review] Fixes mising focus events when exiting menus as well >- *aState |= nsIAccessibleStates::STATE_FOCUSABLE; >+ nsCOMPtr<nsIXULDocument> xulDoc(do_QueryInterface(mDocument)); >+ if (!xulDoc) { >+ *aState |= nsIAccessibleStates::STATE_FOCUSABLE; >+ } Please add this XXX section for bug 376803 since this behaviour can be changed. >- NS_ASSERTION(mDocument, "mDocument should not be null unless mDOMNode is"); >- if (gLastFocusedNode) { >- nsCOMPtr<nsIDOMDocument> rootAccessibleDoc(do_QueryInterface(mDocument)); >- nsCOMPtr<nsIDOMDocument> focusedDoc; >- gLastFocusedNode->GetOwnerDocument(getter_AddRefs(focusedDoc)); >- if (rootAccessibleDoc == focusedDoc) { >- *aState |= nsIAccessibleStates::STATE_FOCUSED; >- } >- } here I guess too. If XUL document can be focusable then root document can be focusable too if we won't think about Firefox and e.t.c.

Yes on first, but no on second XXX That old code just tells us if focus was anywhere in the root accessible. From now on we'll only want to use STATE_FOCUSABLE on a doc or root when it's not an inner child control that has actual focus.

Checked that crash fix in as well.

Apparently the focus events are still happening.

New fix checked in.

test added via changeset: http://hg.mozilla.org/mozilla-central/rev/073164ba54ce

Backed out changeset that included test.