Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#15900 closed defect (fixed)

dijit/Calendar doesn't use id and headers attributes to associate data cells with header cells (WCAG SC 1.3.1)

Reported by: brodrick Owned by: mikeb
Priority: undecided Milestone: 1.8.4
Component: Accessibility Version: 1.8.0
Keywords: Cc: cjolif
Blocked By: Blocking:

Description (last modified by bill)

dijit.form.DateTextBox doesn't satisfy WCAG SC 1.3.1 http://www.w3.org/TR/WCAG20-TECHS/H43, which states that, in a complex data table, th and td elements must be related via scope or headers. This creates problems for accessibility, and causes certain a11y test tools to report multiple severe violations when the calendar is displayed on the widget.

I'm assuming this is because:

  1. The <table> does not have role="presentation" (which seems right).
  2. The table includes more than one row of <th> elements.
  3. None of these table headers are associated with data cells via id and headers attributes, or even scope attributes.

Attachments (9)

Calendar.patch (13.7 KB) - added by mikeb 7 years ago.
Calendar a11y: add scope="col", set default summary to the month, split html into two tables, please proxy commit for Michael Billau CCLA on file with IBM
Calendar3.patch (13.6 KB) - added by bill 7 years ago.
start of revised patch, with just one <table> node
Calendar4.patch (15.3 KB) - added by bill 7 years ago.
Patch using CSS rather than a table for the month display. Works well everywhere except IE6 and IE7, where a bunch of workaround rules are needed to stop the Calendar from expanding to the full width of the viewport, and to get the arrows to appear in the right place.
Calendar5.patch (14.2 KB) - added by bill 7 years ago.
patch of the future, restricting use of <table> to actual grid of days, and degrading on IE6/7; the problem is that it renders too wide on IE quirks mode
Calendar5-addendum-CalendarLite.patch (2.2 KB) - added by mikeb 7 years ago.
update Calendar5 patch to also add the correct ID to the month element in CalendarLite?
calendar6.patch (10.8 KB) - added by mikeb 7 years ago.
update calendar5 patch, keep calendar as a single table but add scope=col to the month buttons
15900-backport.patch (9.2 KB) - added by mikeb 6 years ago.
Backport Calendar accessibility fixes to 1.8, please proxy commit for Michael Billau CCLA on file
15900-backport-v2.patch (9.2 KB) - added by mikeb 6 years ago.
Backport Calendar accessibility fixes to 1.8, take 2 with updated spacing
15900-backport-sans-DateTextBox-changes.patch (8.1 KB) - added by mikeb 6 years ago.
Backport calendar a11y fixes to 1.8 ignoring DateTextBox? changes

Download all attachments as: .zip

Change History (29)

comment:1 Changed 7 years ago by bill

Description: modified (diff)
Summary: dijit.form.DateTextBox doesn't use id and headers attributes to associate data cells with header cells (WCAG SC 1.3.1)dijit/Calendar doesn't use id and headers attributes to associate data cells with header cells (WCAG SC 1.3.1)

Presumably you are talking about the drop down Calendar, not the DateTextBox itself.

Changed 7 years ago by mikeb

Attachment: Calendar.patch added

Calendar a11y: add scope="col", set default summary to the month, split html into two tables, please proxy commit for Michael Billau CCLA on file with IBM

comment:2 Changed 7 years ago by mikeb

Hello and thanks for the report. I have fixed up the Calendar HTML, I tried to follow the best practices. Changes:

  1. Added scope="col" to the days of the week cells (Sun, Mon, Tue., etc) since these are the headers. scope="col" should be sufficient since it is not a very complex table
  2. If the user doesn't specify a summary, use the current month as a summary
  3. Following best practices and examples online, I split the Calendar into two different tables, the first is a presentation table that contains the month drop down widget and prev/next month buttons, the second table has role="grid" and is contains all of the numbers. This was needed because the control widgets are not part of the grid and couldn't be made into headers.
  4. Copied the new template into tests/_altCalendar.html which is used on the test page...I'm not really sure what it is for or if there is a reason why the test page is using _altCalendar.html and not ../templates/Calendar.html
  5. Fixed up the test pages to make them more compliant.

comment:3 Changed 7 years ago by mikeb

Cc: cjolif added

comment:4 Changed 7 years ago by bill

I'm starting to look at this now. There are some issues with the patch though. The main issue is that the changes to the template are scary:

  • You've replaced a single table with three tables, which will likely degrade performance significantly.
  • The first nested table (with the month name and the left/right arrows) probably doesn't need to be a <table> at all; you can just use float:left and float:right for the arrows.
  • Likewise with the bottom section listing previous/current/next year, there's no reason for that to be part of a nested table.
  • Also, the template is malformed: a <thead> is closed with a </th>.

The other thing is that if if you want to send the summary to the nested grid node, the way to do it is to override _PaletteMixin, setting:

	_setSummaryAttr: "paletteTableNode",
Version 2, edited 7 years ago by bill (previous) (next) (diff)

comment:5 Changed 7 years ago by bill

Milestone: tbd1.9

Changed 7 years ago by bill

Attachment: Calendar3.patch added

start of revised patch, with just one <table> node

Changed 7 years ago by bill

Attachment: Calendar4.patch added

Patch using CSS rather than a table for the month display. Works well everywhere except IE6 and IE7, where a bunch of workaround rules are needed to stop the Calendar from expanding to the full width of the viewport, and to get the arrows to appear in the right place.

Changed 7 years ago by bill

Attachment: Calendar5.patch added

patch of the future, restricting use of <table> to actual grid of days, and degrading on IE6/7; the problem is that it renders too wide on IE quirks mode

comment:6 Changed 7 years ago by mikeb

Hi Bill. Thanks a ton for checking this out. I looked at your patch and can confirm that there are no accessibility violations on my end. There was one though with messed up IDs; I fixed this in CalendarLite?.js and just uploaded that single page (with your changes.) The problem is that the template for the big calendar with a drop down references the "{id}_mddb" (month drop down box) but this drop down is absent on CalendarLite?, so I just added that particular ID. I'm not seeing any more problems.

Changed 7 years ago by mikeb

update Calendar5 patch to also add the correct ID to the month element in CalendarLite?

comment:7 Changed 7 years ago by bill

OK. But the problem with my patch is that it fails for IE quirks mode: the calendar expand to the full width of the page or containing div. So if we use my patch [pre 2.0], need to find some way to avoid that.

The alternate approach is to leave the template mainly as it is in 1.8, using a single <table>, but add appropriate a11y attributes to avoid the a11y violations. I know that it's OK to have a table with multiple rows of headers because that's the first example in http://www.w3.org/TR/WCAG20-TECHS/H43.

comment:8 Changed 7 years ago by mikeb

Well I suppose we could add scope="col" to the <th> containing the month buttons and drop down. I tested this and surprisingly it didn't generate a violation or seem to reduce JAWS useability. It does seem strange though because the buttons and month I don't think really count as headers for the rest of the data (the month value may, but the buttons cant.) So setting scope="col" works to remove the violation while keeping the calendar as a single <table>. It would be better semantically to pull the buttons out but it seems that currently this doesn't affect accessibility. I created a new patch based off calendar5 just without the CSS or template change.

While you are looking at this Bill, what do you think about adding keyboard support to the month buttons and drop down month selector? I tried using ondijitclick instead of onclick in _connectControls() but wasn't having any luck. We don't really need keyboard support for these dijits since there are keyboard controls on the grid itself to jump ahead months/years, but if it is an easy fix maybe it would be a good idea.

Changed 7 years ago by mikeb

Attachment: calendar6.patch added

update calendar5 patch, keep calendar as a single table but add scope=col to the month buttons

comment:9 in reply to:  8 Changed 7 years ago by bill

Replying to mikeb:

Well I suppose we could add scope="col" to the <th> containing the month buttons and drop down. I tested this and surprisingly it didn't generate a violation or seem to reduce JAWS useability. It does seem strange though because the buttons and month I don't think really count as headers for the rest of the data (the month value may, but the buttons cant.) So setting scope="col" works to remove the violation while keeping the calendar as a single <table>. It would be better semantically to pull the buttons out but it seems that currently this doesn't affect accessibility.

I definitely agree with everything you said. It's just a question of whether to pull the buttons and year labels out of the <table> for the 1.9 release, or in 2.0. I would tend towards 2.0, so I filed #16210 to track that.

While you are looking at this Bill, what do you think about adding keyboard support to the month buttons and drop down month selector? I tried using ondijitclick instead of onclick in _connectControls() but wasn't having any luck. We don't really need keyboard support for these dijits since there are keyboard controls on the grid itself to jump ahead months/years, but if it is an easy fix maybe it would be a good idea.

Hmm, well there would be no new functionality, because like you said there's already keyboard suport for jumping ahead months/years. It would be an improvement in (what the usability folks call) discoverability, since people might not know about (or be able to guess) keystrokes like Ctrl-page-down. The downside though would be increased tab stops in the Calendar widget, having perhaps three tab stops (for the month, the calendar grid, and the year label) instead of one. And I'm not sure how that would work out with JAWS, meaning I'm not sure how a blind person would know that when they tabbed to the month label that they were (a) inside the calendar and (b) on the month label. So, overall, I'm not sure what to think of the idea but in any case it doesn't sound like a trivial task.

comment:10 Changed 7 years ago by bill

Resolution: fixed
Status: newclosed

In [29851]:

Avoid a11y violations in Calendar by adding scope="col". And add auto-setting of summary if it isn't explicitly specified.

Also switched DateTextBox to use autoSize:true rather than fixedSize:true, so that the Calendar's width isn't truncated with the DateTextBox field is narrow.

Also, removed _altCalendar.html. This file is in the test/ directly, but was actually checked in as an example of how a user could customize the Calendar widget. However, it's annoying to maintain.

Finally, some test file updates for accessibility.

Patch Mike Billau (IBM, CCLA), thanks.

Fixes #15900 !strict.

comment:11 Changed 7 years ago by bill

Checked in your patch. It should really have tests for the summary setting though, in particular the part where it sets the summary to the month name if no explicit summary was specified.

comment:12 Changed 7 years ago by bill

In [29859]:

Add minimal tests for summary getting set and updated correctly, refs #15900.

comment:13 Changed 6 years ago by mikeb

Hello. I think that we need to try to backport this to 1.8, would that be possible? I created a patch combining the changes from 29851 and 29859 and it passed all of the tests.

Changed 6 years ago by mikeb

Attachment: 15900-backport.patch added

Backport Calendar accessibility fixes to 1.8, please proxy commit for Michael Billau CCLA on file

comment:14 Changed 6 years ago by bill

This patch has a bunch of changes unrelated to accessibility, and there are spacing problems.

comment:15 Changed 6 years ago by mikeb

I uploaded a version with correct spacing. However, I think all of the changes are accessibility related. Which changes looked odd to you?

CalendarLite.js:

  • add scope=col to the days-of-the-week elements in Calendar
  • sets the summary on the "gridNode" attach point
  • auto set the summary if it isn't set
  • change MonthWidget's ID to match the aria-labelledby in the template

_DateTimeTextBox.js:

  • use autoWidth instead of forceWidth so that Calendar's width isn't truncated when the DateTextBox field is narrow.

templates/Calendar.html:

  • add the gridNode attach-point to the Calendar's table dom node
  • add scope=col to the prev/next month buttons

tests/CalendarLite.html

  • fixed spacing
  • adds automated tests for checking and setting the summary
  • aria updates on the test page

tests/form/test_DateTextBox.html

  • altered test to make sure calendar shows up in narrow dateTextBox

test/test_Calendar.html

  • fixed spacing
  • added aria on the test page

Also this patch should remove the tests/_altCalendar.html file that was removed in [29851]

Changed 6 years ago by mikeb

Attachment: 15900-backport-v2.patch added

Backport Calendar accessibility fixes to 1.8, take 2 with updated spacing

comment:16 Changed 6 years ago by bill

At first glance these changes seem unrelated:

  • change MonthWidget's ID to match the aria-labelledby in the template _DateTimeTextBox.js:
  • use autoWidth instead of forceWidth so that Calendar's width isn't truncated when the DateTextBox field is narrow.
  • altered test to make sure calendar shows up in narrow dateTextBox test/test_Calendar.html
  • remove the tests/_altCalendar.html file that was removed in [29851]

Maybe I missed something?

comment:17 Changed 6 years ago by mikeb

  • MonthWidget : In the Calendar template, it has: aria-labelledby="${id}_mddb ${id}_year" on the main table that has role=grid. This is so that when the month changes, JAWS will read out the month name. It was originally put into trunk in [26667] but that ticket only changed Calendar and forgot about CalendarLite?.js
  • _DateTimeTextBox.js - use autoWidth instead of forceWidth - this is so that if somebody sets a date picker to be narrow, _HasDropDown won't truncate the calendar to the width of the date picker. I experimented with this and could not find a difference between auto and force, even on the nightly build.
  • altered test to make sure calendar shows up in narrow dateTextBox - I guess thinking about it now you're right these changes to dateTextBox dont really seem to have much to do with accessibility
  • remove tests/_altCalendar.html - it is no longer being used after changes to the test_Calendar.html that are made in this ticket.

I uploaded a new version of the patch without the changes to dateTextBox if you think it would be better.

comment:18 Changed 6 years ago by Douglas Hays

In [30318]:

Refs #15900. Backport a11y parts of [29851] and [29859] to 1.8. !strict

Changed 6 years ago by mikeb

Backport calendar a11y fixes to 1.8 ignoring DateTextBox? changes

comment:19 Changed 6 years ago by Douglas Hays

Milestone: 1.91.8.4

comment:20 Changed 6 years ago by Douglas Hays

In [30319]:

Refs #15900. Change month id in 1.8 so JAWS will read it (to match trunk). !strict

Note: See TracTickets for help on using tickets.