Bug 35418 - [Gtk] Every table, including layout tables, should be exposed as a table
: [Gtk] Every table, including layout tables, should be exposed as a table
Status: RESOLVED FIXED
: WebKit
Accessibility
: 528+ (Nightly build)
: PC Linux
: P2 Normal
Assigned To:
:
: Gtk
:
: 25531
  Show dependency treegraph
 
Reported: 2010-02-25 20:13 PST by
Modified: 2010-02-26 12:30 PST (History)


Attachments
proposed fix (3.54 KB, patch)
2010-02-25 20:28 PST, Joanmarie Diggs (irc: joanie)
no flags Review Patch | Details | Formatted Diff | Diff
proposed fix - style nit corrected (3.54 KB, patch)
2010-02-25 20:35 PST, Joanmarie Diggs (irc: joanie)
xan.lopez: review-
Review Patch | Details | Formatted Diff | Diff
proposed fix - nuke unneed parens (3.54 KB, patch)
2010-02-25 23:36 PST, Joanmarie Diggs (irc: joanie)
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-02-25 20:13:27 PST
By default, WebKit only exposes "data tables" as tables; "layout tables" get turned into a series of panels (GroupRole). And some of these subsequently get ignored as being unnecessary hierarchical elements.

ATs providing access to the GNOME desktop expect all tables to be exposed as tables. It is then up to the individual AT to determine if a given object of ATK_ROLE_TABLE should be treated as a proper table or not. (One of the ways it does this, at least for Gecko, is through an object attribute indicating that a given table is suspected as being a layout table. I'll open a separate bug for this.)

One example of the impact of this issue is downstream: https://bugs.launchpad.net/ubuntu/+source/software-center/+bug/433104. The primary issue is that focus is being given to a table cell which WebKit is treating as a panel -- and ignoring. This problem can largely be addressed simply by exposing the software center's "layout table" as a proper table, at which point Orca will (for the most part) do the RightThing(tm) automatically.
------- Comment #1 From 2010-02-25 20:28:31 PST -------
Created an attachment (id=49562) [details]
proposed fix

This causes Orca to (for the most part*) do the right thing automatically when a user arrows within the layout table of the Software Center.

*Note that when focus is initially given to the table, Orca will speak the entire contents of the table. This is, I believe, due to the problematic table hierarchy (aka bug 30895) which I'll tackle next.

Also note that I'm not including a test at this point for a couple of reasons:

1. The most logical type of test for this situation/bug is, I believe, the same sorts of tests being worked on as part of bug 34449.

2. There's no point IMHO in creating a test which will largely need to be changed just as soon as I fix the problematic table hierarchy.

Please review. Thanks!
------- Comment #2 From 2010-02-25 20:30:07 PST -------
Attachment 49562 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp:55:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #3 From 2010-02-25 20:35:39 PST -------
Created an attachment (id=49563) [details]
proposed fix - style nit corrected

Added the whitespace to the comment as per the stylebot error.

Please review. Thanks!
------- Comment #4 From 2010-02-25 23:14:29 PST -------
(From update of attachment 49563 [details])
>+#if PLATFORM(GTK)
>+    // Gtk ATs expect all tables, data and layout, to be exposed as tables.
>+    if (node && (node->hasTagName(tdTag)))
>+        return CellRole;
>+
>+    if (node && (node->hasTagName(tableTag)))
>+        return TableRole;
>+#endif

The parenthesis around the second term in each if are not reeded.

Other that that looks good, if you can attach a new patch with that detail fixed I'll r+ it.
------- Comment #5 From 2010-02-25 23:36:16 PST -------
Created an attachment (id=49565) [details]
proposed fix - nuke unneed parens

> The parenthesis around the second term in each if are not reeded.

D'oh!

Thank you Xan. :-)

> Other that that looks good, if you can attach a new patch with that detail
> fixed I'll r+ it.

Awesome. I appreciate it.
------- Comment #6 From 2010-02-25 23:38:37 PST -------
(From update of attachment 49565 [details])
Bam!
------- Comment #7 From 2010-02-26 12:08:12 PST -------
(From update of attachment 49565 [details])
Clearing flags on attachment: 49565

Committed r55297: <http://trac.webkit.org/changeset/55297>
------- Comment #8 From 2010-02-26 12:08:16 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #9 From 2010-02-26 12:10:41 PST -------
This patch was missing a test case.  Any reason one could not be provided?
------- Comment #10 From 2010-02-26 12:30:20 PST -------
(In reply to comment #9)
> This patch was missing a test case.  Any reason one could not be provided?

Please see comment 1.