Bug 57826 - [GTK] Implement AccessibilityUIElement::cellForColumnAndRow in DRT
Summary: [GTK] Implement AccessibilityUIElement::cellForColumnAndRow in DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 57463
Blocks: 30796 57854
  Show dependency treegraph
 
Reported: 2011-04-05 04:13 PDT by Mario Sanchez Prada
Modified: 2011-06-14 18:43 PDT (History)
2 users (show)

See Also:


Attachments
Patch proposal + Updated layout test (9.81 KB, patch)
2011-04-05 04:35 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff
Patch proposal + Updated layout tests (11.37 KB, patch)
2011-04-05 07:45 PDT, Mario Sanchez Prada
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2011-04-05 04:13:00 PDT
We need this feature to be able to unskip the following tests:

 accessibility/aria-tables.html
 accessibility/table-cells.html
 accessibility/table-cell-spans.html

Also, specially thinking of the first test of the list, prior to this we need to get ARIA tables being exposed in a consistent way compared to normal HTML tables (so we can actually inspect their cells), which is what bug 57463 is about. Thus, marking this new bug as dependent on the resolution of that other one.
Comment 1 Mario Sanchez Prada 2011-04-05 04:13:46 PDT
This blocks the rich internet apps metabug
Comment 2 Mario Sanchez Prada 2011-04-05 04:35:42 PDT
Created attachment 88201 [details]
Patch proposal + Updated layout test

Attaching a simple patch (assuming 57463 fixed) + some changes in the aria-tables.html layout test to allow unskipping it in GTK.

It might seem weird to have changed the html file to be able to unskip it, but I think it was a good thing (providing semantics keep being the same) since that way we don't need to have different HTML file for each platform, just different expected files with the different strings got in the output for the role names. See more fine-grained detail in the ChangeLog entry.

Last, I'd like to have unskipped as well the other two tests, but it seems that would need some more changes somewhere else, so I'm not doing it right now.
Comment 3 Mario Sanchez Prada 2011-04-05 04:37:13 PDT
Adding Chris Fleizach to CC, as the currently proposed test changes the implementation of a cross-platform layout test, changes the expected file for the mac platform and skip the aria-tables.html test in the mac-leopard platform.
Comment 4 Mario Sanchez Prada 2011-04-05 07:45:25 PDT
Created attachment 88233 [details]
Patch proposal + Updated layout tests

(In reply to comment #2)
> [...]
> Last, I'd like to have unskipped as well the other two tests,
> but it seems that would need some more changes somewhere else,
> so I'm not doing it right now.

Actually, I could have skipped the table-cells.html one by just adding some new expectations :P

Patch updated!
Comment 5 chris fleizach 2011-04-05 08:28:00 PDT
Comment on attachment 88233 [details]
Patch proposal + Updated layout tests

View in context: https://bugs.webkit.org/attachment.cgi?id=88233&action=review

r=me with minor wording changes

> LayoutTests/ChangeLog:10
> +        Also, done changes inside the aria-tables.html layout test, in

This sentence is worded awkwardly. What do you mean by "Also, done changes"

> LayoutTests/ChangeLog:16
> +        just print role names instead of comparing their actual values

..."just prints..."

> LayoutTests/ChangeLog:17
> +        with the expected ones in one specific platform (Mac)

"...just prints the..."
Sorry for duplicate comment. Can't edit comment on iphone

> Tools/ChangeLog:13
> +

-> Implemented by relying on....
Comment 6 Mario Sanchez Prada 2011-04-05 08:36:11 PDT
(In reply to comment #5)
> (From update of attachment 88233 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88233&action=review
> 
> r=me with minor wording changes


Thanks! I'll update those before committing as soon as the bug 57463 gets reviewed+ then.

> > LayoutTests/ChangeLog:10
> > +        Also, done changes inside the aria-tables.html layout test, in
> 
> This sentence is worded awkwardly. What do you mean by "Also, done changes"

Sorry about that. I meant something like this:

"Some changes were made also inside..."

That is, that I also touched the code of the layout test itself, not just updated expectations. I'll re-write that before committing.
 
> > LayoutTests/ChangeLog:16
> > +        just print role names instead of comparing their actual values
> 
> ..."just prints..."

Ok

> > LayoutTests/ChangeLog:17
> > +        with the expected ones in one specific platform (Mac)
> 
> "...just prints the..."
> Sorry for duplicate comment. Can't edit comment on iphone

No problem. Quite impressed about getting my patches reviewed from a phone, though!

> > Tools/ChangeLog:13
> > +
> 
> -> Implemented by relying on....

Yes, sorry about that.
Comment 7 Mario Sanchez Prada 2011-04-11 11:55:02 PDT
Committed r83465: <http://trac.webkit.org/changeset/83465>
Comment 8 Ryosuke Niwa 2011-06-14 18:43:21 PDT
It seems like accessibility/aria-tables.html is failing on the bot:
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r88877%20(23374)/results.html

--- /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/accessibility/aria-tables-expected.txt	2011-06-14 17:36:05.233910440 -0700
+++ /home/slave/webkitgtk/gtk-linux-64-debug/build/layout-test-results/accessibility/aria-tables-actual.txt	2011-06-14 17:36:05.221346865 -0700
@@ -13,8 +13,8 @@
 AXRole: table
 AXRole: table
 AXRole: table cell
+AXRole: unknown
 AXRole: table cell
-AXRole: table cell
-AXRole: table cell
+AXRole: unknown
 Test passed