Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call out missing accessible name on various ARIA roles #2421

Closed
11 tasks done
eps1lon opened this issue Jul 27, 2020 · 16 comments
Closed
11 tasks done

Call out missing accessible name on various ARIA roles #2421

eps1lon opened this issue Jul 27, 2020 · 16 comments
Labels
feat New feature or enhancement rules Issue or false result from an axe-core rule
Milestone

Comments

@eps1lon
Copy link
Contributor

eps1lon commented Jul 27, 2020

Expectation: <div role="tree" /> triggers a violation because it does not have an accessible name. Example: https://2m3yw.csb.app/

Actual: No violation reported

Motivation:


axe-core version: 3.5.5
chrome extension: 4.5.3

axe-core should probably report any element without a name that has a role that requires an accessible name.

Rules to add:
Taken from #2421 (comment)

@straker straker added feat New feature or enhancement rules Issue or false result from an axe-core rule labels Jul 27, 2020
@straker
Copy link
Contributor

straker commented Jul 27, 2020

Thanks for the proposal. It's up to @WilcoFiers if we want the new rule, but I'll add some information on how we would write it:

// rules/aria-required-name.json
{
	"id": "aria-required-name",
	"selector": "[role]",
	"metadata": {
		"description": "Ensures elements with ARIA roles that require an accessible name have one",
		"help": "Required accessible name must be present"
	},
	"all": [],
	"any": ["aria-required-name"],
	"none": []
}

The check would then check if the role requires a name (which we would need to add to the standards object, something like requiredName), and then look for an accessible name.

@WilcoFiers
Copy link
Contributor

Tackling this was proposed in one of the oldest still open issues: #470 I think this is a good idea. It goes a little beyond role=tree. We need rules for all of the "requires accessible name" role we don't have one for already, and probably group them into a few rules.

@eps1lon
Copy link
Contributor Author

eps1lon commented Jul 29, 2020

We need rules for all of the "requires accessible name" role we don't have one for already, and probably group them into a few rules.

Naively I would just collect all the roles that aren't currently checked and put them into a new rule. That way we automatically avoid redundant violations. It may help to further divide them by the technique that names them so that it's easier to give actionable advise.

I'll draft a list of all roles that require an accessible name but do not trigger any violation and then it might be obvious how/if we group those missing violations.

@eps1lon
Copy link
Contributor Author

eps1lon commented Jul 30, 2020

Using https://9p5yw.csb.app/ the following roles are not flagged even though they require an accessible name but have none:

If you can adjust the advise based on the role it seems like grouping all these into a single rule makes the most sense. Otherwise we should probably group them by their naming technique because, for example, treeitem allows naming techniques that do not apply to dialog (dialog cannot be named by content).

@WilcoFiers
Copy link
Contributor

WilcoFiers commented Jul 31, 2020

This is something we were thinking to do for axe-core 4.1. Much appreciate the PR. I've gone through the existing ARIA roles, and think we can organise them in the following way:

aria-toggle-field-name - tag: sc412

  • checkbox
  • menuitemcheckbox
  • menuitemradio
  • radio
  • switch
  • option (new)

aria-input-field-name - tag: sc412 (unchanged)

  • combobox
  • listbox
  • searchbox
  • slider
  • spinbutton
  • textbox

role-img-alt - tag: sc111

  • img

aria-command-name (NEW) - tag: sc412

  • menuitem
  • button
  • link

aria-structure-name (NEW) - tag: best-practice

CANCELLED: Requiring all these to have an accessible name is a little questionable. While for many it can help, it is questionable if this is the case for all of them.

  • application
  • form
  • grid
  • table
  • region
  • tree
  • tabpanel
  • treegrid

aria-dialog-name (NEW) - tag: best-practice

  • alertdialog
  • dialog

aria-table-header-name (NEW) - tag: 1.3.1

Needs review only:

  • columnheader
  • rowheader

aria-item-name (NEW) - tag: best-practice

  • treeitem

aria-meter-name (NEW) - tag: sc111

  • meter

aria-progressbar-name (NEW) - tag: sc111

  • progressbar

aria-tooltip-name (NEW) - tag: sc412

  • tooltip

@eps1lon
Copy link
Contributor Author

eps1lon commented Jul 31, 2020

aria-meter-name (NEW) - tag: sc111
progressbar

Is meant to be aria-progressbar-name?

And considering

In addition to Steven's comments, we should break this PR down.

-- #2431 (review)

you prefer each rule in a separate PR or each role in a separate PR?

@WilcoFiers
Copy link
Contributor

Hey @eps1lon, apologies for the slow response.

Is meant to be aria-progressbar-name?

Yes, you're right, my bad.

you prefer each rule in a separate PR or each role in a separate PR?

Each new rule should be in a new PR.

@WilcoFiers WilcoFiers changed the title Trigger violation for a tree without a name Call out missing accessible name on various ARIA roles Aug 21, 2020
@eps1lon
Copy link
Contributor Author

eps1lon commented Aug 21, 2020

@WilcoFiers If you want to group the roles by their "concept" then I don't know how you want to give actionable advise with the current json format of the rules. For example tree gets the name from author only while heading can get it from contents and author. I don't think reporting an error is helpful if no actionable advise is given.

It's also not clear how #2421 (comment) fits into #2431 (review)

I lean more towards #2431 (review) since this focuses on the action that should be taken. I see little value in telling the user that a tree is in the structure group. Or are there some additional axe ressources where you go into detail how structures should be handled?

@WilcoFiers
Copy link
Contributor

Leaving off marque for now. I don't quite see why it requires an accessible name. Opened an issue with the ARIA WG for it: w3c/aria#1339

@eps1lon
Copy link
Contributor Author

eps1lon commented Oct 9, 2020

@WilcoFiers Could you clarify whether you still want to group the rules by concept and what kind of message you would display in aria-structure-name considering that heading can be named by content but tree can't?

@WilcoFiers
Copy link
Contributor

WilcoFiers commented Oct 13, 2020

@eps1lon Thanks for checking back in. You are right, heading doesn't work in aria-structure-map because it is named from content, so it's probably best just left where it is. We'll leave that out in the empty-headings rule. The rest of those roles aren't named from content, so they can be grouped together.

Updating the above comments so it's all up to date again.

@jlin95
Copy link
Contributor

jlin95 commented Oct 15, 2020

I'd like to try tackling aria-treeitem-name (NEW) - tag: best-practice. @WilcoFiers just a heads up!

@WilcoFiers
Copy link
Contributor

Alrighty, update. I've discussed these new rules internally, and gained some new perspective on things. Here's what has been decided:

  • aria-structure-name should not be added. There are lots of scenarios where grid, form, application, etc. do not benefit from having an accessible name. The only one that generally would benefit from getting an accessible name is table, since tables can be pulled up in a list in screen readers, but in that case the best practice should apply to all tables, not just ARIA tables. See Rule idea: Table has accessible name #2603.

  • aria-table-header-name should not be added. Empty table headers are suspicious, but don't necessarily mean there's an accessibility issue. Instead of testing this just for ARIA, it would be better to have a role that checks all table headers, native or ARIA if they have an accessible name and flag it for as needs review if it does not have one. Here's the ticket: Rule idea: flag empty table headers for review #2604

That leaves a few more changes, see the top post of this issue.

@WilcoFiers
Copy link
Contributor

Important: @jlin95 @eps1lon If you'd like to contribute to this issue, we will need your pull request completed by Friday at the latest. Feature freeze for axe-core 4.1 happens on Monday November 2nd. I want to get these rules in by then. Whatever hasn't been completed by Monday morning, @straker and I will finish up that day.

@eps1lon
Copy link
Contributor Author

eps1lon commented Oct 28, 2020

@WilcoFiers Do you know why these names are required per ARIA then? If there are legitimate scenarios where a name doesn't make sense then ARIA requiring them might degrade user experience if devs simply follow ARIA (which is perfectly fine since not everybody can be an expert). But it feels odd that ARIA would require them unless they aren't aware of legitimate use cases.

If you'd like to contribute to this issue,

My original issue was with tree and accessible names. But now you said that aria-structure-name should not be added while not adding a specific rationale for tree. Could you clarify when a tree should not have an accessible name? Maybe we can resolve this with ARIA since it's a bit confusing to me as a component author otherwise.

@padmavemulapati
Copy link

Verified all the new implemented rules: with the latest axe-coconut(v4.6.1.30800) - chrome v86
List of new rules:
aria-command-name
aria-dialog-name
aria-meter-name
aria-progressbar-name
aria-toggle-field-name- feature
aria-tooltip-name
aria-treeitem-name
autocomplete-valid
button-name
link-name

image
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or enhancement rules Issue or false result from an axe-core rule
Projects
None yet
Development

No branches or pull requests

5 participants