From 59132b1983cc9b651cef47583d5f73b8c000e885 Mon Sep 17 00:00:00 2001 From: Albina Starykova Date: Tue, 25 Jun 2024 11:30:51 +0300 Subject: [PATCH] Modify regex, clean code, add tests --- client/src/includes/a11y-result.test.ts | 20 +--- client/src/includes/a11y-result.ts | 18 ++-- wagtail/admin/tests/test_userbar.py | 125 ++++++++++++++++++++++++ wagtail/admin/userbar.py | 23 ++--- 4 files changed, 140 insertions(+), 46 deletions(-) diff --git a/client/src/includes/a11y-result.test.ts b/client/src/includes/a11y-result.test.ts index d4bb72df3842..fee973217769 100644 --- a/client/src/includes/a11y-result.test.ts +++ b/client/src/includes/a11y-result.test.ts @@ -2,7 +2,6 @@ import axe, { AxeResults, Spec } from 'axe-core'; import { sortAxeViolations, WagtailAxeConfiguration, - customChecks, addCustomChecks, checkImageAltText, getA11yReport, @@ -63,14 +62,6 @@ describe('sortAxeViolations', () => { }); }); -describe('customChecks', () => { - it('should have function values for each custom check', () => { - Object.values(customChecks).forEach((value) => { - expect(typeof value).toBe('function'); - }); - }); -}); - describe('addCustomChecks', () => { it('should integrate custom checks into the Axe spec', () => { const spec: Spec = { @@ -102,16 +93,13 @@ describe('addCustomChecks', () => { // Options for checkImageAltText function const options = { - pattern: - '\\.(apng|avif|gif|jpg|jpeg|jfif|pjp|png|svg|tif|webp)|(http:\\/\\/|https:\\/\\/|www\\.)', + 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} - ${'Bad alt text.TIFF more text'} | ${false} - ${'https://Bad.alt.text'} | ${false} ${''} | ${true} `('checkImageAltText', ({ text, result }) => { const resultText = result ? 'should not be flagged' : 'should be flagged'; @@ -127,12 +115,6 @@ describe('checkImageAltText edge cases', () => { const image = document.createElement('img'); expect(checkImageAltText(image, options)).toBe(true); }); - - test('should return null if no pattern is provided', () => { - const image = document.createElement('img'); - image.setAttribute('alt', 'Good alt text with words like GIFted and moTIF'); - expect(checkImageAltText(image, {})).toBeUndefined(); - }); }); jest.mock('axe-core', () => ({ diff --git a/client/src/includes/a11y-result.ts b/client/src/includes/a11y-result.ts index a507494cfc68..b24fdccd8885 100644 --- a/client/src/includes/a11y-result.ts +++ b/client/src/includes/a11y-result.ts @@ -78,9 +78,10 @@ export const getAxeConfiguration = ( * 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) => { - if (!options.pattern) return undefined; - +export const checkImageAltText = ( + node: HTMLImageElement, + options: { pattern: string }, +) => { const altTextAntipatterns = new RegExp(options.pattern, 'i'); const altText = node.getAttribute('alt') || ''; @@ -129,15 +130,8 @@ interface A11yReport { export const getA11yReport = async ( config: WagtailAxeConfiguration, ): Promise => { - let spec = config.spec; - // Apply custom configuration for Axe. Custom 'check-image-alt-text' is enabled by default - if (spec) { - if (spec.checks) { - spec = addCustomChecks(spec); - } - axe.configure(spec); - } - // Initialise Axe based on the context (whole page body by default) and options ('button-name', empty-heading', 'empty-table-header', 'frame-title', 'heading-order', 'input-button-name', 'link-name', 'p-as-heading', and a custom 'alt-text-quality' rules by default) + 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, diff --git a/wagtail/admin/tests/test_userbar.py b/wagtail/admin/tests/test_userbar.py index 98528bddef05..56a730836b84 100644 --- a/wagtail/admin/tests/test_userbar.py +++ b/wagtail/admin/tests/test_userbar.py @@ -341,6 +341,131 @@ def get_axe_rules(self, request): }, ) + def test_custom_rules_and_checks(self): + class CustomRulesAndChecksAccessibilityItem(AccessibilityItem): + # Override via class attribute + axe_custom_rules = [ + { + "id": "alt-text-quality", + "impact": "serious", + "selector": "img[alt]", + "tags": ["best-practice"], + "any": ["check-image-alt-text"], + "enabled": True, + }, + ] + axe_custom_checks = [ + { + "id": "check-image-alt-text", + "options": {"pattern": "\\.(avif|gif|jpg|jpeg|png|svg|webp)$"}, + }, + ] + + # Override via method + def get_axe_custom_rules(self, request): + return [ + { + "id": "link-text-quality", + "impact": "serious", + "selector": "a", + "tags": ["best-practice"], + "any": ["check-link-text"], + "enabled": True, + } + ] + super().get_axe_custom_rules(request) + + def get_axe_custom_checks(self, request): + return [ + { + "id": "check-link-text", + "options": {"pattern": "learn more$"}, + } + ] + super().get_axe_custom_checks(request) + + with hooks.register_temporarily( + "construct_wagtail_userbar", + self.get_hook(CustomRulesAndChecksAccessibilityItem), + ): + config = self.get_config() + self.assertEqual( + config["spec"], + { + "rules": [ + # Override via class attribute + { + "id": "alt-text-quality", + "impact": "serious", + "selector": "img[alt]", + "tags": ["best-practice"], + "any": ["check-image-alt-text"], + "enabled": True, + }, + # Override via method + { + "id": "link-text-quality", + "impact": "serious", + "selector": "a", + "tags": ["best-practice"], + "any": ["check-link-text"], + "enabled": True, + }, + ], + "checks": [ + # Override via class attribute + { + "id": "check-image-alt-text", + "options": { + "pattern": "\\.(avif|gif|jpg|jpeg|png|svg|webp)$" + }, + }, + # Override via method + { + "id": "check-link-text", + "options": {"pattern": "learn more$"}, + }, + ], + }, + ) + + def test_pattern_if_custom_alt_text_check_enabled(self): + class CustomAltTextAccessibilityItem(AccessibilityItem): + axe_custom_checks = [ + { + "id": "check-image-alt-text", + "options": {"pattern": "\\.(avif|gif|jpg|jpeg|png|svg|webp)$"}, + }, + ] + + def get_axe_custom_checks(self, request): + return self.axe_custom_checks + super().get_axe_custom_checks(request) + + with hooks.register_temporarily( + "construct_wagtail_userbar", + self.get_hook(CustomAltTextAccessibilityItem), + ): + config = self.get_config() + custom_checks = config["spec"]["checks"] + self.assertIsInstance(custom_checks, list) + + # Check if the custom check is present + check_image_alt_text = next( + ( + check + for check in custom_checks + if check["id"] == "check-image-alt-text" + ), + None, + ) + + if check_image_alt_text: + # Verify pattern only if the check is enabled + self.assertEqual( + check_image_alt_text["options"]["pattern"], + "\\.(avif|gif|jpg|jpeg|png|svg|webp)$", + ) + else: + self.skipTest("check-image-alt-text not enabled in custom checks") + class TestUserbarInPageServe(WagtailTestUtils, TestCase): def setUp(self): diff --git a/wagtail/admin/userbar.py b/wagtail/admin/userbar.py index 9d3656035304..0ce91d3c1ce2 100644 --- a/wagtail/admin/userbar.py +++ b/wagtail/admin/userbar.py @@ -69,7 +69,7 @@ class AccessibilityItem(BaseItem): #: and enabled by default. This rule ensures that alt texts don't contain #: antipatterns like file extensions or URLs. Returns zero false positives. #: Should be used in conjunction with `axe_custom_checks` - #: For more details, see `Axe documentation `__. + #: For more details, see `Axe documentation `_. axe_custom_rules = [ { "id": "alt-text-quality", @@ -77,7 +77,8 @@ class AccessibilityItem(BaseItem): "selector": "img[alt]", "tags": ["best-practice"], "any": ["check-image-alt-text"], - "enabled": True, # If ommited, defaults to True and overrrides configs in `axe_run_only` + # If omitted, defaults to True and overrides configs in `axe_run_only`. + "enabled": True, }, ] @@ -85,13 +86,11 @@ class AccessibilityItem(BaseItem): #: or to override the properties of existing Axe checks. A custom check #: for the quality of the images alt texts is added and enabled by default. #: Should be used in conjunction with `axe_custom_rules` - #: For more details, see `Axe documentation `__. + #: For more details, see `Axe documentation `_. axe_custom_checks = [ { "id": "check-image-alt-text", - "options": { - "pattern": "\.(apng|avif|gif|jpg|jpeg|jfif|pjp|png|svg|tif|webp)|(http:\/\/|https:\/\/|www.)" - }, + "options": {"pattern": "\\.(avif|gif|jpg|jpeg|png|svg|webp)$"}, }, ] @@ -141,26 +140,20 @@ def get_axe_rules(self, request): return self.axe_rules def get_axe_custom_rules(self, request): - """Returns custom rules for Axe""" + """List of rule objects per axe.run API.""" return self.axe_custom_rules def get_axe_custom_checks(self, request): - """Returns custom checks for Axe""" + """List of check objects per axe.run API, without evaluate function.""" return self.axe_custom_checks def get_axe_spec(self, request): """Returns spec for Axe, including custom rules and custom checks""" - spec = { + return { "rules": self.get_axe_custom_rules(request), "checks": self.get_axe_custom_checks(request), } - # If both the lists of custom rules and custom checks are empty, - # no custom configuration should be applied for Axe - if not spec["rules"] and not spec["checks"]: - spec = "" - return spec - def get_axe_messages(self, request): """Returns a dictionary that maps axe-core rule IDs to custom translatable strings.""" return self.axe_messages