Support rowgroup and colgroup HTML scope

RESOLVED FIXED in Firefox 39

Status

()

Core
Disability Access APIs
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: MarcoZ, Assigned: surkov)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla39
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Received this from Paul Boman of Deque Systems during CSUN 2015. Excerpt from e-mail:

The first link below uses the colgroup and rowgroup attributes. The second link uses headers  and ID attributes. Neither method works well with NVDA. It would be great if you could look into it and determine whether the issue is with Firefox or NVDA. 

https://dequeuniversity.com/testsuite/tables/colgroup-rowgroup

https://dequeuniversity.com/testsuite/tables/spanned-headers

I've confirmed that both table samples above don't read well at all with NVDA. Either the headers aren't associated at all, or they're wrong, e. g. off by 1 column, like in the second example.
Flags: needinfo?(surkov.alexander)
(Assignee)

Comment 1

3 years ago
we don't implement rowgroup and colgroup values for scope attribute [1]

[1] https://html.spec.whatwg.org/multipage/tables.html#attr-th-scope-rowgroup
Blocks: 368880
Flags: needinfo?(surkov.alexander)
(Assignee)

Updated

3 years ago
Summary: Some tables with row groups or header IDs have wrong header associations → Support rowgroup and colgroup HTML scope
(Assignee)

Comment 2

3 years ago
Created attachment 8576098 [details] [diff] [review]
patch

please file separate bug for spanned columns issue
Assignee: nobody → surkov.alexander
Attachment #8576098 - Flags: review?(mzehe)
Comment on attachment 8576098 [details] [diff] [review]
patch

> HTMLTableHeaderCellAccessible::NativeRole()
> {
>   // Check value of @scope attribute.
>   static nsIContent::AttrValuesArray scopeValues[] =
>-    {&nsGkAtoms::col, &nsGkAtoms::row, nullptr};
>+    { &nsGkAtoms::col, &nsGkAtoms::colgroup,
>+      &nsGkAtoms::row, &nsGkAtoms::rowgroup, nullptr };
>   int32_t valueIdx =
>     mContent->FindAttrValueIn(kNameSpaceID_None, nsGkAtoms::scope,
>                               scopeValues, eCaseMatters);
> 
>   switch (valueIdx) {
>-    case 0:
>+    case 0: case 1:
>       return roles::COLUMNHEADER;
>-    case 1:
>+    case 2: case 3:

imo its easier to read if case labels get their own line.
(Reporter)

Comment 4

3 years ago
Comment on attachment 8576098 [details] [diff] [review]
patch

Wow, so little code to fix this! Amazing! ;) r=me with the remark Trevor had about the case values being each on one line. I think this makes it easier to read indeed.

I filed bug 1146257 for the second sample.
Attachment #8576098 - Flags: review?(mzehe) → review+
https://hg.mozilla.org/mozilla-central/rev/efdffcda2671
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
(Assignee)

Comment 7

3 years ago
Created attachment 8583102 [details] [diff] [review]
followup test fix
Attachment #8583102 - Flags: review?(mzehe)
(Reporter)

Updated

3 years ago
Attachment #8583102 - Flags: review?(mzehe) → review+
You need to log in before you can comment on or make changes to this bug.