Comment 26 for bug 857153

(In reply to Hub Figuiere [:hub] from comment #18)
> Comment on attachment 572602 [diff] [details] [review]
> PATCH
>
> Review of attachment 572602 [diff] [details] [review]:
> -----------------------------------------------------------------
>
> ::: accessible/src/base/nsAccessibilityService.cpp
> @@ +81,4 @@
> > #include "nsTextFragment.h"
> > #include "mozilla/Services.h"
> > #include "nsEventStates.h"
> > +#include "prenv.h"
>
> is this needed?
>
> @@ +1833,4 @@
> > // Services
> > ////////////////////////////////////////////////////////////////////////////////
> >
> > + mozilla::a11y::FocusManager*
>
> I don't see the need for the indentation. just nitpicking here.

yeah, see my earlier comment, these two are fixed locally, I can upload a new patch if you like.

> ::: widget/src/gtk2/nsWindow.cpp
> @@ +1110,4 @@
> > }
> >
> > #ifdef ACCESSIBILITY
> > + if (aState && a11y::ShouldA11yBeEnabled())
>
> in nsAccessibilityService.h, ShouldA11yBeEnabled() is declared only we use
> ATK, while here it seems to be for the ACESSIBILITY.

I guess saying its only for atk is a little misleading, its really for desktop linux where we use atk / at-spi

However there is a sort of implicit assumption that the gtk backend will be related to using atk accessibility (which is sort of reasonable since gtk uses atk internally).

Another thing is that for atk we need general accessibility to suport atk.

I'd be willing to improve the comment if you have ideas.