Improving the Accessibility Interface for Dolphin
Review Request #106555 - Created Sept. 24, 2012 and submitted
Hi Frank. Sorry for unintended damage. I have removed the layouter function from public in this diff. Also had to make other changes to make the AccessibleInterface work with Orca as well.
Tried it with Orca and KMag
Thanks for the patch! > Hi Frank. Sorry for unintended damage. All right, apology accepted :-) Just remember to format the commits in a working branch nicely before pushing large changes in the future. If this seems to be too much work, which it usually is, I think it's better to push such changes in a single commit rather than including the full history, unless the history may be helpful for understanding the feature (which was IMHO not the case here). Using friends rather than giving public access to the layouter looks like an improvement to me. I still think that there might be corner cases where the row/column calculation could go wrong, but fixing this is a job for me, I'll try to remember to look into it before we enter the beta phase. This is OK to go in from my point of view (after considering my comments below). The only thing that's not quite clear to me is why you propose to remove an assert which actually looks quite reasonable at first sight?
Review request changed
Hi, just updated the diff according to your comments, apart from the assert.
Revision 2 (+20 -17)
Thanks for the explanations and the new patch! I agree that the assert should better be removed then. I have two more little comments on whitespace issues. Feel free to push this ater fixing them, no review request-update needed.