GNOME Bugzilla – Bug 303636
gnopernicus should report the focused line from a table even if there is no selected item
Last modified: 2005-06-07 21:19:14 UTC
If focus is on a line from a table and that line is not selected, gnopernicus reports nothing. This case can be obtained in gedit, "Files" table when moving first time on a table line.
This bug was inspired by bug #301924.
Created attachment 46282 [details] [review] proposed patch
Comment on attachment 46282 [details] [review] proposed patch I think the problem with this patch is that the output in the case where the first item is selected, and the output in the case where the first line is only focussed, is the same - the user can't tell the difference from speech. Isn't that right? I don't think that is desirable, the output should be different in the two cases, i.e. "Table 'foo', 'bar' selected", versus just "Table 'foo', 'bar'". where 'bar' is the text spoken for the line.
I agree. But this is the bug #3003637. That bug solves the problem of transition and also reports if the current line in not selected when move first time to such a line.
I think that the difference between a selected and unselected line needs to be presented every time, not just the first time the user navigates to such a line - otherwise it's too easy for the user to miss that information, and of course the selection state will often be changing as the user navigates. So I think we need a patch that solves this problem and the problem of bug 303637 - it's difficult to evaluate the patches in isolation.
Comment on attachment 46282 [details] [review] proposed patch I think it's OK to commit this patch along with a patch for 303637, once we have an approved version of a patch for that bug.
>and also reports if the current line in not selected when move first time to >such a line. My intension was to say that "unselected" state is reported when user moves to such a line, but is not reported when moves in same line. This is the behavior of the patch.
I see - so 'unselected' will be reported each time the user moves to such a line within the table? That would be the right behavior, if as you suggest "unselected" is only left unsaid when moving within the same line. Think about the case where individual cells can be selected (perhaps StarOffice calc, for instance) - does your patch make sure that selected/unselected is reported correctly when moving within the same line in that case?
do you have an answer to #8?
Created attachment 46609 [details] [review] reworked patch Same idea as in previous, but another algorithm is used to detect the range of visible cells. This is because is possible to not have a cell in the bottom right corner. This is possible if the number of displayed rows is lower than the number of possible visible rows (the vertical scroll-bar is not present or a table with only a row).
Comment on attachment 46609 [details] [review] reworked patch yes, this patch is better (as youdiscovered, getAccessibleAtPoint doesn't always return a child if the whole table is visible)
Comment on attachment 46609 [details] [review] reworked patch Patch committed.
Created attachment 47363 [details] [review] Patch to limit number of DESCENDENTS searched for ACTIVE DESCENDENT reporting In order to deal with apps that don't return a valid object for getAccessibleAtPoint() [think StarOffice 7 & 8 at the moment] (or if it fails for other reasons - the coordinates being used don't return valid data), it is important that this call not attempt to query *every* object in a large table [think StarOffice Calc]. Attached please find a patch which limits the number of child cells queried to SRL_MAX_CHILDREN_CNT. Setting this to 1000 on a 1.4GHz Pentium M means it takes about 4 seconds for Gnopernicus to respond when ALT-Tabbing to a SO Calc window. Setting it to 200 results in something that feels reasonable to me. I wouldn't want to go a whole lot lower than 200 because of tree-tables.
Comment on attachment 47363 [details] [review] Patch to limit number of DESCENDENTS searched for ACTIVE DESCENDENT reporting Hi Peter:please remove the C++ style comments, they break many compilers (including Forte, IIRC). I would make MAX_CHILDREN_CNT smaller, because it's relatively easy to get a spreadsheet whose visible area is bigger than, say, 10x20.
I think you mean "make MAX_CHILDREN_CNT *bigger*", to deal with a larger than 10x20 spreadsheet, right? As I said, we can go with something like 1000 (10x100, 32x32), but we see a ~4 second delay on a 1.4GHz Pentium M. As that system is only a couple of years old, I don't think it represents the minimum performance system, which is why I'm concerned about a number that large. On an 800x600 dpi display a maximized Calc shows 35x12 cells = 420 cells. Perhaps setting it to 600?
Since as we discussed off-list, the search in the Calc case doesn't currently result in significant end-user information (unlike the gtk+ treeview/list case), we probably can trade off speed against this capability, and limit our search further. Pick a value that seems "responsive enough" to you, but can still allow reasonably big lists (say, 200 as you originally suggested? or even 100?). Later on, if gnopernicus does a deeper search for trees (it currently doesn't descend into sub-trees), we may need to revisit that (and the whole algorithm, possibly).
Created attachment 47404 [details] [review] Updated patch - C comments, 200 constant, and debug fprintf() Bill - I put in an fprintf() to show (a) whenever srl_get_focused_cell() is called, and (b) what the return value is. I then ran this edition of srcore with the standard GTK+ file dialog, with Java SwingSet2's JTable, JTree, and JList, and of course with SO Calc. The result is that this function is only called when I tab the first time from a GTK+ table column header into a virgin table (the case of this bug), and in SO Calc whenever I focus to the spreadsheet (Alt-Tab, initial launch, or even going from editing a cell to not editing it). In every other case this function is NOT invoked (that I could find - perhaps Dublin test can find more cases). In fact, from code inspection, the following needs to be true before this function is called: a. the event is an at-spi FOCUS event b. event source has state MANAGES_DESCENDENTS c. event source implements AccessibleSelection d. event source is role TABLE e. of those items that might be in the selection (of AccessibleSelection), there exist NO such cells that are both table cells, visible, & selected If these 5 conditions are met, then this function is called. Given how narrow a place this code is called, I feel very comfortable with my patch above (removing the debugging fprintf() of course). But beyond that, if we don't want to risk this patch, I think it would be quite responsible to revert the initial patch of this bug and let this bug stay alive in exchange for not killing StarOffice Calc. I encourage Dublin test to try this patch in a wide variety of table & tree circumstances, and see what conditions you get any debug output. That will help settle the impact of this function.