Problem Description

When creating a table_select element using the form api, the added column with the checkboxes does not have a valid control labels. A title is added to the input for the "select all" by JS, but is not added to the inputs for each individual row. This causes screen readers such as NVDA to repeat the same "select all rows" title for each row, causing confusion. Ideally the input for each of these rows would be titled with something like 'select row #'.

For instance consider a form like follows:

  public function buildForm(array $form, FormStateInterface $form_state): array {

    $form['message'] = [
      '#type' => 'tableselect',
      '#title' => $this->t('Test Table Select'),
      '#required' => TRUE,
      '#header' => ['1','2','3'],
      '#options' => [
        '1' => ['a','b','c'],
        '2' => ['d','e','f'],
        '3' => ['g','h','i'],
      ],
      '#multiple' => TRUE,
    ];

    $form['actions'] = [
      '#type' => 'actions',
      'submit' => [
        '#type' => 'submit',
        '#value' => $this->t('Send'),
      ],
    ];

    return $form;
  }

If you examine the resultant form using a tool like the drupal editoria11y module (https://www.drupal.org/project/editoria11y) or the industry standard axe dev tools, it reports an accessibility error.

Image from axe dev-tools:

A11y error from Axe Dev Tools indicating error with select controls

CommentFileSizeAuthor
#45 Guided Search Axe Dev Tools Error 1.png199.12 KBkwfinken
#40 interdiff.txt16.84 KBalansaviolobo
#40 tableselect_select-851164-40.patch361 bytesalansaviolobo
#35 interdiff-17-34.txt18.59 KBmgifford
#34 core-js-selectall-check-851164-34.patch17.69 KBmgifford
#30 core-js-selectall-check-851164-30.patch17.77 KBmgifford
#23 Screen Shot 2012-11-02 at 13.12.46.png24.6 KBseutje
#17 core-js-selectall-check-851164.patch1.69 KBnod_
#8 drupal-table-select-all.html_.zip2.2 KBjasonkiss
#6 screen-capture-17.png219.15 KBmgifford
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

It did work for me in VoiceOver. I've attached an inaccessible screen capture of the process here - http://screenr.com/6DR

This was before the patch in #851174: Update options checkbox doesn't include labels was applied/developed.

rfay’s picture

rfay
Primary language English
Location Palisade, CO, USA
CreditAttribution: rfay commented
Component: ajax system » javascript

I assume that javascript is the right component for this.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Yes, thanks!

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

bump

Everett Zufelt’s picture

Everett Zufelt CreditAttribution: Everett Zufelt commented

I haven't taken a look at the code, but I'm not sure how we correct this. Screen-readers are typically non-standard when it comes to tables.

1. we can make sure that the cell isn't a header for the column. But, some AT will implicitly treat it as such.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
FileSize
screen-capture-17.png219.15 KB

Ok, so I've got it up here:
http://drupal7.dev.openconcept.ca/admin/content

The HTML looks like:

<input type="checkbox" class="form-checkbox" title="Select all rows in this table">

But that's injected with jQuery in misc/tableselect.js.

So, I've attached a screenshot with the view with firebug.

However, Everett, if you're not hearing "'Select all rows in this table' and this gets read as the associated header for every checkbox in that column, no matter the row." maybe this was just an issue with @jasonkiss' version or configuration.

Can you get that result? If not we can postpone this issue until it can be repeated.

Everett Zufelt’s picture

Everett Zufelt CreditAttribution: Everett Zufelt commented

The title is being read as the column header. I don't really know how we fix it though. Screen-readers don't really respect any predictable rules re: headers for tables.

See:
http://tink.co.uk/2010/08/screen-reader-support-for-html-tables/

jasonkiss’s picture

jasonkiss CreditAttribution: jasonkiss commented
FileSize
drupal-table-select-all.html_.zip2.2 KB

Here are some screen reader results from the current Content Admin table at http://drupal7.dev.openconcept.ca/admin/content when navigating through a row into the first column's checkbox cells. All screen readers were set with their default out-of-the-box preferences, except where noted otherwise.

JAWS 9.0.2191
IE8: prefaces checkboxes with "Select all rows in this table"
FF3.6: does *not* preface checkboxes with "Select all rows in this table"; but no column headers are read for any cell

JAWS 10.0.1178
IE8: does *not* preface checkboxes with "Select all rows in this table"
FF3.6: prefaces checkboxes with "Select all rows in this table"

JAWS 11.0.1447
IE8 and FF3.6: prefaces checkboxes with "Select all rows in this table"

Window-Eyes 7.2 (when set to read column headers using the CTRL-SHIFT-H rotor)
IE8 and FF3.6: prefaces checkboxes with "Select all rows in this table"

NVDA 2010.2beta1
IE8: does *not* preface checkboxes with "Select all rows in this table", but no column headers are read for any cell
FF3.6: does *not* preface checkboxes with "Select all rows in this table", but does read other column headers

VoiceOver3
Safari5: prefaces checkboxes with "Select all rows in this table"

Incidentally, Mike, the reason you don't find an issue in your screencast is because when you are just using the TAB key to get from focussable element to focussable element in the table, column headers are not announced. If you navigate using VoiceOver's table navigation commands, you'll hear the problem. Press ctrl-option-command-T to set focus to the table. Then press ctrl-option-shift-downarrow to start interacting with the table, at which point you can use ctrl-option and the arrow keys to navigate cell to cell.

My proposed solution is to remove the "Select all rows in this table" checkbox from the table, and place it outside and before the table. CSS can be used to visually position the checkbox over the first cell in the first column in the header row. I've put up a temporary demo page with some additional comments at http://www.kisser.ca/testing/drupal-table-select-all.html. I've also attached it to this comment as a zip file.

You'll note on that demo page that I make these other two suggestions:

  • that individual row checkboxes have explicit, but visually hidden label text, "Select this row". This label text could also be the same as the Title cell's content, or a combination, for example, "Select the row for Roto Facilisis Blandit".
  • that the cells in the "Title" column be marked up as th elements with scope="row" to act as row headers for supporting screen readers like JAWS 11 and NVDA 2010.2beta1, as well as Window-Eyes in IE8. Window-Eyes in FF3.6 does not respect row headers unless they're in the first column. If configured to read row headers, Window-Eyes in FF3.6 will default to using the first cell in the row, whatever it is, in this case, the checkbox. Unfortunately, VoiceOver doesn't do row headers at all. Still, it does no harm to use header cells here, and it should help users of some screen readers.

And while we're on the topic of the admin tables and table headers, I'd be interested to know how real screen reader users feel about table column headers that are also links for sorting the table. In the current implementation, they are just simple links, with no indication, other than their title attributes, that they are for sorting. Some, but not all assistive technologies will get this. Also, I'm not sure that it is clear to a visually impaired user what column and direction the table contents are currently sorted on. Would love to hear your thoughts on this, Everett. Maybe screen reader users who are administering a Drupal install will be familiar enough with this use of table headers as links to sort the table. Otherwise, it is possible, I think, to make things even clearer in this regard by doing one or more of the following:

  • don't link the column header text itself, but include a separate link or button in the header cell, for example, "Sort ascending", where the text is visually hidden
  • this extra link or button could also be a regular image element with alt-text, or it could use a CSS background image as a visual target, and to identify both the direction (ascending or descending) and whether or not it is the column on which the table is currently sorted
  • the header for the column on which the table is currently sorted could include some additional visually hidden text, like "(currently sorted descending)"

Like I said, this may be overkill for what a screen reader user who is also a Drupal administrator may require, but just thought I'd put it out there.

Everett Zufelt’s picture

Everett Zufelt CreditAttribution: Everett Zufelt commented

@jkiss

Thanks for the testing.

You write:
prefaces checkboxes with "Select all rows in this table"

No, it * never * does this, it does not preface the checkbox, it reads a column header and then reads the checkbox. We should make the top left cell not a th though, which may help things for some screen-readers.

that individual row checkboxes have explicit, but visually hidden label text, "Select this row". This label text could also be the same as the Title cell's
content, or a combination, for example, "Select the row for Roto Facilisis Blandit".

Agree completely.

As for the headers, I am comfortable with a linked column header being a design pattern that implies sorting the column. Definitely not a d7 issue.

Thanks again

jasonkiss’s picture

jasonkiss CreditAttribution: jasonkiss commented

Thanks, Everett.

Reading the column header before reading a data cell is what I mean by "prefaces", as in says or reads before by way of introduction. Based on my tests, screen readers almost *always* do this, or can be configured to :)

I did test to see if there was any difference when using a td instead of th, and there is not. JAWS, Window-Eyes, NVDA, and VoiceOver all behave the same, whether it is a td or a th.

What is your view on moving the "Select all rows in this table" checkbox out of the table and placing it before the table? For mouse and sighted keyboard-only users, CSS can be applied so that the "select all" checkbox appears over the top left cell, so they would essentially never know the difference. The benefit is that the individual row checkbox cells would never be introduced by screen readers with the words "Select all rows in the this table".

Cheers

Everett Zufelt’s picture

Everett Zufelt CreditAttribution: Everett Zufelt commented
Version: 7.x-dev » 8.x-dev

@jkiss

My view is that it would be nice to see in D8, but not D7 territory anymore. But, getting titls or labels on the checkboxes definitely is.

Do we have an open issue for that?

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Ultimately, this is about changing the output of the function node_admin_nodes().

I don't think that there is a way to make this function themable, least not in D7, so not sure the best approach to try different structures out for the HTML in a real world environment.

I do wonder if some of this could be brought into D7 using the Accessible Helper Module.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Oh ya, the related issue we're looking at for the checkboxes is #851174: Update options checkbox doesn't include labels

It's sadly still failing the simpletests.

Everett Zufelt’s picture

Everett Zufelt CreditAttribution: Everett Zufelt commented
Title: "select all rows in this table" header checkbox » Tableselect: "select all rows in this table" header checkbox causing accessibility problems
Everett Zufelt’s picture

Everett Zufelt CreditAttribution: Everett Zufelt commented
Issue tags: +Needs issue summary update
mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Ok, so the we need a patch here to bring the select all checkbox outside of the table, but via css position it inside the table.

This is the crux of the html that Jason sent in #8 & that Everett has agreed would be useful.

<form method="post" action="#"><div class="select-all"><input id="select-all2" type="checkbox" class="form-checkbox" title="Select all rows in this table"><label for="select-all2">Select all rows in this table</label></div></form>
<table class="sticky-enabled table-select-processed tableheader-processed sticky-table">
<thead>
<tr>
	<th></th>
	<th><a class="active" title="sort by Title" href="/admin/content?sort=asc&amp;order=Title">Title</a></th>
	<th><a class="active" title="sort by Type" href="/admin/content?sort=asc&amp;order=Type">Type</a></th>
	<th>Author</th>
	<th><a class="active" title="sort by Status" href="/admin/content?sort=asc&amp;order=Status">Status</a></th>
	<th class="active"><a class="active" title="sort by Updated" href="/admin/content?sort=asc&amp;order=Updated">Updated <img title="sort ascending" alt="sort ascending" src="http://drupal7.dev.openconcept.ca/themes/seven/images/arrow-asc.png" typeof="foaf:Image"></a></th>
	<th><a class="active" title="sort by Language" href="/admin/content?sort=asc&amp;order=Language">Language</a></th>
	<th>Operations</th>
</tr>
</thead>
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Active » Needs review
FileSize
core-js-selectall-check-851164.patch1.69 KB

First try it's a little bit different than what it was doing previously. Is is possible to have several checkall checkboxes for one table? If no, there is no problem.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

I don't think I've ever seen the pattern were there are two checkall checkbox columns in a table before. It would be confusing, but I also could see someone trying to do it. Checkall on visible page vs Check all on all pages for instance. Still it's not a pattern that is often used.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

Last time I seen that in drupal was with VBO and they added a link underneath the table header, not a checkbox. I'm still curious to know if the two select-all checkboxes pattern exists somewhere. I'd like it if it wasn't the case but that's not for me to say :)

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

#17: core-js-selectall-check-851164.patch queued for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

So what do we need to bring this to RTBC?
1) Test patch #17 against the latest core
2) Provide a screenshot
3) Review the code changes (all Javascript)
4) Decide to mark it RTBC.

Anything else?

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

#17: core-js-selectall-check-851164.patch queued for re-testing.

seutje’s picture

seutje CreditAttribution: seutje commented
FileSize
Screen Shot 2012-11-02 at 13.12.46.png24.6 KB

1) applies fine
2) see attachment
screenshot
3) Looks fine to me
4) not sure,

but via css position it inside the table.

doesn't seem to be implemented, and seems fragile to implement, is this still a requirement?
also: the first TH is empty, is that acceptable?

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

#17: core-js-selectall-check-851164.patch queued for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: +a11ySprint

The first column TH shouldn't be blank. It should say something about the checkboxes that are in that column. It's a heading & has a semantic value so it shouldn't be blank.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

#17: core-js-selectall-check-851164.patch queued for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Having a blank column isn't great, but in the scheme of things it's more annoying than it is a real barrier as far as I am aware.

Status: Needs review » Needs work

The last submitted patch, core-js-selectall-check-851164.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: +Needs tests

tagging.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Needs review
FileSize
core-js-selectall-check-851164-30.patch17.77 KB

Just a re-roll. No cleanup.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: -Needs tests, -Needs issue summary update, -Accessibility, -a11ySprint

#30: core-js-selectall-check-851164-30.patch queued for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

#30: core-js-selectall-check-851164-30.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs issue summary update, +Accessibility, +a11ySprint

The last submitted patch, core-js-selectall-check-851164-30.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Needs review
FileSize
core-js-selectall-check-851164-34.patch17.69 KB

re-roll.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
FileSize
interdiff-17-34.txt18.59 KB

Providing an interdiff between #17 & #34. I'd done an earlier patch but it was just a re-roll of what @nod_ did earlier.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue tags: -Needs tests, -Needs issue summary update, -Accessibility, -a11ySprint

#34: core-js-selectall-check-851164-34.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs issue summary update, +Accessibility, +a11ySprint

The last submitted patch, core-js-selectall-check-851164-34.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

This should go in after #675446: Use jQuery UI Autocomplete

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue summary: View changes
Issue tags: +Needs reroll
alansaviolobo’s picture

alansaviolobo CreditAttribution: alansaviolobo commented
Status: Needs work » Needs review
FileSize
tableselect_select-851164-40.patch361 bytes
interdiff.txt16.84 KB

it appears to be a case where the patch has effectively been reduced to css changes.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs review » Needs work

@alansaviolobo - I don't understand how the patch addresses this issue.

Did you do any testing with screen readers?

This needs to be tested again, particularly with NVDA, JAWS & WindowsEyes. Also with VoiceOver. I couldn't replicate the problem in VoiceOver, although the description in #8 is very good. That was over 4 years ago now.

alansaviolobo’s picture

alansaviolobo CreditAttribution: alansaviolobo commented

all the changes in the earlier patch have already found their way into the codebase. the only remaining change is contained in the new patch.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Closed (cannot reproduce)

Ok, then lets just close this for now. I don't think that the removal of input.form-autocomplete, does anything for accessibility in this context.

We can re-open it if we find that we can replicate this problem. It may well be fixed rather than cannot reproduce, but would like to have a JAWS user validate that first before doing so.

Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland at University of Waterloo commented

This issue is followed-up in #2766853: Add label to Tableselect select all checkbox.

kwfinken’s picture

kwfinken CreditAttribution: kwfinken as a volunteer and at Michigan State University for Michigan State University commented
Version: 8.0.x-dev » 11.x-dev
Issue summary: View changes
Status: Closed (cannot reproduce) » Needs work
FileSize
Guided Search Axe Dev Tools Error 1.png199.12 KB
Liam Morland’s picture

Liam Morland
Primary language English
Location Ontario, CA 🇨🇦
CreditAttribution: Liam Morland at OpenPlus, Scouting Waterloo Region commented

@kwfinken Is this not covered by #2766853: Add label to Tableselect select all checkbox?

kwfinken’s picture

kwfinken CreditAttribution: kwfinken as a volunteer and at Michigan State University for Michigan State University commented

No, there is an accessible title (not label) on the select all, but not on the individual rows. Slightly different emphasis that seems to go with the earlier discussion on this ticket before it was originally closed. That is why I reopened with better specificity.