One can navigate to the label via Ctrl+Right to have it read, but the Insert+F11 feature to read table cell or read table row doesn't seem to help us make Orca always read the entire row.

Created attachment 74028 [details] [review]
Patch to hopefully fix the problem.

This isn't so minor. Explanation to follow...

The problem here was that the chunk of code:

        action = obj.action
        if action:
            for i in range(0, action.nActions):
                debug.println(debug.LEVEL_FINEST,
                    "speechgenerator.__getTableCellUtterances " \
                    + "looking at action %d" % i)
                if action.getName(i) == "toggle":
                    obj.role = rolenames.ROLE_CHECK_BOX
                    utterances.extend(self._getSpeechForCheckBox(obj,
                                                            already_focused))
                    obj.role = rolenames.ROLE_TABLE_CELL
                    break

really needed to be done for every table cell in the row if
settings.readTableCellRow was True.

Combine that with not doing the chunk of code in _getSpeechForTableCell()
that handles speaking all the rows if the initial table cell had an action
which generated some initial utterances; i.e.

        if (len(utterances) == 0) and (not already_focused):

was wrong.

I've reworked it by adding in a new __getTableCellUtterances()
routine that will get called in three places in _getSpeechForTableCell().

I'm not happy with the name __getTableCellUtterances(). Can you think
of a better one?

Also Will could you cast your eyes over this and tell me if it looks okay.
Mike, could you try applying this patch to speechgenerator.py and see if
table cells still work okay for you? Alternatively I can email you a new
version of this file.

Thanks.

PS: It also seems to have fixed up speaking all the table cells in the
    gtk-demo Tree View ListStore demo as well.

Aha!  Good stuff, Rich.  The individual generators tend to be for a single object rather than a group of objects. A place where grouping is done is in the public braillegenerator.py:getBrailleRegions method, and that's the optional grouping of menu items, menu bars, and tabs.  The handling of the grouping is kind of ugly, IMO, because it is done in a separate piece of code that probably should not have knowledge of the object it is dealing with.

For this patch, I think the 'expanded'/'unexpanded' state may also need to be treated the same as the 'checked'/'unchecked' state.  In addition, braille may also need to be updated to so something similar.

As for an alternative way to do this, would it be possible to keep the _getSpeechForTableCell method as one that merely returns the right thing for a single cell, and then create a _getSpeechForTableCellRow method that just keeps extending a set of utterances using the results of calls to _getSpeechForTableCell (i.e., extend utterances outside the method versus passing it in and extended it inside the method)?  

The _getSpeechForTableCellRow method could be called by moving the "speakAll" logic somewhere else (kind of like the ugly way in which menu/tabs/etc are grouped) or it could keep the same "speakAll" logic and you could map the method as follows:

      self.speechGenerators[rolenames.ROLE_TABLE_CELL]          = \
             self._getSpeechForTableCellRow

What do you think?

Will

I think it's fine, and I'll adjust accordingly now, except I didn't
understand the bit:

  The _getSpeechForTableCellRow method could be called by moving the "speakAll"
  logic somewhere else (kind of like the ugly way in which menu/tabs/etc are
  grouped) or it could keep the same "speakAll" logic and you could map the
  method as follows:

      self.speechGenerators[rolenames.ROLE_TABLE_CELL]          = \
             self._getSpeechForTableCellRow

could you explain further?

Well...I was just trying to think of a couple different ways of tackling this.  The one I think I prefer is this, and it's very very close to what your patch is already doing:

Have _getSpeechForTableCell behave like any of the other methods in speechgenerator (i.e., it merely returns a set of utterances).  Make sure this method incoporates the logic for both the checked and expanded states.

Put the readTableCellRow/speakAll logic for reading a single cell or an entire row in the new _getSpeechForTableCellRow method.  If only a single cell is to be spoken, you merely return the results of _getSpeechForTableCell.  If speaking the entire row is enabled, you concatenate the results of calls to _getSpeechForTableCell for each cell in the row.

Make the generator for a table cell be _getSpeechForTableCellRow:

       self.speechGenerators[rolenames.ROLE_TABLE_CELL]          = \
             self._getSpeechForTableCellRow

There may (or may not) be some related work that has to be done in the StarOffice.py script - it may be that some of the stuff its locusOfFocusChanged method (the stuff in section 4) can be removed.

Created attachment 74079 [details] [review]
Improved patch to fix the problem.

I've revised the patch per Will's suggestions.
Patch does the same logic for brailling table cells too.
I've removed the special custom scripting from the StarOffice.py
script for handling table cells in scalc. Because of this, I've
made one small new changes to the "default" handling of table cells
(both speaking and brailling):

If you have settings.readTableCellRow set to True, it'll only 
speak/braille the table cells if they are showing.

This seems to work very nicely with everything I've tried it with.

Will/Mike, could you try this out any tell me whether it's okay before I
commit it?

Thanks.

Change checked into CVS HEAD. Closing as FIXED. If you find problems
with it, please let me know.

Just want to drop in a note saying I tested this with gtk-demo, gnome-system-monitor, and oocalc.  Looks good to me.  The only oddity is hearing each cell name spoken when read by row is enabled.  I'm guessing that's actually a good thing, but Mike should be the one to let us know.  

In addition should there be some sort of separator between cells on the braille display?  For example, when settings.enableBrailleGrouping is True, all the items in a menu will be displayed on the line, separated by " _ ".

> In addition should there be some sort of separator between cells on the braille
> display?  For example, when settings.enableBrailleGrouping is True, all the
> items in a menu will be displayed on the line, separated by " _ ".

Just to make sure I understand: this would be a new feature suggestion, not
a regression, correct? 

Yeah - probably a new RFE and not a bug.  Before making an RFE, though, let's see what our UI designer (Mike) has to say.  :-)

I'm not really in favor of adding a marker between table cells because it will really affect how much will fit on a display without panning.  If we do this we will lose two braille cells per table cell.