Mottie / tablesorter

Github fork of Christian Bach's tablesorter plugin + awesomeness ~
https://mottie.github.io/tablesorter/docs/
2.61k stars 754 forks source link

Accessibility checker fails tablesorter rows due to "role='row'" without corresponding roles on cells #955

Open annames opened 9 years ago

annames commented 9 years ago

Hello,

We use tablesorter extensively and have been very happy with it. We also have a lot of government customers who are concerned about meeting accessibility requirements, and I was psyched to find all of your accessibility enhancements in the thead the last time we pulled a tablesorter update (currently running 2.18.3).

This week I've been updating our previous accessibility checker with the new aXe library http://www.deque.com/products/axe/ , and it reports an error in tablesorter tables because tr's get marked up with "role='row'" but cells within the tbody do not get either "role='rowheader'" or "role='gridcell'". (I see that cells in the thead are nicely marked up with "role='columnheader'".)

Are you planning to add roles on cells at some point, or would you expect page authors to put that in the markup themselves?

Thanks!

Mottie commented 9 years ago

Hi @annames!

Thanks for checking the accessibility of this fork with the new aXe library. I bookmarked it, but I just haven't had the time to follow up.

I think I might have to make an "accessibility" widget to add those role attributes to the tbody content; because, if I add code to the core that adds roles to every tbody row and cell, I fear it may cause more issues for developers that want to display a large amount of data (10K+ rows, see #770).

Eventually, I plan on implementing something like Clusterize into a widget, then it wouldn't be any problem to include the accessibility role attributes.

Mottie commented 9 years ago

Actually, I looked at the following pages:

And now I'm not sure if "gridcell" or "rowheader" should be applied to tbody cells.

The role="row" was added for ARIA support. Interestingly, this page shows it under a label of "Use in fixing an incorrect table structure".

So, now I'm even more confused. What changes need to be made? And what, if anything needs to be removed?

annames commented 9 years ago

Thanks for the quick reply!

The WAI spec http://www.w3.org/TR/wai-aria/roles#row states that if you apply role="row" to a tr, then it must contain columnheader, gridcell, and/or rowheader elements (the "required owned elements"). Columnheader is for th within thead, gridcell is for td within tbody, and rowheader is for th within tbody. I believe that once you apply role="grid" to a table you are required to apply the proper roles to all table elements within it.

However - and here is where my understanding gets a bit fuzzy - I also think that if you do not intend for the cells themselves to be keyboard-navigable, as with a spreadsheet, for example, you do not necessarily need to apply role="grid" to your table. It almost seems like default tablesorter behavior could be to leave out the grid roles altogether, with an add-on widget to provide that sort of behavior if desired. There are many references to role="grid" applying to "interactive" tables, and tablesorter certainly produces interactive tables, but in its most basic function of re-sorting the table rows, it may not be a complex enough interaction to require a grid. But.. I'm really not sure about that.

I can say that to meet the requirements of the people doing accessibility testing for our government customers, the tabindex and nicely-updating aria-label that you've added to header elements are good enough for simple tables; we got dinged (on a previous tablesorter version) for not having accessible sorting controls, so I was very thrilled to get them on our next tablesorter update! I'm not even sure that they would find this problem with grid roles; to my knowledge they are using the Web Accessibility Toolbar for IE http://www.paciellogroup.com/resources/wat/ , which takes a lot more human interaction than the nice, concise report from the axe library. You can use the WAT to show elements that have ARIA roles, but I haven't yet found a tool in there that makes judgement calls on whether or not you're doing it right (though it may be there and I just haven't found it yet).

It is possible that the experience of a sortable table for an actual screen reader user is better with full grid markup, but I couldn't say for sure.

I'm not sure if that helps or not, and now I need to go to a meeting. :-) The one clear thing, though, is that once you start down the path of role="grid", all table markup needs an appropriate grid-related role.

Mottie commented 9 years ago

Getting... more... confused.... LOL

I tested the main demo page on fae (Functional Accessibility Evaluator 2.0). It showed a result of 9 violations, none of which were related to the table (I suck at ARIA):

http://fae20.cita.illinois.edu/one-page/14e311df30ec5378/rc/3/

Does aXe provide suggestions? Maybe I need to ask about this on Stackoverflow.

annames commented 9 years ago

It actually does show up on FAE under Widgets/Scripting. If you go in there there's a violation for "Widgets must have child roles". I tested http://mottie.github.io/tablesorter/docs/example-option-sort-list.html . I think the bottom line is, if you start with role="grid", every tr, th, and td in your table needs a grid-related role.

I have to run and am on vacation for the next week and a half so probably will be offline most of that time. I do think it's worth investigating whether a tablesorter table with just basic sorting really must have a grid role at all, or if it could just use plain table markup. Maybe if you're using aria-live you need that? That's ringing some bells... but I really am not sure of any of this beyond the simple "thou shalt use gridcell and rowheading on tbody td's if thou usest role='grid' on thy table". I will look into it further, just maybe not in the next week and a half...

Thanks again for getting back to me so quickly! I'll check in when I get back.

Mottie commented 9 years ago

I hope you had a nice vacation... I pretty much put this issue on the back burner, but I did investigate and found these interesting sites:

then, I found an aXe extension for Chrome which has been really helpful.

I think to satisfy the requirements, tablesorter will need to add role="gridcell" to every tbody cell. I'm not happy about this fact because some users are telling me about performance issues when they have a table with thousands and thousands of rows with numerous columns. Including this change will only make the performance that much worse.

I am considering adding this missing accessibility requirement into a widget, so users can choose to add it, or not.... I'm not sure what else to do.

annames commented 8 years ago

Hi! Sorry for dropping this thread. I agree with your fix with one exception: for "tbody th" you'd want role="rowheader", while for "tbody td" you'd want role="gridcell", as you said. The aXe library accepts a th with role="gridcell", but since a th is a header, might as well give it role= "rowheader".

It seems totally appropriate to make this a plugin. Or, people can do as I just did and add some code to the initialized event:

initialized: function (table) {
    var $tbody = $(table).find("tbody");
    $tbody.find("th").attr("role", "rowheader");
    $tbody.find("td").attr("role", "gridcell");
}

If you decide to move accessibility options into a plugin, you may be able to remove roles from the base code entirely; the aXe library won't complain about tables that have no roles at all - just ones that have an incomplete set of roles.

Thanks for looking into this, and thanks for your excellent plugin! I think my problem is solved for now.

john-hascall commented 7 years ago

I will be the first to disclaim expertise with ARIA, but here is what I have found was needed to make my tablesorter tables pass the AInspector validator which seems to be among the most thorough.

$(function() {
        var $tbl = $("#requests");
        $tbl.tablesorter( { ... } );
        $tbl.find('thead th').wrapInner('<span role="button"></span>'); 
        $tbl.find('tbody td').attr('role', 'gridcell')
                             .attr('aria-readonly', 'true') 
                             .keypress( function(e) { return false; } );
});

The oddest part to me is the need for the keypress event handler, especially as I marked those gridcells as readonly.