[webkit-reviews] review requested: [Bug 57463] [GTK] ARIA tables not exposing cells as HTML tables do : [Attachment 88393] Patch proposal + Layout test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 6 04:20:10 PDT 2011


Mario Sanchez Prada <msanchez at igalia.com> has asked  for review:
Bug 57463: [GTK] ARIA tables not exposing cells as HTML tables do
https://bugs.webkit.org/show_bug.cgi?id=57463

Attachment 88393: Patch proposal + Layout test
https://bugs.webkit.org/attachment.cgi?id=88393&action=review

------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
(In reply to comment #9)
> (From update of attachment 88199 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=88199&action=review
> 
> Is it possible to override what GTK returns for parent and children, and
> check if there's an AX table row in that equation, and if so, either pass
> on to the parent of the table row, or the children of the table row?

Problem is that at the moment in GTK accessibility objects for rows should
ignore accessibility, thus they should not be part of the accessibility
hierarchy, so I'm not sure how to do this by just overriding those functions.
Perhaps I'm missing something, or not understanding your suggestion properly,
though.

> I ask because I fear that there's a lot of edge cases that rely on a table
row
> for calculating or retrieving data.  A good example is rowIndexRange().
> It seems the rowIndex() method is really good for that, but GTK code 
> now has to do a lot of work to get that same information. 

Yeah, we need to do that extra calculations in GTK since accessible rows are
not added to he hierarchy, so we can't use the API from AccessibilityTableRow.

> In the Mac wrapper we do something similar when handling attachments.
> For children and parents, we check if there's an attachmentView and
> if there, we do something different. 

I've checked the code in the mac wrapper, but still not sure if it would be a
similar case to what happens here: we just want to leave (and handle everything
else accordingly) accessibility rows out from the a11y hierarchy, as it's done
with regular HTML tables.

But as I commented above, perhaps I'm not understanding properly your point, in
which case I'd appreciate some further comments from your side, if possible.
Thanks in advance.

> I also think that change may be a lot less code

I'm currently attaching a new patch which, even though it's not perhaps perfect
either, it's a lot less code than the previous one, mainly because I removed
all the "#if PLATFORM(GTK).. #endif" regions by writing all the changes as if
having rows ignoring accessibility was not a matter of the GTK platform only,
but something more general, and leaving the accessibilityIsIgnored() check to
do the work of making the right decision.

To tell the truth, I didn't like very much adding so much platform specific
code in the previous patch, but I did it so in order not to touch at all the
behavior of other platforms (that is, Mac), as I thought it could be a good
trade off.

But after reading the last line of your comment, I realized you wouldn't be as
much opposed to a more general patch as I initially thought (my fault, I'm the
king when it comes to making wrong assumptions) so I'm coming now with just the
opposite thing: a patch with no "#if PLATFORM(GTK).. #endif" regions at all.

Also, I've run the tests of the Mac port in a MacMini I have at hand, and it
seems it doesn't break anything either, so I guess it couldn't be that bad
option at the end :-)

And I have proof of that...

** BUILD SUCCEEDED **

Compiling Java tests
make: Nothing to be done for `default'.
Running tests from /Users/mario/Source/WebKit/LayoutTests
Testing 194 test cases.
accessibility
...............................................................................
.......
platform/mac/accessibility
...............................................................................
.............................
52.30s total testing time

all 194 test cases succeeded

 
> > Source/WebCore/accessibility/AccessibilityARIAGrid.cpp:85
> > +
> 
> I don't think this change needs to be platform dependent. I think the
accessibilityIsIgnored() check covers the platform dependentness.

   ^---> This is the "last line of your comment" I was referring to above.


More information about the webkit-reviews mailing list