Closed Bug 1141978 Opened 7 years ago Closed 7 years ago

Support rowgroup and colgroup HTML scope

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: MarcoZ, Assigned: surkov)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files)

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)
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: htmla11y
Flags: needinfo?(surkov.alexander)
Summary: Some tables with row groups or header IDs have wrong header associations → Support rowgroup and colgroup HTML scope
Attached patch patchSplinter Review
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.
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
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Attachment #8583102 - Flags: review?(mzehe)
Attachment #8583102 - Flags: review?(mzehe) → review+
You need to log in before you can comment on or make changes to this bug.