469688 - <table role="*"> should expose inner table structure only for weak roles

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

We do get this right if the role is presentation or grid.
However, we get it wrong for other cases. See the "main" and "menu" cases in the testcase at:
http://codetalks.org/source/simple/tables-with-roles.html

Comment 1

[Marco Zehe (:MarcoZ)]

I'd like to request that role="log" be exposed with table structure as well. I'm still convinced that a log can also be viewed as a table structure, like ChatZilla shows, which adds more navigational convenience when being able to navigate the content quickly by row or column.

Comment 2

[Aaron Leventhal]

Marco, fair enough. Do you have Codetalks.org SVN access and can add that case to the test?

Comment 3

[Marco Zehe (:MarcoZ)]

Done in revision 201.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Seems important, I'll take a shot at it.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

I'm not seeing inner table structure for the "menu" case (on linux). The "main" case does expose the inner table structure. I'm not sure why this is a bug?

Comment 6

[Aaron Leventhal]

What about "grid".

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

I'll check tomorrow (I hope), but note my point about "main" was that I don't understand why we shouldn't expose the inner table structure (as the codetalks test implies). We do expose it, and I see that as OK. Are we supposed to treat landmarks the same as presentational? (not sure that makes sense)

Comment 8

[Aaron Leventhal]

If the author uses <table role="search">, <table role="navigation"> or <table role="contentinfo"> or use any landmark they are saying, "here is a pane in the web page". It's a pane of info, which to me would indicate text contents not columnized data. I can see your argument though -- it's *possible* that the main content would be a data table.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #8)

Right. I wonder what the default should be? As an author I'd be tempted to do:

<div role="search"><table role="presentation">... if I didn't want table structure exposed.

Comment 10

[Aaron Leventhal]

Ok. I think it should be discussed as part of the ARIA implementation group.

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Alexander, I'm going to self-unassign this one since it overlaps your table, treegrid work.

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Should also consider what to do with <table role="log"> since as Marco points out... log is like a landmark really (except with implicit live properties).

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

I wonder if this was fixed via work on bug 481114...

Comment 14

[David Bolter [:davidb] (NeedInfo me for attention)]

Confirmed, closing as fixed via work on bug 481114.

Comment 15

[alexander :surkov (:asurkov)]

(In reply to comment #14)
> Confirmed, closing as fixed via work on bug 481114.

Hey, David. You might be a bit quick. The question is should we expose table interface for <table role="something". Bug 481114 was about timer, log and marquee. We expose table for all of them if I get right (though I'm not sure if we should expose table for timer and marquee). Also it's not clear what should we do for "search", "main" and other landmarks in law and what should we do for role="anything else".

Comment 16

[David Bolter [:davidb] (NeedInfo me for attention)]

Sure but can you give me a working example of where we might do the wrong thing here? I can make this an action item in the UIA-TF group if you like; then we can open a bug if we do it against consensus. Sound good?

Comment 17

[alexander :surkov (:asurkov)]

(In reply to comment #16)
> Sure but can you give me a working example of where we might do the wrong thing
> here? I can make this an action item in the UIA-TF group if you like; then we
> can open a bug if we do it against consensus. Sound good?

I can give nether wrong nor right example. I just not sure if we need to expose table interface and underlying table structure for some roles like it was said in bug description. We need to discuss this with groups but I don't think it would be worth to close this bug until we are 100% sure about its status.

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

OK, I've added a whiteboard indicator called "[a11y discuss]" -- which I just made up. Think of this as sort of like "doc needed". This indicator will allow us to push our work, and move bugs out of the way that are probably fixed; but at the same time allow us to easily query for all bugs needing discussion, or group consensus. When we decide discussion is no longer needed we can clear the whiteboard field.

In general I think we should aim for discussion/consensus before implementation, but the other browsers are so far behind here I think leaving these open is just creating noise at this point.

Thoughts?

Comment 19

[alexander :surkov (:asurkov)]

David, if you want to keep the bug closed then you should mark it as invalid. It's not fixed. And imo bug 481114 is not related with this one. But you can't do this until you're 100% sure :).

I don't know if we should wait for someone. I think we need to ask AT and toolkit developers and send question to aria group. If I get correctly then Marco was intended to aks someone.

Btw iirc one AT said they won't use IAccessibleTable until accessible has proper role. So it might not make sense to expose table for <table role="listitem"/> for example.

Comment 20

[David Bolter [:davidb] (NeedInfo me for attention)]

I think bug 481114 does apply to the codetalks example Aaron mentioned in his STR. Yeah we should always talk to AT folk.

What do you mean by "proper role" -- do you mean role=table?

Comment 21

[alexander :surkov (:asurkov)]

So do you insist we should always expose table interface and underlying table structure for HTML table elements without table role?

(In reply to comment #20)
> I think bug 481114 does apply to the codetalks example Aaron mentioned in his
> STR. Yeah we should always talk to AT folk.

How is it applied? bug 481114 address neither role="menu" nor role="main".

> What do you mean by "proper role" -- do you mean role=table?

Yes.

Comment 22

[David Bolter [:davidb] (NeedInfo me for attention)]

I rarely insist on anything :)

IIRC this bug is about exposing table based inner structure accessibles like for cells; not about nsIAccessibleTable.

I've testing the codetalks examples and I don't see any inner table structure exposed. I should probably close this as WORKSFORME (currenty INVALID).

Comment 23

[alexander :surkov (:asurkov)]

(In reply to comment #22)
> I rarely insist on anything :)
> 
> IIRC this bug is about exposing table based inner structure accessibles like
> for cells; not about nsIAccessibleTable.

It's about both. If there is no table inner structure then it's wrong to expose nsIAccessibleTable.

What's about landmarks that don't affect on role? Should we expose table structure and interface?

Comment 24

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #23)
> What's about landmarks that don't affect on role? Should we expose table
> structure and interface?

Yes. Do you agree?

Comment 25

[alexander :surkov (:asurkov)]

(In reply to comment #24)
> (In reply to comment #23)
> > What's about landmarks that don't affect on role? Should we expose table
> > structure and interface?
> 
> Yes. Do you agree?

Ok. It's probably worth. Though originally "main" landmark was mentioned in bug report. But since we use native role then we shouldn't stop the table to exposed.

David, ok. If bug is worksforme then we should provide mochitests if we haven't.

Comment 26

[David Bolter [:davidb] (NeedInfo me for attention)]

Reopening to add tests.

Comment 27

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: more tests

I think this combined with the tests for bug 481114 provide decent coverage.

Comment 28

[Marco Zehe (:MarcoZ)]

Comment on attachment 385121 [details] [diff] [review]
more tests

r=me

Comment 29

[David Bolter [:davidb] (NeedInfo me for attention)]

Do we have test util functions for testing the non-existence of certain child roles?

Comment 30

[alexander :surkov (:asurkov)]

Comment on attachment 385121 [details] [diff] [review]
more tests


>+        var iAccessibleTable = getAccessible(id, [nsIAccessibleTable]);
>+        ok(iAccessibleTable, "landmarked table should have nsIAccessibleTable");

getAccessible report error if fails, use isAccessible instead.

>+  <!-- landmarks are tables -->
>+  <table role="application" id="application_table">application table</table>
>+  <table role="banner" id="banner_table">banner table</table>

you need also to test underlying table structure.

> // test gEmptyRoleMap
>      testRole("cell", ROLE_NOTHING);

>   <!-- test gEmptyRoleMap -->
>   <table role="label">
>     <tr>
>       <td id="cell">cell</td>
>     </tr>
>   </table>


please extend this test: table interface and accessible tree.

Comment 31

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: addresses alex comments

Comment 32

[alexander :surkov (:asurkov)]

Comment on attachment 394029 [details] [diff] [review]
addresses alex comments

r=me, though probably testAccessibleTree is preferable than cellRefAt

Comment 33

[alexander :surkov (:asurkov)]

Comment on attachment 394029 [details] [diff] [review]
addresses alex comments


>+      for (l in weak_landmarks) {
>+        var id = weak_landmarks[l]+"_table";

forgot to say: wrap '+' by spaces

>+        ok(accessibleTable? true : false,

nit: space before ?

Comment 34

[David Bolter [:davidb] (NeedInfo me for attention)]

Nits fixed thanks. This issue is considered resolved. Tests pushed as changeset:
http://hg.mozilla.org/mozilla-central/rev/7544a85acf9b

Comment 35

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 394029 [details] [diff] [review]
addresses alex comments

a192=beltzner

Comment 36

[alexander :surkov (:asurkov)]

landed on 1.9.2 - http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5b9497ca575b

Comment 37

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b2pre) Gecko/20091029 Namoroka/3.6b2pre (.NET CLR 3.5.30729)