Problem/Motivation

In an issue in a different queue @dmundra figured out how to integrate Axe testing into our Nightwatch tests: #2857808: Automate Accessibility Checks for Core. In the time since that discovery, Nightwatch has added Axe support, so this can be done without any new dependencies.

Steps to reproduce

Proposed resolution

  • Create a profile for a11y testing
  • Create default and admin theme tests that visit a few common pages which are then scanned with Axe
  • Ideally, some of these tests will include a few failing tests, so we can verify Axe is doing its job and establish the value of these tests.
  • Update readme with info on running a11y tests

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Core Nightwatch tests now include Axe accessibility scans that check common pages and forms for accessibility bugs.

Issue fork drupal-3293469

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

bnjmnm created an issue. See original summary.

dmundra made their first commit to this issue’s fork.

dmundra’s picture

@bnjmnm thanks for creating the issue and branch. I pushed up commits to install nightwatch-axe-core and two accessibility tests. To run the tests do I create a merge request?

bnjmnm’s picture

nod_’s picture

Issue tags: +JavaScript

finnsky made their first commit to this issue’s fork.

finnsky’s picture

Status: Active » Needs review

Added possible usage in new MR:

* To run an ally test for olivero and claro.
`yarn test:nightwatch --tag a11y`
* To run an ally test only for claro.
`yarn test:nightwatch --tag adminA11y`
* To run an ally test for custom theme.
`yarn test:nightwatch --tag themeA11y --defaultTheme bartik`
* To run an ally test for custom admin theme.
`yarn test:nightwatch --tag adminA11y --adminTheme classy`

Works with composer installed themes like https://www.drupal.org/project/adminimal_theme
`yarn test:nightwatch --tag adminA11y --adminTheme adminimal_theme`

- Issues wih [['#toolbar-administration']] so it is excluded from axe.conf.

- Should be some command to fill CKEDITOR fields

- For testing default theme should be created new testing profile with some content.

andypost’s picture

Status: Needs review » Needs work

To fix CI

dmundra’s picture

@finnsky those are great additions. Thank you! Should we add it to the other MR?

finnsky’s picture

Yes, this is just example of possible approach for contrib and other themes. I see few issues about it at the moment. Not sure which one is correct

dmundra’s picture

@finnsky I ran eslint fixes to your code and then merged it into our MR. I might make some more changes but I think it is good to have your code in there.

dmundra’s picture

In the last updates we have a few accessibility errors, seen here https://dispatcher.drupalci.org/job/drupal_patches/139557/console. Brief breakdown:

20:39:00 TEST FAILURE: 3 assertions failed, 1976 passed (6m 9s)
20:39:00
20:39:00 ✖ Tests/adminA11yTest
20:39:00 – Accessibility Structure Block Page (3.298s)
20:39:00
20:39:00 ✖ Tests/themeA11yTest
20:39:00 – Accessibility Test Search (737ms)
20:39:00 – Accessibility Test Login (762ms)

finnsky’s picture

Re: #13
Closed MR

finnsky’s picture

As temporary solution we can extend
axe.conf.js with `exclude` option and report in claro/olivero issues.
https://www.deque.com/axe/core-documentation/api-documentation/#include-...

same as excluded '#toolbar-administration' now.
and remove it when issues will fixed

andypost’s picture

StatusFileSize
new709 bytes

I think kinda test should help to catch duplicated IDs on page (we have some tests like it in core - use git grep pageContainsNoDuplicateId

andypost’s picture

dmundra’s picture

@finnsky if that is acceptable. That was my question when starting this.

Nice find @andypost that would probably help use TDD to fix.

finnsky’s picture

So imo next steps should be:

1. Create all pages tests listed in https://www.drupal.org/project/drupalci/issues/2857808
2. Gather all core a11y issues.
3. Add them to `exclude` part of axe configuration.
4. Create/update tickets for fixes. With links to `excluded` lines.

dmundra’s picture

That makes sense @finnsky. A semantic question I had, should we add separate tests for those or try to incorporate an axe() check to existing tests for those pages? I would prefer the latter to encourage a11y tests being added to potentially any tests specially interactive ones. Maybe a11y only tests should go in a folder called 'a11y_only' or something like that?

bnjmnm’s picture

StatusFileSize
new2.24 KB
new101.43 KB

Whoa! This is moving along nicely. This is in the shape of something I'd likely RTBC once some details are addressed. Here are some more general things we'd need done for me to do that:

  • axe-core and nightwatch-axe-core both require a dependency evaluation. See #3201547: [Policy] Dependency evaluation critera for some thoughts on how to approach evaluating dev only dependencies (it can be a little more relaxed).
  • I'm adding a "nightwatch control case" patch that runs core's Nightwatch tests without the A11y tests added here to see if the A11y tests increase test run times more than a typical Nightwatch test. It would be good to have confirmation these tests don't add significant time to test duration.
  • Since there are now legitimate tests in the MR, I think we can get rid of the exampleA11y test.
  • Perhaps I'm missing something, but the output on test failures don't seem to provide information outside the test doesn't pass. There needs to be more information on what specific issues were found. . I suspect this is possible via a configuration setting, I can't imagine Axe not making this info available in some way.
  • It looks like these tests use the standard profile. I've seen tests get kicked back for using standard profile, requesting that only 100% necessary extensions are enabled. For a11y tests, there may be legitimate benefits to using the standard profile vs. targeted enabling, especially since extension enabling in Nightwatch tests is more expensive than with other tests. It's probably worth having some justification prepared as to why it's best to use the standard profile, as it will undoubtedly be a concern somewhere in the review chain.
dmundra’s picture

StatusFileSize
new156.1 KB

Thanks @bnjmnm. Here are my comments:

  • I can take a look at the dependency evaluations. I will work on that.
  • If I am reading this correctly your "nightwatch control case" patch showed 10 mins total time (https://dispatcher.drupalci.org/job/drupal_patches/140339/). The current MR run is 14 mins total time (https://dispatcher.drupalci.org/job/drupal_patches/139593/). I think the addition of 4 mins is worth it. What do you think?
  • I will remove the exampleA11y test.
  • Actually if you scroll up to the individual test you should see the Axe outputs. Screenshot of 'Accessibility Test Login' test errors from console.
  • Ah I didn't know about the standard profile for tests. I will dig into that as well.
dmundra’s picture

Dependency evaluation for axe-core and nightwatch-axe-core:

axe-core

Repository https://github.com/dequelabs/axe-core
Release cycle Regular. Minor release every 3 to 5 months, which usually introduces new rules and features. Recommended to schedule time for upgrade. Details at Axe-core Release & Support Policy.
Security policies Security updates will be provided for major and minor versions up to 18 months old. This is a dev tool and will not be present on production.
Security issue reporting N/a
Contact(s) https://www.deque.com/company/contact/

nightwatch-axe-core

Repository https://github.com/rikki-iki/nightwatch-axe-core
Release cycle N/a
Security policies None. This is a dev tool and will not be present on production.
Security issue reporting N/a
Contact(s) https://github.com/rikki-iki
dmundra’s picture

With the default nightwatch profile (https://dispatcher.drupalci.org/job/drupal_patches/141154/console) we only see 1 a11y error:

09:27:22 - Running Accessibility Content Page:
09:27:22
09:27:22 -----1 aXe violations-----
09:27:23 #1: Contentinfo landmark should not be contained in another landmark (landmark-contentinfo-is-top-level)
09:27:23 Impact: moderate
09:27:23 Count: 1
09:27:23 See: https://dequeuniversity.com/rules/axe/4.4/landmark-contentinfo-is-top-level
09:27:23 #1.1: Fix any of the following:
09:27:23 The contentinfo landmark is contained in another landmark.
09:27:23

09:29:47 TEST FAILURE: 1 assertions failed, 1780 passed (5m 23s)
09:29:47
09:29:47 ✖ Tests/adminA11yTest
09:29:47 – Accessibility Content Page (1.094s)

finnsky’s picture

Status: Needs work » Needs review

Added nightwatch_a11y_testing minimal profile for current tests.

dmundra’s picture

Thanks @finnsky. With the 'nightwatch_a11y_testing' minimal profile here is the current accessibility errors:

On '/admin/structure/block' page:

04:50:23 - Running Accessibility Structure Block Page:
04:50:23 
04:50:23 -----3 aXe violations-----
04:50:25 #1: Elements must have sufficient color contrast (color-contrast)
04:50:25 Impact: serious
04:50:25 Count: 4
04:50:25 See: https://dequeuniversity.com/rules/axe/4.4/color-contrast
04:50:25 #1.1: Fix any of the following:
04:50:25   Element has insufficient color contrast of 3.78 (foreground color: #828388, background color: #ffffff, font size: 12.0pt (16px), font weight: normal). Expected contrast ratio of 4.5:1
04:50:25   <em>No blocks in this region</em>
04:50:25 #1.2: Fix any of the following:
04:50:25   Element has insufficient color contrast of 3.78 (foreground color: #828388, background color: #ffffff, font size: 12.0pt (16px), font weight: normal). Expected contrast ratio of 4.5:1
04:50:25   <em>No blocks in this region</em>
04:50:25 #1.3: Fix any of the following:
04:50:25   Element has insufficient color contrast of 3.78 (foreground color: #828388, background color: #ffffff, font size: 12.0pt (16px), font weight: normal). Expected contrast ratio of 4.5:1
04:50:25   <em>No blocks in this region</em>
04:50:25 #1.4: Fix any of the following:
04:50:25   Element has insufficient color contrast of 3.78 (foreground color: #828388, background color: #ffffff, font size: 12.0pt (16px), font weight: normal). Expected contrast ratio of 4.5:1
04:50:25   <em>No blocks in this region</em>
04:50:25 
04:50:25 #2: IDs of active elements must be unique (duplicate-id-active)
04:50:25 Impact: serious
04:50:25 Count: 13
04:50:25 See: https://dequeuniversity.com/rules/axe/4.4/duplicate-id-active
04:50:25 #2.1: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-header-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-header-title">
04:50:25 #2.2: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-primary-menu-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-primary-menu-title">
04:50:25 #2.3: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-secondary-menu-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-secondary-menu-title">
04:50:25 #2.4: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-hero-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-hero-title">
04:50:25 #2.5: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-highlighted-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-highlighted-title">
04:50:25 #2.6: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-breadcrumb-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-breadcrumb-title">
04:50:25 #2.7: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-social-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-social-title">
04:50:25 #2.8: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-content-above-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-content-above-title">
04:50:25 #2.9: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-content-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-content-title">
04:50:25 #2.10: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-sidebar-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-sidebar-title">
04:50:25 #2.11: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-content-below-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-content-below-title">
04:50:25 #2.12: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-footer-top-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-footer-top-title">
04:50:25 #2.13: Fix any of the following:
04:50:25   Document has active elements with the same id attribute: edit-blocks-region-footer-bottom-title
04:50:25   <div class="region-title__action js-form-wrapper form-wrapper" id="edit-blocks-region-footer-bottom-title">
04:50:25 
04:50:25 #3: All page content should be contained by landmarks (region)
04:50:25 Impact: moderate
04:50:25 Count: 1
04:50:25 See: https://dequeuniversity.com/rules/axe/4.4/region
04:50:25 #3.1: Fix any of the following:
04:50:25   Some page content is not contained by landmarks
04:50:25   <h2 id="secondary-tabs-title" class="visually-hidden">Secondary tabs</h2>

On the home page:

04:52:44 - Running Accessibility Homepage:
04:52:48 
04:52:48 -----1 aXe violations-----
04:52:49 #1: All page content should be contained by landmarks (region)
04:52:49 Impact: moderate
04:52:49 Count: 1
04:52:49 See: https://dequeuniversity.com/rules/axe/4.4/region
04:52:49 #1.1: Fix any of the following:
04:52:49   Some page content is not contained by landmarks
04:52:49   <h2 id="primary-tabs-title" class="visually-hidden">Primary tabs</h2>

On the search page:

04:52:50 - Running Accessibility Test Search:
04:52:50 
04:52:50 -----2 aXe violations-----
04:52:50 #1: IDs used in ARIA and labels must be unique (duplicate-id-aria)
04:52:50 Impact: critical
04:52:50 Count: 1
04:52:50 See: https://dequeuniversity.com/rules/axe/4.4/duplicate-id-aria
04:52:50 #1.1: Fix any of the following:
04:52:50   Document has multiple elements referenced with ARIA with the same id attribute: edit-keys
04:52:50   <input title="Enter the terms you wish to search for." placeholder="Search by keyword or phrase." data-drupal-selector="edit-keys" type="search" id="edit-keys" name="keys" value="" size="15" maxlength="128" class="form-search form-element form-element--type-search form-element--api-search">
04:52:50 
04:52:50 #2: Heading levels should only increase by one (heading-order)
04:52:50 Impact: moderate
04:52:50 Count: 1
04:52:50 See: https://dequeuniversity.com/rules/axe/4.4/heading-order
04:52:50 #2.1: Fix any of the following:
04:52:50   Heading order invalid
04:52:50   <h3>Your search yielded no results.</h3>

On the login page:

04:52:50 - Running Accessibility Test Login:
04:52:50 
04:52:50 -----1 aXe violations-----
04:52:51 #1: All page content should be contained by landmarks (region)
04:52:51 Impact: moderate
04:52:51 Count: 1
04:52:51 See: https://dequeuniversity.com/rules/axe/4.4/region
04:52:51 #1.1: Fix any of the following:
04:52:51   Some page content is not contained by landmarks
04:52:51   <h2 id="primary-tabs-title" class="visually-hidden">Primary tabs</h2>

My suggestion is to do the following:

  • Comment out the 'Accessibility Structure Block Page' test and document the a11y errors
  • Comment out the 'Accessibility Test Search' test.
  • Exclude 'primary-tabs-title'.

bnjmnm credited rikki_iki.

bnjmnm’s picture

Status: Needs review » Needs work

Green MR!

Because dependencies can make it difficult for an issue to land in core, and https://github.com/rikki-iki/nightwatch-axe-core doesn't have a regular release cycle, I checked with @rikki_iki to see if it was fine directly adding some version of that Nightwatch axe command directly to core. It can largely be the same, but with any needed code standard refactoring and removing any blocks that aren't needed for Drupal core testing.

The resource is small and effective enough that I don't think it'll add a maintenance burden to core - and if it does it's arguably easier than coordinating a new release.

So TLDR, lets move nightwatch-axe-core into core's Nightwatch code and this will be an easier sell.

dmundra’s picture

Status: Needs work » Needs review

Thanks @bnjmnm. That makes sense. I will work on that.

dmundra’s picture

Status: Needs review » Active
dmundra’s picture

Status: Active » Needs review

I moved the nightwatch-axe-core package into Nightwatch/Commands folder. I used eslint to adjust the code styling and I left the eslint-disable items. If those are show stoppers than I could use some help to convert the code to pass eslint requirements. In any case back to needs review.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bnjmnm’s picture

Here's an intentionally failing patch to demonstrate the Axe checks are doing what we expect

bnjmnm’s picture

Apparently Nightwatch now includes Axe support. There's already an issue to update Nightwatch in core, so once it lands we'll have the Axe integration. The issue is #3306446: Update Nightwatch to 2.4.1.

dmundra’s picture

Nice that will definitely make it even easier to add a11y tests.

anchal_gupta’s picture

StatusFileSize
new32.71 KB
new823 bytes

I have Fixed CS error. Please review it

bnjmnm’s picture

Title: [Plan] Automated A11y tests in Nightwatch » Automated A11y tests in Nightwatch
Issue summary: View changes
Related issues: +#3318394: Block UI A11y errors identified with Axe + Nightwatch, +#3318396: Address Primary tabs "missing region" A11y test failure, +#3318398: Address A11y test fails in search/node
StatusFileSize
new28.81 KB

This is a version of the MR without the skipped tests to demonstrate the Axe checker is doing its job.

I also filed followup issues for the rules that are necessary to skip for tests to pass. They are all noted with a @todo in the test code.

dmundra’s picture

Thank you @bnjmnm. This is looking good!

nod_’s picture

couple of questions in the MR

nod_’s picture

Status: Needs review » Needs work

for addressing the few comments and rerolling the MR

bnjmnm’s picture

Status: Needs work » Needs review

Feedback and reroll needs addressed.

nod_’s picture

Status: Needs review » Reviewed & tested by the community

Pretty minor comments. In any case it works, a11y tests are run, I didn't see any obvious problems so RTBC.

quietone’s picture

I'm thinking this should have a CR to explain how to run these tests and a release note too.

bnjmnm’s picture

Issue summary: View changes
Issue tags: -Needs change record

Added CR and release notes snippet

nod_’s picture

Reviewed the changes to code and change notice, RTBC +1

mgifford’s picture

I think we'll need a new issue shortly on how to document this and include it as part of the Testing Core Gate.

It is also going to have an impact on the Accessibility Coding Standards document.

quietone’s picture

+1. I was just coming here to leave a comment about the Core gate and see it is already done!

xjm’s picture

Issue tags: +Needs followup

Based on number #52 to it sounds like a follow-up needs to be filed?

xjm’s picture

xjm’s picture

Also could you move the dependency evaluation issue to the issue summary? Thanks!

bnjmnm’s picture

Also could you move the dependency evaluation issue to the issue summary? Thanks!

I believe the first paragraph of the issue summary already does this? If there's there's something additional that should be there, specify and it can get taken care of.

lauriii’s picture

Which dependency is #56 requesting to evaluate? There are no new dependencies being added in this issue. Automated accessibility testing is a feature of Nightwatch which was added in 2.3.0: https://nightwatchjs.org/guide/using-nightwatch/accessibility-testing.html.

I didn't review any of the config changes in the MR in detail but the approach seems fine. I don't didn't see anything controversial there. Looking forward to having these tests running automatically as part of the test suite! Thanks everyone who worked on this.

xjm’s picture

#25 contains a dependency evaluation. I guess the issue changed scope since then?

xjm’s picture

Hiding confusing patches.

xjm’s picture

I think this should probably be documented both on the accessibility and testing gates and the frontend developer tools?

Edit: Or basically what was already said in #52.

xjm’s picture

Also is the release note accurate if the Axe dependency is no longer being added?

xjm’s picture

Status: Reviewed & tested by the community » Needs work

NW for docs updates and to clarify the CR and release note: Either this is making better use of something that is already a dependency, or the dependency was a prior approach that was abandoned.

VladimirAus made their first commit to this issue’s fork.

bnjmnm’s picture

contains a dependency evaluation. I guess the issue changed scope since then?

Yes, the issue changed scope and the issue summary was changed accordingly on Nov 1 2022 Since that change, the opening sentence of the issue summary has been (and continues to be)

In the time since that discovery, Nightwatch has added Axe support, so this can be done without any new dependencies.

Also is the release note accurate if the Axe dependency is no longer being added?

The release note is accurate. Axe support is now included with Nightwatch. Added October 2022 #3306446: Update Nightwatch to 2.4.1

Either this is making better use of something that is already a dependency, or the dependency was a prior approach that was abandoned.

Neither. When work on this issue started, Nightwatch did not have built-in Axe support. Nightwatch introduced Axe support in version 2.3.6. The overall approach is effectively the same, but can now be done directly via the Nightwatch API.

NW for docs updates and to clarify the CR

The CR is accurate and was written after Nightwatch introduced built in Axe support. While I could add points that clarify what the change isn't, I think it's best and least confusing to report soley on what it is.

bnjmnm’s picture

Removing needs issue summary update as the tag was added requesting a dependency evaluation and no new dependency is being added.

Removing docs tag as the additions are below. These can get added to the documentation pages when this issue lands.

**Accessiblity Gates**

Will add to https://www.drupal.org/about/core/policies/core-change-policies/core-gat...

The change does not result in any Nighwatch + Axe test failures.

Drupal's Nightwatch tests include basic accessibliity scans with Axe as of Drupal 10.1. a11yTestAdmin.js scans a set of admin pages using Claro as the theme, and a11yTestDefault.js scans a set of default pages using Olivero as the theme. (These tests can be configured to run with other themes, but Claro/Olivero is what is used in core tests). Like any change to Drupal, all tests must pass for a change to be considered for core inclusion. If a change introdces a failure in an A11y test, it must either be addressed, or there needs to be sufficient evidence that the failure was a false positive, in which case that specific A11y assertion can be bypassed.

More information on the Nightwatch + Axe testing can be found in core/tests/README.md

** Testing Gates**

Will Add to https://www.drupal.org/about/core/policies/core-change-policies/core-gat...

If a new UI element or pattern is introduced, it should be included somewhere in the Nightwatch accessbility scans.

From Drupal 10.1 forward, Nightwatch runs Axe accessibility scans on a set of admin/default pages. The tests are run using the nightwatch_a11y_testing profile. If a change to Drupal introduces any kind of element/interaction/etc that is distinct enough from existing elements to have different accessiblity concerns, it should be added to one of the pages scanned in the automated accessiblity tests. If it can't be easily added to an existing page scanned in the tests, a new page should be added to the nightwatch_a11y_testing profile

The tests should be added to one (or both) of the two Nightwatch A11y tests: a11yTestAdmin.js, which scans a set of admin pages using Claro as the theme, and/or a11yTestDefault.js, which scans a set of default pages using Olivero as the theme

More information on the Nightwatch + Axe testing can be found in core/tests/README.md

** Frontend Developer Tools**

A line added to https://www.drupal.org/about/core/policies/core-change-policies/frontend...

Under the yarn command under the "Run Tests" h2 add "As of Drupal 10.1 Nightwatch tests include basic Axe accessiblity scans "

mgifford’s picture

Very cool. Thanks so much @bnjmnm

In #52 I was just thinking that the accessibility standards documentation that we proposed as part of the GAAD Pledge (but that haven't been approved by the Coding Standards team).

I made some slight changes to your draft, but I think much of it is going to be basically the same (spellcheck these before posting). I am not sure where we will be able to see the results of these tests on D.o. Also not sure how well this applies to Drupal Modules/Themes. Suspect the Webform module is the first one that might take advantage of this. For the most part, Nightwatch is still largely being used by Core.

Nightwatch-axe tests

From Drupal 10.1 forward, Nightwatch runs Axe accessibility scans on a set of Core admin/default pages. The tests are run using the nightwatch_a11y_testing profile. If a change to Drupal introduces any kind of element/interaction/etc. that is distinct enough from existing elements to have different accessibility concerns, it should be added to one of the pages scanned in the automated accessibility tests. If it can't be easily added to an existing page scanned in the tests, a new page should be added to the nightwatch_a11y_testing profile.

The tests should be added to one (or both) of the two Nightwatch A11y tests: a11yTestAdmin.js, which scans a set of admin pages using Claro as the theme, and/or a11yTestDefault.js, which scans a set of default pages using Olivero as the theme.

It is recommended that where possible, nightwatch-axe tests are added to test dynamic elements that are using Nightwatch. This will expand our accessibility coverage and reduce the need for manual testing.

More information on the Nightwatch + Axe testing can be found in core/tests/README.md

bnjmnm’s picture

Status: Needs work » Needs review
Issue tags: -Needs release manager review, -Needs followup

@mgifford confirmed the "needs followup" he added was addressed by the doc snippets I provided above and the tag can be removed. I'm setting it back to needs review as the docs are done and the CR and IS are current and accurate.

catch’s picture

Category: Plan » Task

Moving this to a task since it's an implementation issue.

vladimiraus’s picture

Hiding patches to concentrate on MR.

lauriii’s picture

Leaving few thoughts since @bnjmnm reached out for a review.

  1. I think we should move the core gate changes to a new issue. That way we can focus on the actual tool that is being added in this issue, not the change control around changing the core gates.
  2. I'm not sure about what should we mention in the release notes since this should not have any impact on end users. However, what is there seems accurate.
  3. The change records is accurate and seems useful for someone who is interested in using this tooling to test their theme.
  4. It would be helpful to update the docs to include the new a11y specific tags, and the new arguments for selecting a theme.

I'd be comfortable moving this to RTBC after the core gate changes are moved to a separate issue.

bnjmnm’s picture

@lauriii makes a good point about moving the core gates portion to a separate issue, and I created #3339065: [PP-1] Consider updating core gates to include automated Nightwatch A11y tests as a followup. These tests benefit Drupal accessibility whether or not they're part of a defined gate, and their presence do not alter how existing defined gates work.

andy-blum’s picture

Adding #3301173: Allow starterkit theme generator tool to clone Olivero as a related issue as we intend to use the testing methods introduced here.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: -JavaScript +JavaScript, +Needs Review Queue Initiative

Seems the MR 2918 has some failures.

bnjmnm’s picture

Status: Needs work » Needs review
Related issues: +#3349028: Improper use of aria-label in "System Powered By" block

Added #3349028: Improper use of aria-label in "System Powered By" block, which is a newly-caught A11y error by the tests added here. The aria-allowed-attr tests need to be bypassed for the default theme until that issue is resolved.

smustgrave’s picture

Status: Needs review » Needs work

Seems to be a valid failure. But using this to help test 3349028

bnjmnm’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

All the tests now pass.
Undoing the change 'aria-allowed-attr': { enabled: false }, that @bnjmnm added I can see the error when running

ddev exec -d /var/www/html/web/core yarn test:nightwatch tests/Drupal/Nightwatch/Tests/a11yTestDefault.js

So appears to be doing its job

nod_’s picture

Status: Reviewed & tested by the community » Needs work

#3349028: Improper use of aria-label in "System Powered By" block has been committed, we can remove 'aria-allowed-attr': { enabled: false }

bnjmnm’s picture

Status: Needs work » Reviewed & tested by the community

Removed the @todos and the rule skipping for the now-closed #3349028: Improper use of aria-label in "System Powered By" block.

@VladimirAus if an issue needs work could you not merge HEAD without if it isn't accompanied by additional changes working towards completing this? Whoever is working on the work-that-needs-to-be-done can get the MR current with HEAD pretty easily, it doesn't really need to be done as an isolated task.

  • lauriii committed 9e84cb4e on 10.1.x
    Issue #3293469 by bnjmnm, dmundra, finnsky, Anchal_gupta, andypost,...

lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 9e84cb4 and pushed to 10.1.x. Thanks!

wim leers’s picture

EPIC work here! 🤩

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

bnjmnm’s picture