Comment 53 for bug 857153

Revision history for this message
In , Trevor Saunders (trev-saunders) wrote :

> I wonder whether the PreInit() in ShouldA11yBeEnabled() is really necessary
> or
> could be replaced with an assertion to check that PreInit had already been
> called. Perhaps it is needed for unit tests?

I don't think I'm familiar enough with when things happen in widget/ to be sure that call to PreInit() will come first.

> >- if (aState && sAccessibilityEnabled) {
> >+ if (aState && a11y::ShouldA11yBeEnabled())
> > CreateRootAccessible();
> >- }
>
> Please don't unbrace the block here. Convention in this file is heading
> towards always brace except for jump statements, but usually leave existing
> code as is.

ok :(

>
> What does CreateRootAccessible() actually achieve? It looks like
> mRootAccessible is unused. Is it holding a reference so that
> DispatchEventToRootAccessible will somehow not need to create another
> nsAccessible?

well, msot of that code came before me, but the only current purpose I can think of is to not fire the event if we don't need to since the root accessible is already created. Of course using a ref to the root accessible seems a silly way to do that, maybe Ginn or Alex knows more. I suspect a spin off bug to clean up thi would make sense, I'll gues this code hasn't been worked on in a good while for the most part.

I suppose its fairly clear though that the main purpose is to cause a11y to create a root accessible.
>