516133 - ARIA treegrid role on HTML:table makes thead/tbody accessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

simple testcase:

<table role="treegrid" id="grid" border="1" cellpadding="10" cellspacing="0">
    <thead>
      <tr role="row">
        <th role="columnheader">subject</td>
        <th role="columnheader">sender</th>
        <th role="columnheader">date</th>
      </tr>
    </thead>
    <tbody>
      <tr role="row">
        <td role="gridcell" tabindex="0">about everything</td>
        <td role="gridcell">president</td>
        <td role="gridcell">today</td>
      </tr>
      <tr role="row">
        <td role="gridcell">new bugs</td>
        <td role="gridcell">mozilla team</td>
        <td role="gridcell">today</td>
      </tr>
    </tbody>
  </table>

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

mochitests fails because of bug 516135 (cell's rowExtent fails), I suggest to check in this bug after that one.

Comment 2

[Marco Zehe (:MarcoZ)]

Comment on attachment 400242 [details] [diff] [review]
patch

>+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=491683">Mozilla Bug 516133</a>

Nit: Wrong bug number in the href attribute, the actual link text is correct.

r=me with that fixed.

Comment 3

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

Comment on attachment 400242 [details] [diff] [review]
patch

>-          if (tableAccessible &&
>-              nsAccUtils::Role(tableAccessible) != nsIAccessibleRole::ROLE_TABLE) {
>-            NS_ASSERTION(!roleMapEntry, "Should not be changing ARIA role, just overriding impl class role");
>-            // Not in table: override role (roleMap entry was null).
>-            roleMapEntry = &nsARIAMap::gEmptyRoleMap;
>+          if (tableAccessible) {
>+            PRUint32 role = nsAccUtils::Role(tableAccessible);
>+            if (role != nsIAccessibleRole::ROLE_TABLE &&
>+                role != nsIAccessibleRole::ROLE_TREE_TABLE) {
>+
>+              NS_ASSERTION(!roleMapEntry,
>+                           "Should not be changing ARIA role, just overriding impl class role");
>+              // Not in table: override role (roleMap entry was null).

I don't understand this case, or when it happens.

>+              roleMapEntry = &nsARIAMap::gEmptyRoleMap;
>+            }
>           }
>           break;

Comment 4

[alexander :surkov (:asurkov)]

I think it never happens !roleMapEntry can't be false because it is true on top if statement. Sort of rudiment I think.

Comment 5

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

I guess we can remove the assertion then... but I'm also not sure I understand the comment "// Not in table: override role (roleMap entry was null)."

How would we have a tableAccessible, not be in a table, and not have a roleMapEntry?

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> I guess we can remove the assertion then... but I'm also not sure I understand
> the comment "// Not in table: override role (roleMap entry was null)."
> 
> How would we have a tableAccessible, not be in a table, and not have a
> roleMapEntry?

it's easy <table role="log"><td role="bla"></td><td></td></table>

these cells shouldn't be a table cells, therefore we override their ARIA role. That's how it worked earlier I think.

Comment 7

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

Comment on attachment 400242 [details] [diff] [review]
patch

OK r=me... but:

(In reply to comment #6)
> (In reply to comment #5)
> > I guess we can remove the assertion then... but I'm also not sure I understand
> > the comment "// Not in table: override role (roleMap entry was null)."
> > 
> > How would we have a tableAccessible, not be in a table, and not have a
> > roleMapEntry?
> 
> it's easy <table role="log"><td role="bla"></td><td></td></table>
> 

>+              // Not in table: override role (roleMap entry was null).

Please add an example as a comment here. Thanks!

Comment 8

[alexander :surkov (:asurkov)]

landed on 1.9.3 with David's comment - http://hg.mozilla.org/mozilla-central/rev/66058a1c016c

Comment 9

[Marco Zehe (:MarcoZ)]

Comment on attachment 400242 [details] [diff] [review]
patch

Needed for completion of a11y support for tables in 3.6. See https://wiki.mozilla.org/Accessibility/Remaining_Mozilla-1.9.2_Nominations for more info.

Comment 10

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

Comment on attachment 400242 [details] [diff] [review]
patch

a192=beltzner

Comment 11

[alexander :surkov (:asurkov)]

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

Comment 12

[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)