Bug 30896 - [Gtk] Implement atk_table_get_column_header
Summary: [Gtk] Implement atk_table_get_column_header
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: Gtk
Depends on: 36128
Blocks: 25531
  Show dependency treegraph
 
Reported: 2009-10-28 22:25 PDT by Joanmarie Diggs (irc: joanie)
Modified: 2010-06-25 13:23 PDT (History)
5 users (show)

See Also:


Attachments
test case (309 bytes, text/html)
2010-06-14 08:26 PDT, Joanmarie Diggs (irc: joanie)
no flags Details
Screenshot with Accerciser showing expected results (179.73 KB, image/png)
2010-06-14 08:28 PDT, Joanmarie Diggs (irc: joanie)
no flags Details
possible fix? (1.27 KB, patch)
2010-06-14 08:33 PDT, Joanmarie Diggs (irc: joanie)
no flags Details | Formatted Diff | Diff
Full patch proposal (with unit test) (9.92 KB, patch)
2010-06-23 10:27 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff
Some cleanup ON TOP of the full patch proposal (5.71 KB, patch)
2010-06-23 11:18 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs (irc: joanie) 2009-10-28 22:25:17 PDT
The fixes for bug 25534 implemented the bulk of AtkTable. Support for atk_table_get_column_header was removed due to being broken:

* when a cell was returned, it did claim to be in the proper row, but
* it didn't have a child of ROLE_TEXT with the cell contents as expected, and
* often a cell wasn't returned

What's puzzling to me is that the nearly identical implementation for atk_table_get_row_header works like a charm. Unless I'm just missing something (a perfectly reasonably possibility), this might be due to "non-traditional" table hierarchy exposed to Atk apps by WebKitGtk (for which I've opened bug 30895).
Comment 1 Joanmarie Diggs (irc: joanie) 2010-06-14 08:26:32 PDT
Created attachment 58654 [details]
test case
Comment 2 Joanmarie Diggs (irc: joanie) 2010-06-14 08:28:03 PDT
Created attachment 58655 [details]
Screenshot with Accerciser showing expected results

(hoping a picture is worth a thousand words. :-) )
Comment 3 Joanmarie Diggs (irc: joanie) 2010-06-14 08:33:03 PDT
Created attachment 58657 [details]
possible fix?

Not flagging for review for a couple of reasons:

1. No test.

2. Mario had indicated an interest in working on some of these bugs blocking Orca support (yay! thanks!!). One less thing to worry about. :-) So really, I'm attaching this in the hopes that he'll take this bug, refine the patch as needed, and propose a proper fix with a test. :-)
Comment 4 Mario Sanchez Prada 2010-06-14 10:58:29 PDT
(In reply to comment #3)
> Created an attachment (id=58657) [details]
> possible fix?
> 
> Not flagging for review for a couple of reasons:
> 
> 1. No test.
> 
> 2. Mario had indicated an interest in working on some of these bugs blocking Orca support (yay! thanks!!). One less thing to worry about. :-) So really, I'm attaching this in the hopes that he'll take this bug, refine the patch as needed, and propose a proper fix with a test. :-)

That's the plan but first I need to finish some pending things on other bugs, such as bug 36128, which is btw blocker for this one (so it makes sense to finish work first there :-))
Comment 5 Mario Sanchez Prada 2010-06-23 10:27:22 PDT
Created attachment 59528 [details]
Full patch proposal (with unit test)

Hi, after spending most of the time struggling with another bug found while working on this one [*], now that one is fixed I was finally able to finish Joanmarie's work by adding a new unit test in testatk, as well as fixing some small issues in get_row_header() to keep it coherent with get_column_header(), and make it work well.

Hence, attaching now the patch that fixes this stuff. Hope you like it.

[*] https://bugs.freedesktop.org/show_bug.cgi?id=28659
Comment 6 Xan Lopez 2010-06-23 11:13:12 PDT
Comment on attachment 59528 [details]
Full patch proposal (with unit test)

Looks good.
Comment 7 Mario Sanchez Prada 2010-06-23 11:18:19 PDT
Created attachment 59535 [details]
Some cleanup ON TOP of the full patch proposal

Get rid of g_timeout_add() by using g_idle_add() instead. This patch must be applied after the previos one "Full patch proposal (with unit test)".
Comment 8 Xan Lopez 2010-06-23 11:55:09 PDT
Comment on attachment 59535 [details]
Some cleanup ON TOP of the full patch proposal

I'm Krusty the Klown and I approve this message.
Comment 9 WebKit Commit Bot 2010-06-25 13:09:35 PDT
Comment on attachment 59528 [details]
Full patch proposal (with unit test)

Clearing flags on attachment: 59528

Committed r61884: <http://trac.webkit.org/changeset/61884>
Comment 10 WebKit Commit Bot 2010-06-25 13:23:12 PDT
Comment on attachment 59535 [details]
Some cleanup ON TOP of the full patch proposal

Clearing flags on attachment: 59535

Committed r61893: <http://trac.webkit.org/changeset/61893>
Comment 11 WebKit Commit Bot 2010-06-25 13:23:17 PDT
All reviewed patches have been landed.  Closing bug.