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 422924 - [pending] Add more support for accessing HTML tables in Firefox
[pending] Add more support for accessing HTML tables in Firefox
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: general
2.18.x
Other All
: Normal normal
: ---
Assigned To: Orca Maintainers
Orca Maintainers
Depends on:
Blocks: 404403
 
 
Reported: 2007-03-26 09:00 UTC by Joanmarie Diggs (IRC: joanie)
Modified: 2007-05-01 20:17 UTC
See Also:
GNOME target: ---
GNOME version: 2.17/2.18


Attachments
patch to hopefully solve the problem (22.78 KB, patch)
2007-03-26 18:35 UTC, Joanmarie Diggs (IRC: joanie)
committed Details | Review
test case (1.53 KB, text/html)
2007-04-22 06:39 UTC, Joanmarie Diggs (IRC: joanie)
  Details
addition of basic support for non-uniform tables (13.66 KB, patch)
2007-04-22 07:24 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review
revised version of the previous test case: Adds captions (1.61 KB, text/html)
2007-04-22 20:36 UTC, Joanmarie Diggs (IRC: joanie)
  Details
revised version to reflect Mike's specs plus some captions changes (24.02 KB, patch)
2007-04-22 21:02 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review
Another test case (602 bytes, text/html)
2007-04-23 22:37 UTC, Joanmarie Diggs (IRC: joanie)
  Details
even better test case: Non-uniform table with multiple headers (1.54 KB, text/html)
2007-04-24 00:58 UTC, Joanmarie Diggs (IRC: joanie)
  Details
handle complex non-uniform tables (see most recent test case) (32.41 KB, patch)
2007-04-24 04:26 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review
handle blank cells AND add 4 app-unique table settings! :-) (32.87 KB, patch)
2007-04-26 14:35 UTC, Joanmarie Diggs (IRC: joanie)
committed Details | Review

Description Joanmarie Diggs (IRC: joanie) 2007-03-26 09:00:26 UTC
According to the Orca specifications:

> The letters "t" and "Shift+t" will be used to move between tables. 
> When the table is reached its title will be spoken as well as the value 
> of the cell with focus. Shift+Alt+{Left,Right,Up,Down} will be used to 
> move from cell to cell. Shift+Alt+{Home,End} will be used to move 
> between the beginning and the end of the current table. When changing
> column or row, the appropriate headings should also be read.

This is currently not implemented.
Comment 1 Joanmarie Diggs (IRC: joanie) 2007-03-26 18:35:01 UTC
Created attachment 85324 [details] [review]
patch to hopefully solve the problem

This patch adds the following functionality:

* next table
* previous table
* first cell
* last cell
* cell on left
* cell on right

It also changes how we check for table headers as we're not always getting that information exposed via at-spi in an ideal fashion.

Thoughts?
Comment 2 Willie Walker 2007-03-27 21:28:47 UTC
Yeah!  Table cell navigation is going to be done soon.  Thanks!  I think this looks pretty good, and thanks for hammering down the row/column header stuff.  Gecko certainly doesn't make it easy for us.  

The only question I have is regarding the ngettext stuff for rows and columns.  It's definitely a difficult problem (there are four possibilities of singular/plural combinations).  I say go ahead and check this in and I'll bring it up with the i18n folks when I look at bug 412200. 
Comment 3 Joanmarie Diggs (IRC: joanie) 2007-03-27 23:51:55 UTC
> The only question I have is regarding the ngettext stuff for rows and columns.

Yup, you and me both.   I'll keep an eye out on bug 412200.

Patch is checked in.  Thanks for looking it over!  Closing this one out as FIXED.
Comment 4 Joanmarie Diggs (IRC: joanie) 2007-03-28 01:16:09 UTC
I'm reopening this as I'm finding some tables that are not behaving properly. <frown>  I'll keep at it.
Comment 5 Joanmarie Diggs (IRC: joanie) 2007-03-28 18:03:52 UTC
Yesterday I checked in a patch which solved some of the tables "not behaving properly" issue.  The other issue which is still not being addressed is non-uniform tables (i.e. tables in which a cell spans multiple rows or columns).  Theoretically we should be able to use getRowColumnExtentsAtIndex(), but this may be giving us bogus values.  

Just got back from CA a little while ago and desperately need a nap; then I'll investigate this further and comment/patch here.
Comment 6 Joanmarie Diggs (IRC: joanie) 2007-03-29 19:00:40 UTC
Unfortunately, we are indeed getting bogus values from Firefox.  They fixed the basic issues surrounding get{Row, Column}AtIndex() (see https://bugzilla.mozilla.org/show_bug.cgi?id=360184).  But the presence of a cell which spans multiple rows or columns throws things off.  I added a new test case and sample output to the existing bug.  
Comment 7 Joanmarie Diggs (IRC: joanie) 2007-03-30 04:42:14 UTC
Evan separated the rowspan/colspan issue from the original bug.  The new bug can be found here: https://bugzilla.mozilla.org/show_bug.cgi?id=375934

Adjusting the summary to reflect our blocked status.
Comment 8 Willie Walker 2007-04-20 18:09:10 UTC
The Mozilla guys have marked their bug as FIXED.  Joanie, please verify.
Comment 9 Joanmarie Diggs (IRC: joanie) 2007-04-22 06:39:08 UTC
Created attachment 86769 [details]
test case
Comment 10 Joanmarie Diggs (IRC: joanie) 2007-04-22 07:24:53 UTC
Created attachment 86770 [details] [review]
addition of basic support for non-uniform tables

> The Mozilla guys have marked their bug as FIXED.  Joanie, please 
> verify.

Verified.  (Wooo hooo!)

This patch adds in basic support for non-uniform tables.  Aside from actually taking rowspan and colspan into account, this patch stores coordinates.  Imagine you're in a column 3 in a cell that spans rows 2, 3, and 4. You use Alt+Shift+Right Arrow to move to the next cell on the right.  Should you move to column 4 row 2, column 4 row 3, or column 4 row 4?  Well, that depends on what row you were in back in column 2 -- assuming you were there before you landed in column 3. :-) This patch also compares the present location (pre-move) against the stored location to be sure that obj.index matches (i.e. just in case we changed locations via the arrow keys, mouse clicking, alien abduction, whathaveyou).  Seems to work.

In using this patch with the test case I just attached, I realized I need guidance from Mike:

1. Should we read the header if we change rows/columns but the header hasn't changed?  See the second table where the header "Column Header 2 through 4" spans 3 columns.  Also see the third table where the header "Row Header 1 and 2" spans 2 rows.

2. If we navigate to a cell which spans multiple rows/columns and has multiple headers as a result, do we read them all?  Do we say anything to indicate which header is which?  See the third table in which the cell that contains "data cells 2, 6, and 10" spans rows 2 and 3 (whose header is "Row Header 1 and 2) as well as row 4 (whose header is "Row Header 3").

3. If we navigate to a cell which spans multiple rows/columns, do we want to say anything to indicate that this cell is special? (e.g. "Column Header 2 through 4 <pause> spans 3 columns")

4. If we press T/Shift T to navigate to a table and that table is a "non-uniform" table, do we want to announce that to give the user a heads up?  (e.g. "Non-uniform table with 4 rows, 5 columns")

Thanks in advance!
Comment 11 Mike Pedersen 2007-04-22 18:06:23 UTC
> 1. Should we read the header if we change rows/columns but the header hasn't
> changed?  See the second table where the header "Column Header 2 through 4"
> spans 3 columns.  Also see the third table where the header "Row Header 1 and
> 2" spans 2 rows.
> 
No, I really don't think so.
> 2. If we navigate to a cell which spans multiple rows/columns and has multiple
> headers as a result, do we read them all?  Do we say anything to indicate which
> header is which?  See the third table in which the cell that contains "data
> cells 2, 6, and 10" spans rows 2 and 3 (whose header is "Row Header 1 and 2) as
> well as row 4 (whose header is "Row Header 3").
> 
hmmmmmmmm I don't know.  I could see this getting really verbose but we should at least expose this through where am I.  What do you think? 
> 3. If we navigate to a cell which spans multiple rows/columns, do we want to
> say anything to indicate that this cell is special? (e.g. "Column Header 2
> through 4 <pause> spans 3 columns")
> 
I think yes to this one as these types of layout can get really confusing.  
> 4. If we press T/Shift T to navigate to a table and that table is a
> "non-uniform" table, do we want to announce that to give the user a heads up? 
> (e.g. "Non-uniform table with 4 rows, 5 columns")
Yes I think this would be quite useful.
Comment 12 Joanmarie Diggs (IRC: joanie) 2007-04-22 18:38:01 UTC
Cool, thanks!!  I'll get on 1, 3, and 4.

As for question 2 (should we read all of the headers that apply to the cell when a cell spans multiple rows/columns):  I agree that we should expose that through whereAmI.  And, yes, whereAmI in Firefox is something that needs some attention.... In the meantime, we need to decide what Orca will say in terms of header reading when this situation occurs.  :-)

Right now, if we approach the cell in question from the top (i.e. by Alt+Shift+Down Arrowing into it), Orca will read the first of the two headers.  If there were a fifth row from which we could approach the cell in question from the bottom, Orca would in theory read the last of the two headers.  Basically, we are treating spanned cells as if they were multiple cells so that we can maintain correct horizontal and vertical position when we leave those cells.  And it is from this perspective that we're determining what headers apply to our present location.

As for what I think.... I think that such a table is ugly and probably not all that likely to be found in the wild.  But the fact that it can exist makes me think that we need to determine how to handle it.  I also think that you are the UI guy. <grin>
Comment 13 Joanmarie Diggs (IRC: joanie) 2007-04-22 20:36:54 UTC
Created attachment 86796 [details]
revised version of the previous test case:  Adds captions

Revised test case.
Comment 14 Joanmarie Diggs (IRC: joanie) 2007-04-22 21:02:09 UTC
Created attachment 86800 [details] [review]
revised version to reflect Mike's specs plus some captions changes

Alrighty then.  This version implements everything Mike specified, namely:

1. T/Shift+T should announce when one has landed on a non-uniform table. 

2. Navigating by cell navigation commands into a cell that spans multiple rows and/or columns causes the span to be announced.  It only announces the aspect that is more than one.  For instance, if you land in a cell that occupies a single row, but three columns, you're told that the cell spans three columns but not that the rowspan is 1.  

3. Don't repeat the header if it hasn't changed.

In addition, I made two adjustments to captions:

1. I'm not sure if this is something new in Firefox or not (I believe that it is), but captions can be embedded objects now.  <sarcasm>Woo hoo</sarcasm>.  So I adjusted how we get caption text to use getDisplayedText().  Tested it on captions that were and were not embedded objects and it seems to work.

2. In the process of testing the above, I felt that it was confusing to hear the caption info after the table info when one pressed T/Shift+T.  Hearing the table info followed by the caption info followed by the cell text doesn't distinguish the caption info from the cell info.  The text that is spoken might be the caption and the first cell is empty; it might be the first cell and there might not be a caption; it might be the combination of the caption and the first cell. So here is my proposition of information presentation:

a. The caption, if any
b. The table information (non-uniform, dimensions)
c. The contents of the first cell, if any

Having tested it on the latest test case I attached, I personally think it is much clearer.  What do y'all think?  Any objections?
Comment 15 Mike Pedersen 2007-04-22 21:09:40 UTC
> 
> a. The caption, if any
> b. The table information (non-uniform, dimensions)
> c. The contents of the first cell, if any
> 
> Having tested it on the latest test case I attached, I personally think it is
> much clearer.  What do y'all think?  Any objections?
I agree I like this approach.  
> 

Comment 16 Joanmarie Diggs (IRC: joanie) 2007-04-22 21:27:10 UTC
> I agree I like this approach.  

Phew!  I mean, "thanks!" <grin>

In terms of UI guidance, that leaves my earlier question about what happens when you navigate into a cell that spans multiple rows/columns and, as a result, has multiple headers that apply to it.  And one new item:

If you drop by one of Hermann's favorite sites:  http://www.sueddeutsche.de/

You'll find a couple of tables there.  The table that has 14 rows  has 7 rows that consist of empty sections.  We could treat those cells as useless, but then it seems like there are only 7 rows which causes the user to wonder about the "14 rows" bit.  Plus, as has been pointed out by Krister and others on the list, there is value in knowing exactly how a page is laid out.  So I don't think we should treat them as useless and skip them.  I do think we should do something to indicate that we've landed in such a beast.  In the case of this table, prior to the changes for non-uniform table support, we were dead silent when we hit these cells.  Now we speak the spans.  That's something.  But what about cells that are empty and useless and do NOT span multiple cells?  (see the other table on the page that has one row and 2 columns, one of which is empty). I'm leaning towards saying "blank" in this instance.  Mike, what do you think?

Besides the above questions, is there anything else we want to include in terms of table support?  Are there any bugs you see in the current patch?
Comment 17 Joanmarie Diggs (IRC: joanie) 2007-04-23 22:37:36 UTC
Created attachment 86880 [details]
Another test case

In speaking with Mike, I came up with a test case that is very likely to occur in the wild (attached).  In discussing the test case, Mike indicated that when a given cell spans multiple headers all of those headers should be spoken, separated by pauses.
Comment 18 Joanmarie Diggs (IRC: joanie) 2007-04-24 00:58:46 UTC
Created attachment 86888 [details]
even better test case:  Non-uniform table with multiple headers

This test case covers it all, I think.  If we can do this nicely, I'd say we're golden in the HTML table dept.
Comment 19 Joanmarie Diggs (IRC: joanie) 2007-04-24 04:26:08 UTC
Created attachment 86898 [details] [review]
handle complex non-uniform tables (see most recent test case)

This version includes handling of complex non-uniform tables such as the most recently attached test case.  In that test case, not only is the table non-uniform, but it contains multiple levels/depths of headers.  For instance, the headers "week 1" and "week 2" apply to the headers for the days of the week, which in turn apply to the items in the schedule, some of which span multiple rows and/or columns.  A similar situation can be seen in the column headers.  The goal is to only speak the headers that change.  For instance, if you go from Monday to Tuesday in Week 1, don't speak week 1.  On the other hand, if you go from Friday of Week 1 to Monday of Week 2, first speak the week change, then speak the day change.  And so on, and so forth.

Mike:  This patch doesn't address the speaking of blank cells yet.  It looks like different things may constitute "blankness," and I'm starting to .... well, go blank. :-)  I'll look at that later.  In the meantime, if you could give this some hammering, including on the aforementioned test case, and let me know what you think, that would be great.  Thanks!!
Comment 20 Joanmarie Diggs (IRC: joanie) 2007-04-26 14:35:20 UTC
Created attachment 87065 [details] [review]
handle blank cells AND add 4 app-unique table settings! :-)

As the description indicates, this version detects blank cells.  The case I wasn't catching before is a cell whose contents consist of a single, non-breaking space.  Seems that some tables which are mostly serving as data tables can still have layout aspects like this in them.  Anyhoo, about the new app-unique table settings:

Mike, because no two tables are alike, and because no two of are user community are either. <grin> I have added the following settings:

1. speakCellCoordinates.  If set to True, Orca will speak the row and column number after it speaks the cell contents.  Default value is currently True.

2. speakCellSpan.  If set to True, Orca will speak the number of cells spanned when a cell spans multiple cells.  This information is spoken after the cell coordinates (or if that's disabled, after the cell contents).  Default value is currently True.

3. speakCellHeaders.  If set to True, Orca will speak the cell headers as you navigate.  Cell header information precedes the cell contents.  Default value is currently True.

4. skipBlankCells.  If set to True, Orca will not place you in a blank cell when you are navigating using Alt+Shift+Arrows.  This does not impact what happens when you use T+Shift T to jump to a table (rationale:  The upper left corner cell is often blank and starting in row 1, col 1 is good for orientation.)  Nor does it impact what happens when you use Alt Shift Home or Alt Shift End (rationale:  If you give the command to move to the very first or very last cell, you probably intend to move to the very first or very last cell).  Default value is currently False.

So, put another way, this still does everything you spec'ed out, as you spec'ed it out, adds the speaking of cell coordinates, and allows the user to turn on/off whatever works for him/her.  What do you think?

I tested this patch and the various combinations of settings on the test cases attached to this bug, plus on the tricky tables at http://www.sueddeutsche.de/.  I think it's good but given the complexity of all of this, it could use some hammering on.

In order to test the different settings for now, use orca-customizations.py:  Here are the opposites to the default:

import orca.Gecko
orca.Gecko.speakCellHeaders = False
orca.Gecko.speakCellCoordinates = False
orca.Gecko.speakCellSpan = False
orca.Gecko.skipBlankCells = True

After Rich has checked in the Gecko-related app-unique settings, I can make a new patch to add these.
Comment 21 Joanmarie Diggs (IRC: joanie) 2007-04-26 16:04:32 UTC
Mike, I just thought of another table-related setting I think we should have:

includeLayoutTables.  default value would absolutely, positively be False. <grin>  However, the user could choose to switch it to True, in which case pressing T/Shift T would move you to the next/previous table regardless of whether or not Firefox has set the layout-guess attribute or not.  What do you think?
Comment 22 Mike Pedersen 2007-04-26 17:16:19 UTC
(In reply to comment #21)
> Mike, I just thought of another table-related setting I think we should have:
> 
> includeLayoutTables.  default value would absolutely, positively be False.
> <grin>  However, the user could choose to switch it to True, in which case
> pressing T/Shift T would move you to the next/previous table regardless of
> whether or not Firefox has set the layout-guess attribute or not.  What do you
> think?
> 

(In reply to comment #21)
> Mike, I just thought of another table-related setting I think we should have:
> 
> includeLayoutTables.  default value would absolutely, positively be False.
> <grin>  However, the user could choose to switch it to True, in which case
> pressing T/Shift T would move you to the next/previous table regardless of
> whether or not Firefox has set the layout-guess attribute or not.  What do you
> think? 
>
I think this is a good idea.  I am also quite happy with the other 4 proposed additions.  I just spoke to Richh and we'd like to have you check these in so that he can add the GUI.
thanks much 

Comment 23 Joanmarie Diggs (IRC: joanie) 2007-04-26 17:52:49 UTC
Okay, I just checked in everything but the includeLayoutTables because I haven't had the chance to look at it yet.  Silly day job.... :-)  That change should be easy, but any change to isLayoutOnly() or isUselessObject() impacts everything in Gecko.py so I'm not going to rush that one.   Hope that's okay!

Rich thanks for doing this.  'Tis going to be slick!!
Comment 24 Joanmarie Diggs (IRC: joanie) 2007-05-01 20:17:42 UTC
Closing as FIXED.