Last Comment Bug 525909 - Support ARIA role "rowgroup"
: Support ARIA role "rowgroup"
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: mozilla19
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks: aria
  Show dependency treegraph
 
Reported: 2009-11-02 10:20 PST by David Bolter [:davidb]
Modified: 2012-11-17 05:11 PST (History)
5 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (567 bytes, text/html)
2009-11-02 11:00 PST, Marco Zehe (:MarcoZ) on PTO until August 15
no flags Details
patch (3.82 KB, patch)
2012-11-12 05:53 PST, alexander :surkov
no flags Details | Diff | Splinter Review
patch2 (5.18 KB, patch)
2012-11-12 05:54 PST, alexander :surkov
tbsaunde+mozbugs: review+
dbolter: feedback+
Details | Diff | Splinter Review

Description David Bolter [:davidb] 2009-11-02 10:20:17 PST
Member only: http://www.w3.org/WAI/PF/Group/track/issues/372

"There might be problems introducing the rowgroup role, we need to test impacts on existing implementation prior to ARIA 2nd LC"

We should add mochitests (and possible implementation), to make sure that if rows are contained in an element with role="rowgroup" that nothing breaks horribly. If this new role is something that breaks us for 3.6 we should request pushing it to ARIA 2.0.
Comment 1 David Bolter [:davidb] 2009-11-02 10:55:33 PST
Would be good to know what happens in the case of role="group" as well.
Comment 2 Marco Zehe (:MarcoZ) on PTO until August 15 2009-11-02 11:00:04 PST
Created attachment 409726 [details]
Testcase

Simple testcase showing what I think is meant.

Testing shows with AccProbe that whether the div has a role of rowgroup or not does not make any difference to Minefield or Namoroka. The table interface yields the same result in both cases.

Note that there is also no accessible being created for the rowgroup. The rows inside the rowgroup div appear to be at the same level as the others.
Comment 3 David Bolter [:davidb] 2009-11-02 11:06:32 PST
Just to be clear, the node with role=rowgroup is not in the accessible tree at all?
Comment 4 Marco Zehe (:MarcoZ) on PTO until August 15 2009-11-02 11:16:43 PST
(In reply to comment #3)
> Just to be clear, the node with role=rowgroup is not in the accessible tree at
> all?

Correct!
Comment 5 Marco Zehe (:MarcoZ) on PTO until August 15 2009-11-02 11:20:02 PST
When I just did the change from "rowgroup" to "group", I now see what happens:

1. With the div having a role of "rowgroup", a "section" node is being created on the same level as the table. So the document accessible has two children: An empty section and the table.
2. When changing this to "group", the doucment also gets two children: a grouping and the table. Inside the table, the hierarchy is the same in both cases: The table has 4 rows as children, with each having cell children of their own again.

So for some reason, we're creating accessibles outside the table hierarchy if the node appears inside the table but we don't expect it there.
Comment 6 David Bolter [:davidb] 2009-11-02 14:07:16 PST
We should check it against the DOM which I will do as soon as I catch my breath.
Comment 7 David Bolter [:davidb] 2009-11-02 14:12:32 PST
Yep that matches the DOM so I think we are doing the right thing. We should get this into suite so that we can tell if/when behaviour changes.
Comment 8 alexander :surkov 2009-11-02 18:09:18 PST
(In reply to comment #0)
> Member only: http://www.w3.org/WAI/PF/Group/track/issues/372
> 
> "There might be problems introducing the rowgroup role, we need to test impacts
> on existing implementation prior to ARIA 2nd LC"
> 
> We should add mochitests (and possible implementation), to make sure that if
> rows are contained in an element with role="rowgroup" that nothing breaks
> horribly. If this new role is something that breaks us for 3.6 we should
> request pushing it to ARIA 2.0.

Yes, it breaks. Now our implementation allows ARIA trees to use group to bring hierarchy. However we could support rowgroups in ARIA treegrid in timeframe of Firefox 3.7 I think. However could you clarify why does ARIA need new role and is role="group" not suitable for ARIA treegrid?
Comment 9 David Bolter [:davidb] 2009-11-26 10:27:39 PST
From the current spec:

rowgroup

A group containing one or more row elements in a grid.

The rowgroup role establishes a relationship between owned row elements. It is a structural equivalent to the thead, tfoot, and tbody elements in an HTML table element.

Note: This role does not differentiate between types of row groups (e.g. thead vs. tbody), but an issue has been raised for WAI-ARIA 2.0.
Comment 10 alexander :surkov 2009-11-26 17:47:46 PST
Ah, that's interesting. I thought it's analogue of ARIA group in ARIA trees  but I was wrong. 

Good news, we don't need to support ARIA rowgroup at all ;)

1) We don't expose thead and etc
2) "establishes a relationship" is too diluted to consider this as implementation manual.
Comment 11 David Bolter [:davidb] 2009-11-30 12:40:14 PST
OK :)

Let's keep this low priority until/unless someone asks for it.
Comment 12 alexander :surkov 2010-11-18 23:55:40 PST
Per discussion with Rich we should expose it as role="group" and use member_of relation. Bumping to P2 (P1 I believe should be reserved for FX4 blocking stuffs) since it's required by ARIA to pass candidate recommendation.
Comment 13 alexander :surkov 2012-11-12 05:53:28 PST
Created attachment 680608 [details] [diff] [review]
patch

just expose it as grouping role. No relations since I don't see any utility of them.
Comment 14 alexander :surkov 2012-11-12 05:54:35 PST
Created attachment 680609 [details] [diff] [review]
patch2

(correct patch)
Comment 15 alexander :surkov 2012-11-12 06:07:32 PST
Comment on attachment 680609 [details] [diff] [review]
patch2

now we expose an accessible for role="rowgroup" with no role actually (MSAA role is "rowgroup", IA2 role is from native markup). In either case role from native markup as MSAA string role doesn't seem right here.

Btw, spec allows us to not expose an accessible for it at all:

"Not mapped" + included elements: "Elements that have a mappable role in the role attribute string (see Role Mapping below). "

But having no accessible might be not correct so. It seems reasonable to expose it as grouping role:

IA2: ROLE_SYSTEM_GROUPING
ATK: ATK_ROLE_PANEL
OSX: NSAccessibilityGroupRole

(we could request to change ARIA impl guide for IA2 and ATK I guess but I'm not sure about OS X). What do you think?
Comment 16 David Bolter [:davidb] 2012-11-12 06:16:00 PST
Comment on attachment 680609 [details] [diff] [review]
patch2

Review of attachment 680609 [details] [diff] [review]:
-----------------------------------------------------------------

A group role here seems good to me as well.
Comment 17 Trevor Saunders (:tbsaunde) 2012-11-12 09:12:21 PST
(In reply to alexander :surkov from comment #15)
> Comment on attachment 680609 [details] [diff] [review]
> patch2
> 
> now we expose an accessible for role="rowgroup" with no role actually (MSAA
> role is "rowgroup", IA2 role is from native markup). In either case role
> from native markup as MSAA string role doesn't seem right here.
> 
> Btw, spec allows us to not expose an accessible for it at all:
> 
> "Not mapped" + included elements: "Elements that have a mappable role in the
> role attribute string (see Role Mapping below). "
> 
> But having no accessible might be not correct so. It seems reasonable to
> expose it as grouping role:
> 
> IA2: ROLE_SYSTEM_GROUPING
> ATK: ATK_ROLE_PANEL
> OSX: NSAccessibilityGroupRole
> 
> (we could request to change ARIA impl guide for IA2 and ATK I guess but I'm
> not sure about OS X). What do you think?

for atk having a pain under a table seems funny, same for OSX I think, but I'm not sure I have a better idea.

Really however I tend to think we should only add this role if its somehow different from group and it makes sense to expose it differently.
Comment 18 alexander :surkov 2012-11-12 17:57:51 PST
(In reply to Trevor Saunders (:tbsaunde) from comment #17)

> > IA2: ROLE_SYSTEM_GROUPING
> > ATK: ATK_ROLE_PANEL
> > OSX: NSAccessibilityGroupRole
> > 
> > (we could request to change ARIA impl guide for IA2 and ATK I guess but I'm
> > not sure about OS X). What do you think?
> 
> for atk having a pain under a table seems funny, same for OSX I think, but
> I'm not sure I have a better idea.

yep, that why I asked

> Really however I tend to think we should only add this role if its somehow
> different from group and it makes sense to expose it differently.

You talk abut ARIA roles "group" vs "rowgroup"? It seems sometimes ARIA adds extra roles for the author that doesn't have any special processing by AT.
Comment 19 alexander :surkov 2012-11-14 20:34:33 PST
Jamie, do you have opinion on this?
Comment 20 alexander :surkov 2012-11-15 22:23:41 PST
Trev pointed that rowgroup elements may be used to introduce groups (I know that sounds tautology and you already named us mr obvious slapping your head) but yep it should affect on group info we expose. If we have rowgroup accessible then it happens automatically, if we reject to have it an accessible for it (ARIA allows that to us) then we need to do some magic on this way. Btw, having rowgroup accessible we could expose group info for rowgroups as well (but we aren't allowed by ARIA spec).

So, I think I'd want to keep an accessible for rowgroup (it seems to be lesser problems approach). And rowgroup shouldn't be a weak role like it's implemented now. Role GROUPING having mapping described in comment #15 should be more appropriate than native markup role even if they aren't really suitable (btw it contradicts to ARIA impl guide, I'll file a bug). In either case "rowgroup" is inherited from "group" ARIA role which has GROUPING role mapping so we should be in a good shape from that point of view.

If we get another feedback then let's change roles to a better fit. Until that I suggest to take this patch. I need a decision taken here to proceed with bug 804461.
Comment 21 Ryan VanderMeulen [:RyanVM] 2012-11-17 05:11:52 PST
https://hg.mozilla.org/mozilla-central/rev/b401cbf179c1

Note You need to log in before you can comment on or make changes to this bug.