GNOME Bugzilla – Bug 486897
Where Am I doesn't present row/column headers
Last modified: 2008-07-22 19:33:53 UTC
When in a table, it would be nice if "Where Am I" presented row/column header information. This is an enhancement to the spec and Mike needs to spec out where the information should appear in speech.
I think the header info should come just before the content of the cell. For example if you were on a cell containing the file name foo you might hear filename just before cell: foo.
Created attachment 99999 [details] [review] take 1 This patch does the "filename cell: foo" thang. It also makes a couple of other changes/fixes that I think are very relevant to presenting table cell information: 1. Currently we don't speak the cell we're in (forget about its headers and whether or not it's toggle-able for the moment). We say "cell" and then speak the entire row. Given a single-columned table (or one with multiple columns comprised of a bunch of checkboxes), one might not notice this. I assume we actually want to speak the cell we're in during a where am I. 2. Currently we don't indicate that we're in a table, though the example in the spec indicates we should do so. So now we speak the parent role (table, tree table), we speak the column header, we speak the cell contents, and *then* we go off looking for the row contents. Questions for Mike: 1. Thoughts on the above changes? 2. I haven't yet addressed items with row headers. Do they exist "in the wild"? Assuming they do, exactly what should be said given an object which has a row header as well as a column header? 3. Having tried my changes, I liked what we were doing right up to the point where we went off to get the row contents. Properly speaking the cell information and then speaking the full row seemed like an awful lot of verbiage without some indication that Orca was done telling me about my immediate location (table cell) and had moved on to describing my surroundings. <smile> What do you think? Would it be worth changing things so that basic Where Am I gives you information about the cell only (perhaps tacking on the coordinates for good measure) and detailed Where Am I gives you the row information?
Created attachment 100002 [details] [review] Slight variant - change needed as part of the fix to bug 486899 Looking at bug 486899, we're going to need to get the cell a bit differently so might as well do it here. :-)
> Questions for Mike: > 1. Thoughts on the above changes? These changes all sound quite good to me. > 2. I haven't yet addressed items with row headers. Do they exist "in the > wild"? Assuming they do, exactly what should be said given an object which has > a row header as well as a column header? If they exist, I've never seen them before. > 3. Having tried my changes, I liked what we were doing right up to the point > where we went off to get the row contents. Properly speaking the cell > information and then speaking the full row seemed like an awful lot of verbiage > without some indication that Orca was done telling me about my immediate > location (table cell) and had moved on to describing my surroundings. <smile> > What do you think? Would it be worth changing things so that basic Where Am I > gives you information about the cell only (perhaps tacking on the coordinates > for good measure) and detailed Where Am I gives you the row information? I really don't see the need for any further changes here. I think for the amount of times a user will likely do this the current implementation is just fine.
> > 2. I haven't yet addressed items with row headers. Do they exist "in the > > wild"? Assuming they do, exactly what should be said given an object which has > > a row header as well as a column header? > If they exist, I've never seen them before. If we wanted to try to emulate these, we could use a spreadsheet in OOo and use dynamic row headers. But...that more of a fix to the OOo script than the default where am I functionality. Mike, what should the OOo where am I do in this case (I think it might need updating)?
I think it would be fine to simply speak the row header followed by the column header.
Joanie - how are we with this one? Is it something that can be done by 2.21.5 (Jan 14)? In addition, I think some defensive stuff might need to be done around here: + table = parent.queryTable() + column = table.getColumnAtIndex(obj.getIndexInParent()) + text = table.getColumnHeader(column).name + utterances.append(text) I think getColumnHeader may not be guaranteed to return something, especially if there is no column header. In addition, is .name the thing to grab, or should we be looking to getDisplayedText?
(In reply to comment #7) > Joanie - how are we with this one? Is it something that can be done by 2.21.5 We stopped working on it in order to do performance enhancements. :-) Yeah, I'll tackle this one soon.
Created attachment 102769 [details] [review] revision 3 This includes the changes Will suggested (using getDisplayedText() via _getObjName() and checking for a header). I also added the code for getting this information for a row header (should it exist) and speaking it before the column header. It's hard to test it without row headers, but it doesn't break anything when called. :-) As for StarOffice.py.... Mike, did you mean that we should also speak the row header and then the column header? If so, what else should we change about what the StarOffice script currently does in a spreadsheet? Right now it speaks the role ("cell"), then "column" followed by the column number, then "row" followed by the row number, then the cell contents.
I think this looks good, though it may also end up requiring a number of changes to the assertions in the regression tests that do "Where Am I" operations in tables. Do you have an idea of the number of changes that might need to be made?
This patch seems good to me.
Thanks Mike. As discussed in team meeting yesterday, I'm afraid there is still information needed from you w.r.t. this bug: (In reply to comment #9) > As for StarOffice.py.... Mike, did you mean that we should also speak the row > header and then the column header? In other words, you indicated in comment #6 that we should speak the row header and then the column header in general. However, that's not what we currently do in StarOffice.py. There we speak the column number followed by the row number. So should we reverse the order for the generic code (which wouldn't be noticeable given that I have yet to find a single "generic" row header)? Or should we reverse the StarOffice.py handling? Comment #9 goes on to ask: > If so, what else should we change about what the StarOffice script > currently does in a spreadsheet? Right now it speaks the role > ("cell"), then "column" followed by the column number, then "row" > followed by the row number, then the cell contents. In other words, if we happen to have dynamic headers set and wish to speak those headers, do we speak them *instead* of the column and row numbers or in addition to? What *exactly* should Orca say in this instance? Thanks!
> > (In reply to comment #9) > > As for StarOffice.py.... Mike, did you mean that we should also speak the row > > header and then the column header? > > In other words, you indicated in comment #6 that we should speak the row header > and then the column header in general. However, that's not what we currently > do in StarOffice.py. There we speak the column number followed by the row > number. So should we reverse the order for the generic code (which wouldn't be > noticeable given that I have yet to find a single "generic" row header)? Or > should we reverse the StarOffice.py handling? > Lets leave Openoffice as it is in case there are users used to the way it is. Lets make the orca default follow OpenOffice. > Comment #9 goes on to ask: > > > If so, what else should we change about what the StarOffice script > > currently does in a spreadsheet? Right now it speaks the role > > ("cell"), then "column" followed by the column number, then "row" > > followed by the row number, then the cell contents. > > In other words, if we happen to have dynamic headers set and wish to speak > those headers, do we speak them *instead* of the column and row numbers or in > addition to? What *exactly* should Orca say in this instance? I'm in favor of keeping the numbers as wel. I think they should be the last thing spoken.
Created attachment 105102 [details] Spreadsheet with dynamic row and column headers. As far as I can see, without me doing any changes to fix the where-am-i problem, dynamic row and column headers is currently broken: Typing Insert-C. ... Traceback (most recent call last):
+ Trace 189012
consumed = self.function(script, inputEvent)
table = self.getTable(orca_state.locusOfFocus)
return table
Investigating that now. Looks like another yak is going to get shaved.
Created attachment 105105 [details] [review] Patch to adjust the where-am-i output to speak the dynamic row and column headers (if present). Note that you'll need the very latest version of Orca from SVN trunk as the dynamic row and column headers where broken until a few minutes ago.
Created attachment 105108 [details] [review] combined patch plus updated gtk-demo tests Thanks Rich!! This is the combined version, plus three updated gtk-demo tests.
(In reply to comment #16) > Created an attachment (id=105108) [edit] > combined patch plus updated gtk-demo tests > > Thanks Rich!! > > This is the combined version, plus three updated gtk-demo tests. I think this looks good. In playing with, I noticed that we're getting the following with gtk-demo, but not with soffice: 6. current row (regardless of speak cell/row setting) Comment #2 and comment #4 contain the question an answer for why we're outputting so much information, but I'm curious if we might consider not outputting the entire row. It sure seems like a lot of info. In addition, taking it out of the default where am I would make things consistent across OOo and GTK+. If this whole point is minimal, however, I'm OK with checking the current patch in as is (THANKS SO MUCH FOR UPDATING THE TESTS!!!) and living with the inconsistency.
Created attachment 105348 [details] [review] don't speak the full row unless doing a "detailed" whereAmI Will, I personally think the extra row verbiage makes things confusing. So as a proof of concept.... This version does not speak the full row at the end of a basic whereAmI, but does speak it at the end of a detailed whereAmI. I think this improves things quite a bit. This patch also includes updates to two of the three gtk-demo tests. For some reason, even without the patch, role_table.py has a number of unexpected failures which makes it difficult to update the test. :-( Will, assuming that we all agree to this change, could you please verify that the tests I did revise work as expected for you? And if you wouldn't mind updating role_table.py, that would be great as I'm just not sure what's up with Hardy these days. What I would update is the following: 1. Basic whereAmI will need to have the row info removed from the output. 2. A detailed whereAmI test should be added. Pylinted already.
Created attachment 105351 [details] [review] previous version with a revised role_table.py from Will (thanks!) Rich, when you get a chance, could you please do the following: Run the gtk-demo/role_table.py test in its current/official form and if that works for you, apply this patch and see if it still works? :-) Because the test in question totally fails for me, and because Will sees differences related to whitespace, neither of us can really say that the test is reliable/valid. Thanks in advance!!
Before applying the patch: $ ./runone.sh ../keystrokes/gtk-demo/role_table.py gtk-demo 0 starting test application gtk-demo ... Test 1 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table initial focus Test 2 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table Where Am I Test 3 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table down one line Test 4 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table Where Am I (again) Test 5 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Turn row reading off Test 6 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table up to packages of noodles Test 7 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table up to bottles of coke SUMMARY: 7 SUCCEEDED and 0 FAILED (0 UNEXPECTED) of 7 for ../keystrokes/gtk-demo/role_table.py /usr/bin/orca: line 92: 19796 Killed /usr/bin/python -c "import orca.orca; orca.orca.main()" "$ARGS" ./runone.sh: line 184: 19803 Killed $APP_NAME $ARGS $PARAMS $ After applying the patch I get: $ ./runone.sh ../keystrokes/gtk-demo/role_table.py gtk-demo 0 starting test application gtk-demo ... Macaroon timeout: Wait for tree table to be focused Test 1 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table initial focus Test 2 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table Where Am I Test 3 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table down one line Test 4 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table Where Am I (again) Test 5 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Turn row reading off Test 6 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table up to packages of noodles Test 7 of 7 SUCCEEDED: ../keystrokes/gtk-demo/role_table.py:Table up to bottles of coke SUMMARY: 7 SUCCEEDED and 0 FAILED (0 UNEXPECTED) of 7 for ../keystrokes/gtk-demo/role_table.py /usr/bin/orca: line 92: 18788 Killed /usr/bin/python -c "import orca.orca; orca.orca.main()" "$ARGS" ./runone.sh: line 184: 18795 Killed $APP_NAME $ARGS $PARAMS $ Yes, I know the "before" pid numbers are higher. I ended up forgetting to do the "before" test first, and just backed out the patch, reinstalled and ran it again. In short, the tests look good both with or without the patch.
The complete gtk-demo regression tests worked fine for me (modulo whitespace differences in Solaris) and things seem to pylint well, though I admit I didn't pylint Gecko.py since it's bedtime. I didn't test with StarOffice. But -- if you did (and you even want to write some regression tests for it), and are comfortable with this patch, I think it's a definite improvement and say commit. Thanks!
Created attachment 105556 [details] [review] previous version with an updated test for StarOffice We had a perfectly good regression test for the implementation of dynamic row and column headers (oocalc/bug_361167.py). It just needed the whereAmI test added -- along with the use of assertions. :-) So I updated that test rather than make another one. Patch committed. Moving to pending.