new_audit: add new accessibility audits #9798

brendankenny · 2019-10-07T19:14:48Z

fixes #7127
fixes #9794 (?)
part of #9774

These are mostly part of the wcag2a/wcag2aa tags, we just had them turned off before since we didn't have any audits that used the results.

This is part of the 6.0 push, so can't land until we're ready to make that break, but we've needed lots of time in the past to go over phrasing, associated docs, etc.

One issue is not all these things can be tested together in the smoke tests. Specifically

adding an aria-hidden to the <body to fail the aria-hidden-body breaks most other audit failures as hidden stuff usually isn't considered
adding an xml:lang to fail html-xml-lang-mismatch breaks the other lang audit failures

I've left those as notApplicable rather than break other stuff and these are (probably?) relatively rare things.

In addition, messing with <h1>, etc to fail heading-order makes it so bypass doesn't fail. heading-order is looking to be pretty important (see #9794), so bypass now passes.

Originally I made a second a11y_tester to test these things, but <body aria-hidden="true"> alone makes most other tests notApplicable, so it didn't seem worth the large increase in smoke test execution time. Happy to look at other tradeoffs, though.

brendankenny · 2019-10-07T19:19:18Z

oh, and I used the axe doc links for now, but happy to switch before landing to new docs or land and then switch the docs later.

cc @mfriesenhahn

mfriesenhahn · 2019-10-07T21:06:05Z

oh, and I used the axe doc links for now, but happy to switch before landing to new docs or land and then switch the docs later.

I'm writing the docs as we speak, so I think we'll be able to switch the links before landing. If you don't mind granting me access, I can do that directly and fix some typos I found.😄

patrickhulce

sweet! I think the current smoketest tradeoffs all make sense to me 👍

patrickhulce · 2019-10-07T20:47:38Z

lighthouse-cli/test/smokehouse/a11y/expectations.js

+        'aria-hidden-body': {
+          score: 1,
+          details: {
+            'type': 'table',


the lint in this file is going to be fun :)

whaaaaa... how is this allowed!?

'type'
'headings'

madness!

whaaaaa... how is this allowed!?

ohhh, I see what your comment means now. I'm not sure how it's allowed, but it's how all of them are written...

lighthouse-cli/test/smokehouse/a11y/expectations.js

patrickhulce · 2019-10-07T20:48:50Z

lighthouse-cli/test/smokehouse/a11y/expectations.js

@@ -259,6 +394,10 @@ const expectations = [
            ],
          },
        },
+        'html-xml-lang-mismatch': {
+          score: null,
+          scoreDisplayMode: 'notApplicable',


do we specifically assert notApplicable of other a11y audits in here?

I'm just wondering aloud if this is really worth having in here.

do we specifically assert notApplicable of other a11y audits in here?

I'm just wondering aloud if this is really worth having in here.

I I think so. It is asserting that they're specifically notApplicable, which is different than pass (obvs) and fail, which is unfortunately indistinguishable from not having run that test at all. So it is at least testing that the check is running and passing down results.

It also plainly lists the audits that don't have their failure path exercised, and someone making a future second accessibility smoke test can start with those.

All that and more for four extra lines in the expectations file :)

patrickhulce · 2019-10-07T20:50:36Z

lighthouse-core/audits/accessibility/aria-hidden-body.js

+  /** Title of an accesibility audit that checks if the html <body> element does not have an aria-hidden attribute set on it. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed. */
+  failureTitle: '`[aria-hidden="true"]` is present on the document `<body>`',
+  /** Description of a Lighthouse audit that tells the user *why* they should try to pass. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
+  description: 'Assistive technologies, like screen readers, work inconsistently when `aria-hidden="true"` is set on the document `<body>`. [Learn more](https://dequeuniversity.com/rules/axe/3.3/aria-hidden-body).',


seems odd that it would even be allowed since aria is all about assistive technology hints haha

lighthouse-core/audits/accessibility/aria-hidden-focus.js

lighthouse-core/audits/accessibility/duplicate-id-active.js

lighthouse-core/audits/accessibility/form-field-multiple-labels.js

lighthouse-core/audits/accessibility/heading-order.js

lighthouse-core/audits/accessibility/html-xml-lang-mismatch.js

brendankenny · 2019-10-08T17:56:56Z

suggestions have all be really good, thanks!

mfriesenhahn · 2019-10-09T19:27:42Z

Hey, @brendankenny, I noticed an audit name discrepancy: axe has aria-toggle-field-label, while this PR has aria-toggle-field-name. Is that deliberate?

brendankenny · 2019-10-09T19:41:17Z

Hey, @brendankenny, I noticed an audit name discrepancy: axe has aria-toggle-field-label, while this PR has aria-toggle-field-name. Is that deliberate?

yes, sorry, should have flagged this. I'm not sure what the deal is...in their code the axe audit is definitely named aria-toggle-field-name, but their docs list it as aria-toggle-field-label. I haven't looked into why that is or if anyone else has noticed the problem.

brendankenny · 2019-10-09T19:44:53Z

ah, looks like it was dequelabs/axe-core#1656 that renamed to aria-toggle-field-name but the

[x] Has documentation updated, a DU ticket, or requires no documentation change

checked box was incorrect :)

mfriesenhahn · 2019-10-09T19:54:28Z

Cool, aria-toggle-field-name it is!

mfriesenhahn · 2019-10-10T02:04:27Z

Sorry, another question. Here's my understanding of the duplicate ID audits:

duplicate-id-active checks for duplicate IDs in focusable elements.
duplicate-id-aria checks for duplicate IDs in elements referred to in an aria-labelledby attribute on another element. It does not check duplicates for elements with a role or with an aria-label attribute.
duplicate-id checks everything else, including elements with ARIA roles.

If that's right, it strikes me as pretty complex. I'm inferring that axe separated these out because they deemed duplicate-id less critical than the other two, but I'm having trouble formulating an argument to support that position.

What would you think about rolling these three axe audits into a single LH audit? I think that'd make it easier to understand and fix errors, FWIW.

And I kinda want to make the same argument about aria-toggle-field-name and aria-input-field-name. Like, at a certain point, granularity makes it harder to grasp broader concepts (e.g., make sure everything has an accessible name).

brendankenny · 2019-10-15T21:39:34Z

after various conversations with @mfriesenhahn and @robdodson, this is the plan we've landed on. Let me know if I've missed anything:

remove the manual heading-levels audit now that we'll have the non-manual heading-order
remove old catch-all duplicate-id which fails you even if the duplicate ids won't matter for accessibility purposes (ahem). The new duplicate-id-* audits are more focused on specific harmful cases
don't include html-xml-lang-mismatch after all; it's super niche
point descriptions to new web.dev docs being written in Lighthouse a11y updates web.dev#1648, which will likely land before this PR

brendankenny · 2019-10-15T21:42:52Z

lighthouse-core/audits/accessibility/duplicate-id-aria.js

@@ -1,38 +1,36 @@
 /**
- * @license Copyright 2017 Google Inc. All Rights Reserved.
+ * @license Copyright 2019 Google Inc. All Rights Reserved.


after deleting duplicate-id.js git now thinks this was a move + slight change for this file, which is distracting for review but w/e

robdodson · 2019-10-15T21:57:37Z

I think that sounds ok

patrickhulce

LGTM % title prefix business

patrickhulce · 2019-10-15T22:47:25Z

lighthouse-cli/test/smokehouse/a11y/expectations.js

+        'aria-hidden-body': {
+          score: 1,
+          details: {
+            'type': 'table',


whaaaaa... how is this allowed!?

'type'
'headings'

madness!

patrickhulce · 2019-10-15T22:49:36Z

lighthouse-core/audits/accessibility/aria-input-field-name.js

+  /** Title of an accesibility audit that checks that all ARIA input fields have an accessible name. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'All ARIA input fields have accessible names',
+  /** Title of an accesibility audit that checks that all ARIA input fields have an accessible name. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed. */
+  failureTitle: 'Not all ARIA input fields have accessible names',


Suggested change

failureTitle: 'Not all ARIA input fields have accessible names',

failureTitle: 'ARIA input fields do not have accessible names',

patrickhulce · 2019-10-15T22:50:48Z

lighthouse-core/audits/accessibility/aria-input-field-name.js

+
+const UIStrings = {
+  /** Title of an accesibility audit that checks that all ARIA input fields have an accessible name. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'All ARIA input fields have accessible names',


the All prefixes feel out of place compared to most of our audit titles, we only do it twice out of ~100+ audits and those two were external contribs we should probably fix. it's implied in our results that we looked at everything :)

Suggested change

title: 'All ARIA input fields have accessible names',

title: 'ARIA input fields have accessible names',

patrickhulce · 2019-10-15T22:51:00Z

lighthouse-core/audits/accessibility/aria-toggle-field-name.js

+
+const UIStrings = {
+  /** Title of an accesibility audit that checks that all ARIA toggle fields have an accessible name. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'All ARIA toggle fields have accessible names',


Suggested change

title: 'All ARIA toggle fields have accessible names',

title: 'ARIA toggle fields have accessible names',

patrickhulce · 2019-10-15T22:51:12Z

lighthouse-core/audits/accessibility/aria-toggle-field-name.js

+  /** Title of an accesibility audit that checks that all ARIA toggle fields have an accessible name. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'All ARIA toggle fields have accessible names',
+  /** Title of an accesibility audit that checks that all ARIA toggle fields have an accessible name. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed. */
+  failureTitle: 'Not all ARIA toggle fields have accessible names',


Suggested change

failureTitle: 'Not all ARIA toggle fields have accessible names',

failureTitle: 'ARIA toggle fields do not have accessible names',

patrickhulce · 2019-10-15T22:51:33Z

lighthouse-core/audits/accessibility/duplicate-id-aria.js

-  /** Title of an accesibility audit that evaluates if there are any duplicate id HTML attributes on the page. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed. */
-  failureTitle: '`[id]` attributes on the page are not unique',
+  /** Title of an accesibility audit that checks if there are any duplicate ARIA IDs on the page. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'All ARIA IDs are unique',


Suggested change

title: 'All ARIA IDs are unique',

title: 'ARIA IDs are unique',

patrickhulce · 2019-10-15T22:51:41Z

lighthouse-core/audits/accessibility/duplicate-id-aria.js

+  /** Title of an accesibility audit that checks if there are any duplicate ARIA IDs on the page. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'All ARIA IDs are unique',
+  /** Title of an accesibility audit that checks if there are any duplicate ARIA IDs on the page. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed. */
+  failureTitle: 'ARIA IDs are not all unique',


Suggested change

failureTitle: 'ARIA IDs are not all unique',

failureTitle: 'ARIA IDs are not unique',

patrickhulce · 2019-10-15T22:53:48Z

lighthouse-core/audits/accessibility/form-field-multiple-labels.js

+
+const UIStrings = {
+  /** Title of an accesibility audit that checks if any form fields have multiple label elements. This title is descriptive of the successful state and is shown to users when no user action is required. */
+  title: 'No Form fields have multiple labels',


Suggested change

title: 'No Form fields have multiple labels',

title: 'No form fields have multiple labels',

brendankenny requested review from exterkamp, patrickhulce and paulirish as code owners October 7, 2019 19:14

googlebot added the cla: yes label Oct 7, 2019

This was referenced Oct 7, 2019

[meta] Lighthouse 6.0 Burndown #9774

Closed

evaluate new axe audits for inclusion #7127

Closed

brendankenny added the 6.0 label Oct 7, 2019

vercel bot deployed to staging October 7, 2019 19:35 View deployment

patrickhulce suggested changes Oct 7, 2019

View reviewed changes

vercel bot deployed to staging October 7, 2019 21:26 View deployment

vercel bot deployed to staging October 8, 2019 17:57 View deployment

mfriesenhahn mentioned this pull request Oct 10, 2019

Lighthouse a11y updates GoogleChrome/web.dev#1648

Merged

vercel bot deployed to staging October 15, 2019 21:34 View deployment

brendankenny commented Oct 15, 2019

View reviewed changes

vercel bot deployed to staging October 15, 2019 21:46 View deployment

patrickhulce approved these changes Oct 15, 2019

View reviewed changes

brendankenny added 4 commits October 16, 2019 12:36

new_audit: add new accessibility audits

51a4733

update unit test for sample_v2 update

193537a

gah, that snapshot

d516a71

feedback

ad135d8

brendankenny added 6 commits October 16, 2019 12:36

rm accessibility/manual/heading-levels

f71f5f4

rm accessibility/html-xml-lang-mismatch

b3cef83

rm accessibility/duplicate-id

3d7dbe0

point descriptions to new web.dev docs

859f559

enabled order

6545f54

feedback

554b78d

brendankenny force-pushed the newa11y branch from 0abd085 to 554b78d Compare October 16, 2019 16:39

vercel bot deployed to staging October 16, 2019 16:39 View deployment

brendankenny merged commit c1a5ff5 into master Oct 25, 2019

brendankenny deleted the newa11y branch October 25, 2019 15:08

paulirish pushed a commit that referenced this pull request Nov 6, 2019

new_audit: add new accessibility audits (#9798)

65e9047

connorjclark added the new_audit label Aug 24, 2020

paulirish mentioned this pull request Jun 25, 2021

tests: assert what axe checks matches our a11y audits #12699

Merged

paulirish mentioned this pull request Apr 25, 2022

audit the axe audits #13905

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new_audit: add new accessibility audits #9798

new_audit: add new accessibility audits #9798

brendankenny commented Oct 7, 2019

brendankenny commented Oct 7, 2019

mfriesenhahn commented Oct 7, 2019

patrickhulce left a comment

patrickhulce Oct 7, 2019

patrickhulce Oct 15, 2019

brendankenny Oct 16, 2019 •

edited

Loading

patrickhulce Oct 7, 2019

brendankenny Oct 8, 2019

patrickhulce Oct 7, 2019

brendankenny commented Oct 8, 2019

mfriesenhahn commented Oct 9, 2019

brendankenny commented Oct 9, 2019

brendankenny commented Oct 9, 2019

mfriesenhahn commented Oct 9, 2019

mfriesenhahn commented Oct 10, 2019

brendankenny commented Oct 15, 2019

brendankenny Oct 15, 2019

robdodson commented Oct 15, 2019

patrickhulce left a comment

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

patrickhulce Oct 15, 2019

	failureTitle: 'Not all ARIA input fields have accessible names',
	failureTitle: 'ARIA input fields do not have accessible names',

	title: 'All ARIA input fields have accessible names',
	title: 'ARIA input fields have accessible names',

	title: 'All ARIA toggle fields have accessible names',
	title: 'ARIA toggle fields have accessible names',

	failureTitle: 'Not all ARIA toggle fields have accessible names',
	failureTitle: 'ARIA toggle fields do not have accessible names',

	title: 'All ARIA IDs are unique',
	title: 'ARIA IDs are unique',

	failureTitle: 'ARIA IDs are not all unique',
	failureTitle: 'ARIA IDs are not unique',

	title: 'No Form fields have multiple labels',
	title: 'No form fields have multiple labels',

new_audit: add new accessibility audits #9798

new_audit: add new accessibility audits #9798

Conversation

brendankenny commented Oct 7, 2019

brendankenny commented Oct 7, 2019

mfriesenhahn commented Oct 7, 2019

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny Oct 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented Oct 8, 2019

mfriesenhahn commented Oct 9, 2019

brendankenny commented Oct 9, 2019

brendankenny commented Oct 9, 2019

mfriesenhahn commented Oct 9, 2019

mfriesenhahn commented Oct 10, 2019

brendankenny commented Oct 15, 2019

Choose a reason for hiding this comment

robdodson commented Oct 15, 2019

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny Oct 16, 2019 •

edited

Loading