nsAccessibleWrap::GetNativeInterface() always returns NULL on OS X, because mNativeWrapper is always NULL when it's called. This is because nsAccessibleWrap::Init() is never called on the nsAccessible corresponding to two top-level objects -- an nsXULDocument (whose nsAccessible is an nsRootAccessibleWrap object), and an nsHTMLDocument (whose nsAccessible is an nsDocAccessible object). This is in turn because nsRootAccessible::Init() and nsDocAccessible::Init() override nsAccessibleWrap::Init() without ever calling the latter. Because (on OS X) nsAccessibleWrap::GetNativeInterface() always fails on a window's top-level objects, none of a window's lower-level nsAccessible objects are visible to an accessibility client (like Accessibility Inspector or Accessibility Verifier). So most things that should be "accessible" (and *are* "accessible" on other platforms), aren't "accessible" on OS X. As best I can tell this has been true from the beginning -- from the time our accessibility code first supported OS X.

Here's a patch that fixes the problem. I've tested it in both 32-bit builds (on OS X 10.5.8) and 64-bit builds (on OS X 10.6.3). I tested with Accessibility Inspector and Accessibility Verifier (available under /Developer/Applications/Utilties/Accessibility Tools/). I'm not very familiar with the accessibility code, so I'm not sure this patch is the best way to fix the bug, or even that it won't have undesirable side effects. But I didn't have any trouble in my (rather brief) tests. And I tried my best to conform to how things are done in existing code (several other nsAccessible subclasses also have Init() methods that call nsAccessibleWrap::Init()). Because nsRootAccessible::Init() calls nsDocAccessibleWrap::Init() (which is typedefed to nsDocAccessible::Init() on OS X), I didn't need to also add a call to nsAccessibleWrap::Init() there. There are still a few cases where nsAccessibleWrap::GetNativeInterface() returns NULL -- notably for nsLinkableAccessible objects and their subclasses. But this is because AncestorIsFlat() returns TRUE for those objects. A tryserver build will follow in a few hours. It will be a 32-bit trunk build that (naturally) supports accessibility. Note that Accessibility Verifier returns quite a few errors. But I suspect they have nothing to do with this patch -- that they are caused by errors in other code that's now being exercised for the first time.

Here's a tryserver build made with my patch from comment #1: https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla560939/bugzilla560939-macosx.dmg

A bit surprisingly, there are no failures in the a11y mochitests (make mochitest-a11y) with my patch. I ran them locally with a 32-bit build on OS X 10.5.8.

Steven, great investigative work thanks! Note our a11y mochitests don't test our platform layers, only our common core API nsAccessible* and friends. So they are expected to pass on OSX.

> Steven, great investigative work thanks! You're most welcome. Glad I could help. > Note our a11y mochitests don't test our platform layers, only our > common core API nsAccessible* and friends. So they are expected to > pass on OSX. I suspected something like that :-) But thanks for telling me.

Comment on attachment 440600 [details] [diff] [review] Potential fix that's not right fix since document accessible will register itself in own child cache (on os x), so it backports the patch from bug 543961. I'll try to figure out more correct fix. Steve, thank you much for the help, for the time you've spent on this (and of course for that you've found the problem)!

Probably the best way would be to copy/paste nsAccessibleWrap::Init code to nsDocAccessibleWrap::Init.

How about this? > that's not right fix since document accessible will register itself > in own child cache (on os x), so it backports the patch from bug > 543961. I assume that by "backports" you mean something like "breaks". > Steve, thank you much for the help, for the time you've spent on > this (and of course for that you've found the problem)! You're most welcome. The underlying problem has been bugging me, too, since I first encountered it a year ago. But before now I never knew enough to do anything about it.

Or better still.

Steven, I realize it's really good point to share the code. However possibly not in this case. It might be easier and readable to create nsDocAccessibleWrap and override Init() method. Especially it makes sense because I believe AncestorIsFlat() check (used in nsAccessibleWrap::Init()) doesn't make sense on documents.

(In reply to comment #8) > > that's not right fix since document accessible will register itself > > in own child cache (on os x), so it backports the patch from bug > > 543961. > > I assume that by "backports" you mean something like "breaks". yep, I meant it's partial back out that patch. > The underlying problem has been bugging me, too, since I first > encountered it a year ago. But before now I never knew enough to do > anything about it. Do you mean the problem exists during one year?

> Do you mean the problem exists during one year? By "underlying problem" I mean the following (from comment #0): > Because (on OS X) nsAccessibleWrap::GetNativeInterface() always fails > on a window's top-level objects, none of a window's lower-level > nsAccessible objects are visible to an accessibility client (like > Accessibility Inspector or Accessibility Verifier). So most things > that should be "accessible" (and *are* "accessible" on other > platforms), aren't "accessible" on OS X. This problem (and this bug) have existed for *at least* a year.

That's interesting because I thought it must be regression from bug 543961 which was landed couple months ago.

> That's interesting because I thought it must be regression from bug > 543961 which was landed couple months ago. I simply don't know ... and I really don't have time to find out. All I do know is that the underlying problem (objects inside a window not being "accessible") has existed at least since I first saw it a year ago. And (judging by the number of errors in Accessibility Verifier) I suspect it's existed from the time our accessibility code first supported OS X.

OK then, how about this?

Looks good to me.

Comment on attachment 440803 [details] [diff] [review] Fix rev3 (follow Alex's suggestion) >+class nsDocAccessibleWrap: public nsDocAccessible >+{ >+public: >+ nsDocAccessibleWrap(nsIDOMNode *aNode, nsIWeakReference *aShell); >+ virtual ~nsDocAccessibleWrap(); >+ >+ // creates the native accessible connected to this one. nit: use /** */ style for comments nit: please add "// nsIAccessNode" before the comment >+ NS_IMETHOD Init (); nit: virtual nsresult, since NS_IMETHOD used for interface methods (idl) prototype, though I know there is no difference, just style issue >+ * Contributor(s): >+ * Original Author: Steven Michaud (smichaud@pobox.com) nit: usually the style: Steven Michaud <smichaud@pobox.com> (original author) is used in a11y >+#include "nsDocAccessibleWrap.h" >+ >+#import "mozAccessibleWrapper.h" >+ >+nsDocAccessibleWrap::nsDocAccessibleWrap(nsIDOMNode *aDOMNode, nsIWeakReference *aShell) : nit: if it's more than 80 chars then do nsDocAccessibleWrap:: nsDocAccessibleWrap(nsIDOMNode *aDOMNode, nsIWeakReference *aShell) : >+nsresult >+nsDocAccessibleWrap::Init () >+{ >+ nsresult rv = nsDocAccessible::Init(); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ if (!mNativeWrapper) { actually it shoudn't be possible that Init() is called twice. I'm fine to have a protection from this but please add NS_ASSERTION. >+ // Create our native object using the class type specified in GetNativeType(). >+ mNativeWrapper = new AccessibleWrapper (this, GetNativeType()); this line makes me worry, we create base class for document accessible, however nsChildView is created for chrome and for content documents if I get right. I wonder if we need to use mozRootAccessible here or similar. Any way this way is how it worked before it was broken so it's not a subject for this bug. r=me with nits fixed. Again, thank you for doing this!

The tree is currently closed, and may stay that way for a while. So here's another revision, with (hopefully) all the nits fixed. > I wonder if we need to use mozRootAccessible here or similar. As best I can tell the answer is "no". In mozDocAccessible.h there's the following comment: /* The root accessible. There is one per window. Created by the nsRootAccessibleWrap. */ This implies there should only be one mozRootAccessible per window -- the one created by nsRootAccessibleWrap.

Comment on attachment 440848 [details] [diff] [review] Fix rev4 (fix nits) r=me, thank you

(In reply to comment #18) > This implies there should only be one mozRootAccessible per window -- > the one created by nsRootAccessibleWrap. ok

Comment on attachment 440848 [details] [diff] [review] Fix rev4 (fix nits) I just wanted to confirm that, after building with this patch locally, I can confirm that we're back to a state where we can work on bugs such as bug 499927 and bug 499931, as well as performance. VoiceOver again sees the dialogs and page content with this patch. Thanks Steven!

> VoiceOver again sees the dialogs and page content with this patch. Glad to hear it. > Thanks Steven! You're very welcome! > I just wanted to confirm that, after building with this patch > locally, I can confirm that we're back to a state where we can work > on bugs such as bug 499927 and bug 499931, as well as performance. Hopefully you'll no longer need my help on those two bugs :-) By the way, it's while working on them that I first noticed what I've been calling "the underlying problem" -- objects inside a window not being "accessible". It wasn't quite a year ago ... but I guess it now seems like it :-)

For the record, here's the debugging patch I used to figure out this bug. I've updated it to remove false leads and to include the latest revision of my patch. Most of what I say in comment #0 and comment #1 is based upon logs that it produced. To compile it, you need to add the following you your mozconfig (RTTI is Run-Time Type Information): ac_add_options --enable-cpp-rtti This is just to show how much you can accomplish without stepping through code in a debugger.

The tree is open, but currently in bad shape. I'll take another look this afternoon.

Landed on trunk: http://hg.mozilla.org/mozilla-central/rev/db79d2ce808c