Deep hit-testing (so the deepest accessible will be found at the location of the cursor) doesn't work right now. This bug depends on bug 343904 We will come a long way towards fixing long-standing mac accessibility bugs that want to know the text under the mouse (bug 182126, bug 301451, bug 328153).

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

Deep hit-testing already works on simple XUL documents. For an unknown reason nsIAccessible's GetChildAtPoint() sometimes "stops" even if there are more accessibles beneath it. So I tested Aaron's idea from yesterday, of calling GetChildAtPoint(), then calling it again on the result, etc - iteratively, until I get the same result back. The good news is that this works! It's not the most efficient solution, but it actually makes VoiceOver speak whatever text I point at, or button, or link. I tested this on some major news pages and it works fine. It works *much* better than the VoiceOver + keyboard navigation combo (where I have a lot of performance trouble). Patch coming up.

Changes: 1. Clean up of the "represented view" stuff. I added a new method, GetClosestInterestingAccessible(anAccessible) that will just do the Right Thing. Previously, I did this logic in a lot of places, and the code was fragile and bug-prone. Specifically, the right thing is: * If |anAccessible| is ignored, get the closest unignored parent. * Always return the represented view if it has one. For example, the document view in the browser has an accessible "attached" to it. The OS wants us to return the view when referring to this accessible, and not the accessible object. I've discussed this before (and there are comments about it, if anyone is interested), but repeat because it's probably the most complicated thing about how the mac a11y code works. 2. Iteratively call GetChildAtPoint() when hit-testing. 3. Make nsAccessibleWrap respect whether a native accessible is ignored. This means that when we call nsAccessibleWrap::GetUnignoredChildren() (to get an array of clean, and interesting children), we'll skip over any objects that return YES from their accessibilityIsIgnored method.

The changes to sanityCheckChildren can be ignored. That's debug-only code that is never used (except in my local tree).

Comment on attachment 246781 [details] [diff] [review] Patch v1 >-ObjectOrUnignoredAncestor(id anObject) >+GetClosestInterestingAccessible(id anObject) I can't say I like 'closest interesting' much. Probably GetUnignoredAccessible is cleaner or something like. >+ >+ // start iterating as deep down as we can on this point, with the current accessible as the root. >+ // as soon as GetChildAtPoint() returns null, or can't descend further (without getting the same accessible again) >+ // we stop. >+ nsCOMPtr<nsIAccessible> deepestFoundChild, newChild(mGeckoAccessible); >+ do { >+ deepestFoundChild = newChild; >+ deepestFoundChild->GetChildAtPoint((PRInt32)geckoPoint.x, (PRInt32)geckoPoint.y, getter_AddRefs(newChild)); >+ } while (newChild && newChild.get() != deepestFoundChild.get()); > >- // see if we can find an accessible at that point. >- nsCOMPtr<nsIAccessible> foundChild; >- mGeckoAccessible->GetChildAtPoint ((PRInt32)geckoPoint.x, (PRInt32)geckoPoint.y, getter_AddRefs (foundChild)); That magic is strange. Why is GetChildAtPoint() enough for msaa and atk but it isn't for mac? Also, method looks generic, probably you should move it into nsAccessible object?

(In reply to comment #5) > (From update of attachment 246781 [details] [diff] [review] [edit]) > > >-ObjectOrUnignoredAncestor(id anObject) > >+GetClosestInterestingAccessible(id anObject) > > I can't say I like 'closest interesting' much. Probably GetUnignoredAccessible > is cleaner or something like. Yeah, the problem is that it's not only the closest unignored parent, but it also takes care of returning the represented view (if the accessible has one). I'm not sure that there is a name that can contain all this information... Unless you have other ideas. > > >+ > >+ // start iterating as deep down as we can on this point, with the current accessible as the root. > >+ // as soon as GetChildAtPoint() returns null, or can't descend further (without getting the same accessible again) > >+ // we stop. > >+ nsCOMPtr<nsIAccessible> deepestFoundChild, newChild(mGeckoAccessible); > >+ do { > >+ deepestFoundChild = newChild; > >+ deepestFoundChild->GetChildAtPoint((PRInt32)geckoPoint.x, (PRInt32)geckoPoint.y, getter_AddRefs(newChild)); > >+ } while (newChild && newChild.get() != deepestFoundChild.get()); > > > >- // see if we can find an accessible at that point. > >- nsCOMPtr<nsIAccessible> foundChild; > >- mGeckoAccessible->GetChildAtPoint ((PRInt32)geckoPoint.x, (PRInt32)geckoPoint.y, getter_AddRefs (foundChild)); > > That magic is strange. Why is GetChildAtPoint() enough for msaa and atk but it > isn't for mac? Also, method looks generic, probably you should move it into > nsAccessible object? According to aaronlev, MSAA calls accHitTest() in a loop, so it already has this logic. I haven't looked at ATK. I don't think it's worth it to put this in nsAccessible because it's really a hack. Aaron has talked about making a getDescendantAtPoint() that should do this work, and more efficiently, which is something we should use when it's available. If you want, I could add a comment about this.

What's difference between GetObjectOrRepresentedView(id <mozAccessible> anObject) if ([anObject hasRepresentedView]) and GetClosestInterestingAccessible(id anObject) if ([unignoredObject respondsToSelector:@selector(hasRepresentedView)]) ?

(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 246781 [details] [diff] [review] [edit] [edit]) > > > > >-ObjectOrUnignoredAncestor(id anObject) > > >+GetClosestInterestingAccessible(id anObject) > > > > I can't say I like 'closest interesting' much. Probably GetUnignoredAccessible > > is cleaner or something like. > > Yeah, the problem is that it's not only the closest unignored parent, but it > also takes care of returning the represented view (if the accessible has one). > I'm not sure that there is a name that can contain all this information... > Unless you have other ideas. In any way, "interesting" means lesser than unignored. Even if GetUnignoredAccessible is bad name but imo it is better than interesting one :)

> GetNativeFromGeckoAccessible(nsIAccessible *anAccessible) I always thought prefix 'a' means rather argument word than undefinite article :)

Comment on attachment 246781 [details] [diff] [review] Patch v1 Look good excepting interesting name :) Also, please fix things that have sense http://beaufour.dk/jst-review/?patch=https%3A%2F%2Fbugzilla.mozilla.org%2Fattachment.cgi%3Fid%3D246781&file=

Checked in with relevant JST-review comments addressed. Didn't change the function name (as discussed on IRC), since we can't come up with a explicit name for it, it needs to remain vague unfortunately. Mousing over stuff with VoiceOver should make it speak now! Any testing appreciated.

I don't know about VoiceOver, but I have a PowerPC Mac running Firefox Build 20090209020514 (3.0b3) and Cmd-Ctrl-D while holding my mouse over a word doesn't work like in other Mac apps. This looks like the same kind of thing. Is this the right place to put this, or should I file another report?