From 03f26187b6d9ed31293c2d414dee4d11c7f2c6cd Mon Sep 17 00:00:00 2001 From: Albina <51043550+albinazs@users.noreply.github.com> Date: Thu, 27 Jun 2024 11:33:46 +0300 Subject: [PATCH] Add alt text validation rule in the accessibility checker (#11986) Co-authored-by: Thibaud Colas --- CHANGELOG.txt | 1 + client/src/entrypoints/admin/preview-panel.js | 13 +- client/src/includes/a11y-result.test.ts | 116 +++++++++++++++++- client/src/includes/a11y-result.ts | 77 +++++++++++- client/src/includes/userbar.ts | 16 +-- .../accessibility_considerations.md | 8 ++ docs/releases/6.2.md | 5 + wagtail/admin/tests/test_userbar.py | 71 +++++++++++ wagtail/admin/userbar.py | 44 +++++++ 9 files changed, 328 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 537b80427999..8c3805043f4f 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -15,6 +15,7 @@ Changelog * Remove reduced opacity for draft page title in listings (Inju Michorius) * Adopt more compact representation for StreamField definitions in migrations (Matt Westcott) * Implement a new design for locale labels in listings (Albina Starykova) + * Add alt text validation rule in the accessibility checker (Albina Starykova) * Fix: Make `WAGTAILIMAGES_CHOOSER_PAGE_SIZE` setting functional again (Rohit Sharma) * Fix: Enable `richtext` template tag to convert lazy translation values (Benjamin Bach) * Fix: Ensure permission labels on group permissions page are translated where available (Matt Westcott) diff --git a/client/src/entrypoints/admin/preview-panel.js b/client/src/entrypoints/admin/preview-panel.js index 353ac846f575..e887d38d3671 100644 --- a/client/src/entrypoints/admin/preview-panel.js +++ b/client/src/entrypoints/admin/preview-panel.js @@ -1,6 +1,6 @@ -import axe from 'axe-core'; import { getAxeConfiguration, + getA11yReport, renderA11yResults, } from '../../includes/a11y-result'; import { WAGTAIL_CONFIG } from '../../config/wagtailConfig'; @@ -32,23 +32,18 @@ const runAccessibilityChecks = async (onClickSelector) => { } // Ensure we only test within the preview iframe, but nonetheless with the correct selectors. - const context = { + config.context = { include: { fromFrames: ['#preview-iframe'].concat(config.context.include), }, }; if (config.context.exclude?.length > 0) { - context.exclude = { + config.context.exclude = { fromFrames: ['#preview-iframe'].concat(config.context.exclude), }; } - const results = await axe.run(context, config.options); - - const a11yErrorsNumber = results.violations.reduce( - (sum, violation) => sum + violation.nodes.length, - 0, - ); + const { results, a11yErrorsNumber } = await getA11yReport(config); toggleCounter.innerText = a11yErrorsNumber.toString(); toggleCounter.hidden = a11yErrorsNumber === 0; diff --git a/client/src/includes/a11y-result.test.ts b/client/src/includes/a11y-result.test.ts index 1bc9133fe52f..fee973217769 100644 --- a/client/src/includes/a11y-result.test.ts +++ b/client/src/includes/a11y-result.test.ts @@ -1,5 +1,11 @@ -import { AxeResults } from 'axe-core'; -import { sortAxeViolations } from './a11y-result'; +import axe, { AxeResults, Spec } from 'axe-core'; +import { + sortAxeViolations, + WagtailAxeConfiguration, + addCustomChecks, + checkImageAltText, + getA11yReport, +} from './a11y-result'; const mockDocument = `
@@ -55,3 +61,109 @@ describe('sortAxeViolations', () => { ]); }); }); + +describe('addCustomChecks', () => { + it('should integrate custom checks into the Axe spec', () => { + const spec: Spec = { + checks: [{ id: 'check-id', evaluate: 'functionName' }], + rules: [ + { + id: 'rule-id', + impact: 'serious', + any: ['check-id'], + }, + ], + }; + const modifiedSpec = addCustomChecks(spec); + const customCheck = modifiedSpec?.checks?.find( + (check) => check.id === 'check-id', + ); + expect(customCheck).toBeDefined(); + expect(customCheck?.evaluate).toBe('functionName'); + }); + + it('should return spec unchanged if no custom checks match', () => { + const spec: Spec = { + checks: [{ id: 'non-existent-check', evaluate: '' }], + }; + const modifiedSpec = addCustomChecks(spec); + expect(modifiedSpec).toEqual(spec); + }); +}); + +// Options for checkImageAltText function +const options = { + pattern: '\\.(avif|gif|jpg|jpeg|png|svg|webp)$', +}; + +describe.each` + text | result + ${'Good alt text with words like GIFted and motif'} | ${true} + ${'Bad alt text.png'} | ${false} + ${''} | ${true} +`('checkImageAltText', ({ text, result }) => { + const resultText = result ? 'should not be flagged' : 'should be flagged'; + test(`alt text: "${text}" ${resultText}`, () => { + const image = document.createElement('img'); + image.setAttribute('alt', text); + expect(checkImageAltText(image, options)).toBe(result); + }); +}); + +describe('checkImageAltText edge cases', () => { + test('should not flag images with no alt attribute', () => { + const image = document.createElement('img'); + expect(checkImageAltText(image, options)).toBe(true); + }); +}); + +jest.mock('axe-core', () => ({ + configure: jest.fn(), + run: jest.fn(), +})); + +describe('getA11yReport', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should configure Axe with custom rules and return the accessibility report', async () => { + const mockResults = { + violations: [ + { + nodes: [{}, {}, {}], // 3 nodes with violations + }, + ], + }; + (axe.run as jest.Mock).mockResolvedValue(mockResults); + const config: WagtailAxeConfiguration = { + context: 'body', + options: {}, + messages: {}, + spec: { + checks: [{ id: 'check-image-alt-text', evaluate: '' }], + }, + }; + const report = await getA11yReport(config); + expect(axe.configure).toHaveBeenCalled(); + expect(axe.run).toHaveBeenCalledWith(config.context, config.options); + expect(report.results).toEqual(mockResults); + expect(report.a11yErrorsNumber).toBe(3); + }); + + it('should return an accessibility report with zero errors if there are no violations', async () => { + const mockResults = { + violations: [], + }; + (axe.run as jest.Mock).mockResolvedValue(mockResults); + const config: WagtailAxeConfiguration = { + context: 'body', + options: {}, + messages: {}, + spec: { + checks: [{ id: 'check-image-alt-text', evaluate: '' }], + }, + }; + const report = await getA11yReport(config); + expect(report.a11yErrorsNumber).toBe(0); + }); +}); diff --git a/client/src/includes/a11y-result.ts b/client/src/includes/a11y-result.ts index 3b208f450d46..45ee165c9c0c 100644 --- a/client/src/includes/a11y-result.ts +++ b/client/src/includes/a11y-result.ts @@ -1,9 +1,10 @@ -import { +import axe, { AxeResults, ElementContext, NodeResult, Result, RunOptions, + Spec, } from 'axe-core'; const toSelector = (str: string | string[]) => @@ -39,10 +40,11 @@ export const sortAxeViolations = (violations: Result[]) => * Wagtail's Axe configuration object. This should reflect what's returned by * `wagtail.admin.userbar.AccessibilityItem.get_axe_configuration()`. */ -interface WagtailAxeConfiguration { +export interface WagtailAxeConfiguration { context: ElementContext; options: RunOptions; messages: Record; + spec: Spec; } /** @@ -70,6 +72,77 @@ export const getAxeConfiguration = ( return null; }; +/** + * Custom rule for checking image alt text. This rule checks if the alt text for images + * contains poor quality text like file extensions. + * The rule will be added via the Axe.configure() API. + * https://github.com/dequelabs/axe-core/blob/master/doc/API.md#api-name-axeconfigure + */ +export const checkImageAltText = ( + node: HTMLImageElement, + options: { pattern: string }, +) => { + const altTextAntipatterns = new RegExp(options.pattern, 'i'); + const altText = node.getAttribute('alt') || ''; + + const hasBadAltText = altTextAntipatterns.test(altText); + return !hasBadAltText; +}; + +/** + * Defines custom Axe rules, mapping each check to its corresponding JavaScript function. + * This object holds the custom checks that will be added to the Axe configuration. + */ +export const customChecks = { + 'check-image-alt-text': checkImageAltText, + // Add other custom checks here +}; + +/** + * Configures custom Axe rules by integrating the custom checks with their corresponding + * JavaScript functions. It modifies the provided configuration to include these checks. + */ +export const addCustomChecks = (spec: Spec): Spec => { + const modifiedChecks = spec?.checks?.map((check) => { + if (customChecks[check.id]) { + return { + ...check, + evaluate: customChecks[check.id], + }; + } + return check; + }); + return { + ...spec, + checks: modifiedChecks, + }; +}; + +interface A11yReport { + results: AxeResults; + a11yErrorsNumber: number; +} + +/** + * Get accessibility testing results from Axe based on the configurable custom spec, context, and options. + * It integrates custom rules into the Axe configuration before running the tests. + */ +export const getA11yReport = async ( + config: WagtailAxeConfiguration, +): Promise => { + axe.configure(addCustomChecks(config.spec)); + // Initialise Axe based on the context and options defined in Python. + const results = await axe.run(config.context, config.options); + const a11yErrorsNumber = results.violations.reduce( + (sum, violation) => sum + violation.nodes.length, + 0, + ); + return { + results, + a11yErrorsNumber, + }; +}; + /** * Render A11y results based on template elements. */ diff --git a/client/src/includes/userbar.ts b/client/src/includes/userbar.ts index 7ad2e0048e3b..5a138c76dd1d 100644 --- a/client/src/includes/userbar.ts +++ b/client/src/includes/userbar.ts @@ -1,8 +1,10 @@ -import axe from 'axe-core'; - import A11yDialog from 'a11y-dialog'; import { Application } from '@hotwired/stimulus'; -import { getAxeConfiguration, renderA11yResults } from './a11y-result'; +import { + getAxeConfiguration, + getA11yReport, + renderA11yResults, +} from './a11y-result'; import { DialogController } from '../controllers/DialogController'; import { TeleportController } from '../controllers/TeleportController'; @@ -311,13 +313,7 @@ export class Userbar extends HTMLElement { if (!this.shadowRoot || !accessibilityTrigger || !config) return; - // Initialise Axe based on the configurable context (whole page body by default) and options ('empty-heading', 'p-as-heading' and 'heading-order' rules by default) - const results = await axe.run(config.context, config.options); - - const a11yErrorsNumber = results.violations.reduce( - (sum, violation) => sum + violation.nodes.length, - 0, - ); + const { results, a11yErrorsNumber } = await getA11yReport(config); if (results.violations.length) { const a11yErrorBadge = document.createElement('span'); diff --git a/docs/advanced_topics/accessibility_considerations.md b/docs/advanced_topics/accessibility_considerations.md index 3478fc3d2cf3..fdb7fe0075fe 100644 --- a/docs/advanced_topics/accessibility_considerations.md +++ b/docs/advanced_topics/accessibility_considerations.md @@ -142,6 +142,7 @@ By default, the checker includes the following rules to find common accessibilit - `input-button-name`: `` button elements must always have a text label. - `link-name`: `` link elements must always have a text label. - `p-as-heading`: This rule checks for paragraphs that are styled as headings. Paragraphs should not be styled as headings, as they don’t help users who rely on headings to navigate content. +- `alt-text-quality`: A custom rule ensures that image alt texts don't contain anti-patterns like file extensions. To customize how the checker is run (such as what rules to test), you can define a custom subclass of {class}`~wagtail.admin.userbar.AccessibilityItem` and override the attributes to your liking. Then, swap the instance of the default `AccessibilityItem` with an instance of your custom class via the [`construct_wagtail_userbar`](construct_wagtail_userbar) hook. @@ -155,6 +156,10 @@ The following is the reference documentation for the `AccessibilityItem` class: .. autoattribute:: axe_run_only :no-value: .. autoattribute:: axe_rules + .. autoattribute:: axe_custom_rules + :no-value: + .. autoattribute:: axe_custom_checks + :no-value: .. autoattribute:: axe_messages :no-value: @@ -167,12 +172,15 @@ The following is the reference documentation for the `AccessibilityItem` class: .. method:: get_axe_exclude(request) .. method:: get_axe_run_only(request) .. method:: get_axe_rules(request) + .. method:: get_axe_custom_rules(request) + .. method:: get_axe_custom_checks(request) .. method:: get_axe_messages(request) For more advanced customization, you can also override the following methods: .. automethod:: get_axe_context .. automethod:: get_axe_options + .. automethod:: get_axe_spec ``` Here is an example of a custom `AccessibilityItem` subclass that enables more rules: diff --git a/docs/releases/6.2.md b/docs/releases/6.2.md index 7e9bef204dcf..05b2f9d85e42 100644 --- a/docs/releases/6.2.md +++ b/docs/releases/6.2.md @@ -11,6 +11,11 @@ depth: 1 ## What's new +### Alt text accessibility check + +The [built-in accessibility checker](authoring_accessible_content) now enforces a new `alt-text-quality` rule, which tests alt text for the presence of known bad patterns such as file extensions. This rule is enabled by default, but can be disabled if necessary. + +This feature was implemented by Albina Starykova, with support from the Wagtail accessibility team. ### Other features diff --git a/wagtail/admin/tests/test_userbar.py b/wagtail/admin/tests/test_userbar.py index 98528bddef05..83ff4d9feb6f 100644 --- a/wagtail/admin/tests/test_userbar.py +++ b/wagtail/admin/tests/test_userbar.py @@ -341,6 +341,77 @@ def get_axe_rules(self, request): }, ) + def test_custom_rules_and_checks(self): + class CustomRulesAndChecksAccessibilityItem(AccessibilityItem): + # Override via class attribute + axe_custom_checks = [ + { + "id": "check-image-alt-text", + "options": {"pattern": "\\.[a-z]{1,4}$"}, + }, + ] + + # Add via method + def get_axe_custom_rules(self, request): + return super().get_axe_custom_rules(request) + [ + { + "id": "link-text-quality", + "impact": "serious", + "selector": "a", + "tags": ["best-practice"], + "any": ["check-link-text"], + "enabled": True, + } + ] + + def get_axe_custom_checks(self, request): + return super().get_axe_custom_checks(request) + [ + { + "id": "check-link-text", + "options": {"pattern": "learn more$"}, + } + ] + + with hooks.register_temporarily( + "construct_wagtail_userbar", + self.get_hook(CustomRulesAndChecksAccessibilityItem), + ): + self.maxDiff = None + config = self.get_config() + self.assertEqual( + config["spec"], + { + "rules": [ + { + "id": "alt-text-quality", + "impact": "serious", + "selector": "img[alt]", + "tags": ["best-practice"], + "any": ["check-image-alt-text"], + "enabled": True, + }, + { + "id": "link-text-quality", + "impact": "serious", + "selector": "a", + "tags": ["best-practice"], + "any": ["check-link-text"], + "enabled": True, + }, + ], + "checks": [ + { + "id": "check-image-alt-text", + "options": {"pattern": "\\.[a-z]{1,4}$"}, + }, + { + "id": "check-link-text", + "options": {"pattern": "learn more$"}, + }, + ], + }, + ) + class TestUserbarInPageServe(WagtailTestUtils, TestCase): def setUp(self): diff --git a/wagtail/admin/userbar.py b/wagtail/admin/userbar.py index e1f64ea24c8d..0274ed4cee53 100644 --- a/wagtail/admin/userbar.py +++ b/wagtail/admin/userbar.py @@ -63,6 +63,31 @@ class AccessibilityItem(BaseItem): #: For more details, see `Axe documentation `__. axe_rules = {} + #: A list to add custom Axe rules or override their properties, + #: alongside with ``axe_custom_checks``. Includes Wagtail’s custom rules. + #: For more details, see `Axe documentation `_. + axe_custom_rules = [ + { + "id": "alt-text-quality", + "impact": "serious", + "selector": "img[alt]", + "tags": ["best-practice"], + "any": ["check-image-alt-text"], + # If omitted, defaults to True and overrides configs in `axe_run_only`. + "enabled": True, + }, + ] + + #: A list to add custom Axe checks or override their properties. + #: Should be used in conjunction with ``axe_custom_rules``. + #: For more details, see `Axe documentation `_. + axe_custom_checks = [ + { + "id": "check-image-alt-text", + "options": {"pattern": "\\.(avif|gif|jpg|jpeg|png|svg|webp)$"}, + }, + ] + #: A dictionary that maps axe-core rule IDs to custom translatable strings #: to use as the error messages. If an enabled rule does not exist in this #: dictionary, Axe's error message for the rule will be used as fallback. @@ -87,6 +112,9 @@ class AccessibilityItem(BaseItem): "Link text is empty. Use meaningful text for screen reader users." ), "p-as-heading": _("Misusing paragraphs as headings. Use proper heading tags."), + "alt-text-quality": _( + "Image alt text has inappropriate pattern. Use meaningful text." + ), } def get_axe_include(self, request): @@ -105,6 +133,14 @@ def get_axe_rules(self, request): """Returns a dictionary that maps axe-core rule IDs to a dictionary of rule options.""" return self.axe_rules + def get_axe_custom_rules(self, request): + """List of rule objects per axe.run API.""" + return self.axe_custom_rules + + def get_axe_custom_checks(self, request): + """List of check objects per axe.run API, without evaluate function.""" + return self.axe_custom_checks + def get_axe_messages(self, request): """Returns a dictionary that maps axe-core rule IDs to custom translatable strings.""" return self.axe_messages @@ -139,11 +175,19 @@ def get_axe_options(self, request): options.pop("runOnly") return options + def get_axe_spec(self, request): + """Returns spec for Axe, including custom rules and custom checks""" + return { + "rules": self.get_axe_custom_rules(request), + "checks": self.get_axe_custom_checks(request), + } + def get_axe_configuration(self, request): return { "context": self.get_axe_context(request), "options": self.get_axe_options(request), "messages": self.get_axe_messages(request), + "spec": self.get_axe_spec(request), } def get_context_data(self, request):