After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 484499 - Orca should not braille the node level for every cell in row
Orca should not braille the node level for every cell in row
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: braille
2.20.x
Other All
: Normal normal
: 2.20.1
Assigned To: Joanmarie Diggs (IRC: joanie)
Orca Maintainers
Depends on:
Blocks: 404409
 
 
Reported: 2007-10-07 20:24 UTC by Joanmarie Diggs (IRC: joanie)
Modified: 2007-10-10 20:28 UTC
See Also:
GNOME target: ---
GNOME version: 2.19/2.20


Attachments
proposed patch (1.27 KB, patch)
2007-10-07 20:30 UTC, Joanmarie Diggs (IRC: joanie)
committed Details | Review

Description Joanmarie Diggs (IRC: joanie) 2007-10-07 20:24:01 UTC
Steps to reproduce:

1. Launch Thunderbird
2. Arrow among the messages in your inbox

Expected results:  Orca would tack on the node level at the very end of the braille region.

Actual results:  Orca tacks on the node level at the end of each and every cell.
Comment 1 Joanmarie Diggs (IRC: joanie) 2007-10-07 20:30:54 UTC
Created attachment 96845 [details] [review]
proposed patch

Currently, _getBrailleRegionsForTableCell() is what tacks on the node.  If we only want to tack it on at the end, we should be able to do that in _getBrailleRegionsForTableCellRow() instead -- right??  

I grepped for _getBrailleRegionsForTableCell() and found it was called only by _getBrailleRegionsForTableCellRow().  (StarOffice.py has its own methods).  Therefore, I think this is a safe change that solves the problem.

Will?
Comment 2 Willie Walker 2007-10-08 12:33:57 UTC
> I grepped for _getBrailleRegionsForTableCell() and found it was called only by
> _getBrailleRegionsForTableCellRow().  (StarOffice.py has its own methods). 

This is probably OK, since the self.brailleGenerators table of braillegenerator.py  currently does not include _getBrailleRegionsForTableCell.  It used to, but I believe that was replaced with _getBrailleRegionsForTableCell when the read row work was done.

As an alternative to moving the TREE LEVEL block from _getBrailleRegionsForTableCell to _getBrailleRegionsForTableCellRow, I wonder if adding a new parameter (e.g., includeTreeLevel=True) to _getBrailleRegionsForTableCell might be better?  With this, _getBrailleRegionsForTableCell could still behave as a standalone generator (if we wanted it to behave that way at some point in the future).  The behavior would be that _getBrailleRegionsForTableCellRow would set the parameter to prevent the output of TREE LEVEL as appropriate.

Joanie, what do you think?


Comment 3 Joanmarie Diggs (IRC: joanie) 2007-10-09 05:27:57 UTC
I think that makes a lot of sense.  It's gotten late on me, but I'll implement your suggestion tomorrow.  Thanks!
Comment 4 Joanmarie Diggs (IRC: joanie) 2007-10-09 16:57:50 UTC
So it's now tomorrow. :-)  I still think it makes a lot of sense.  :-)

But....

What I wound up doing is make includeTreeLevel=False when we were speaking/brailling the entire row -- with the exception of the last column.  That seemed to work swimmingly with the things I tested.  However, there was the possibility of the last column not having STATE_SHOWING in which case _getBrailleRegionsForTableCell() would never get called for it with includeTreeLevel == True.  I was chatting with Will about how we can most efficiently address that scenario.  At the end of the conversation, Will suggested that the original patch was fine and if one day braillegenerator.py once again includes _getBrailleRegionsForTableCell() we'll address this stuff then.  

Therefore, I'm adding our new [testing required] to the summary.
Comment 5 Mike Pedersen 2007-10-10 15:03:52 UTC
This is so much better.  thanks much 
Comment 6 Joanmarie Diggs (IRC: joanie) 2007-10-10 15:26:51 UTC
Thanks Mike.  Will, okay to commit?
Comment 7 Willie Walker 2007-10-10 16:15:41 UTC
Please commit to trunk and the gnome-2-20 branch and feel free to close as FIXED.  Thanks!
Comment 8 Joanmarie Diggs (IRC: joanie) 2007-10-10 20:28:44 UTC
Thanks guys.  Committed to both trunk and the gnome-2-20 branch.  Closing as FIXED.