> 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.
>
> I wonder whether the PreInit() in ShouldA11yBeEna bled() 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 && sAccessibilityE nabled) { yBeEnabled( )) sible() ;
> >+ if (aState && a11y::ShouldA11
> > CreateRootAcces
> >- }
>
> 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 :(
> sible() actually achieve? It looks like RootAccessible will somehow not need to create another
> What does CreateRootAcces
> mRootAccessible is unused. Is it holding a reference so that
> DispatchEventTo
> 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.
>