Post-commit review for the new accessibility classes

[Frederik Gladhorn]

On Tuesday 25. September 2012 22.14.12 Frank Reininghaus wrote:
> Hi Amandeep and Frederik,
> 
> I started to go through the new code, fixed a couple of coding style
> issues and removed some unnecessary complexity. The old version of
> KItemListAccessibleCell::navigate() was the most complicated way to
> return -1 I've ever seen ;-)
> 
> I'm by no means done, but I already have a few things that I'd like
> you to take care of/comment on:
> 
> 1. Your new files have no license header. This is totally unacceptable
> and must be fixed ASAP.
I agree, Amandeep wrote them, so I leave it to him to add his name.
> 
> 2. You've wrapped most of the code in #ifndef QT_NO_ACCESSIBILITY, but
> not the #includes for the QAccessible headers. I wonder if these
> headers are actually installed if Qt is built without accessibility or
> if someone who built a non-accessible Qt will get a build error here?
Qt always installs these headers with the same ifdef in them.
> 
> 3. I see that you print some debug output in
> KItemListViewAccessible::cellAt(). Is this still important for your
> current debugging needs? If not, I'd prefer if this was removed.
It should be removed.

By the way, since you asked about testing the stuff. One thing you can do in 
addition to getting kmag built (should hopefully work now, even without extra 
dependencies), you can try Orca or Accerciser from Gnome. In addition you need 
a recent version of qt-at-spi (projects.kde.org/qtatspi). With Orca you get 
nice speech output now for most things in Dolphin. I still wonder what is 
missing, but that's something we will find out by asking users how they hear 
the application. Accerciser is more of a develpment tool which shows what is 
exposed through the accessibility apis.

Greetings
Frederik



> Best regards,
> Frank