|Summary:||[Gtk] Every table, including layout tables, should be exposed as a table|
|Product:||WebKit||Reporter:||Joanmarie Diggs <jdiggs>|
|Severity:||Normal||CC:||apinheiro, commit-queue, sam, walker.willie, webkit.review.bot, xan.lopez|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Joanmarie Diggs 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 Joanmarie Diggs 2010-02-25 20:28:31 PST
Created attachment 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 WebKit Review Bot 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]  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 Joanmarie Diggs 2010-02-25 20:35:39 PST
Created attachment 49563 [details] proposed fix - style nit corrected Added the whitespace to the comment as per the stylebot error. Please review. Thanks!
Comment 4 Xan Lopez 2010-02-25 23:14:29 PST
Comment on attachment 49563 [details] proposed fix - style nit corrected >+#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 Joanmarie Diggs 2010-02-25 23:36:16 PST
Created attachment 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 Xan Lopez 2010-02-25 23:38:37 PST
Comment on attachment 49565 [details] proposed fix - nuke unneed parens Bam!
Comment 7 WebKit Commit Bot 2010-02-26 12:08:12 PST
Comment on attachment 49565 [details] proposed fix - nuke unneed parens Clearing flags on attachment: 49565 Committed r55297: <http://trac.webkit.org/changeset/55297>
Comment 8 WebKit Commit Bot 2010-02-26 12:08:16 PST
All reviewed patches have been landed. Closing bug.
Comment 9 Sam Weinig 2010-02-26 12:10:41 PST
This patch was missing a test case. Any reason one could not be provided?