fix: use axe run options from accessibility-insights-web (disables em…

…pty-table-header rule) (#2402) #### Details Today, Service decides which axe-core rules to use via a `RuleExclusion` mechanism that starts from axe-core's default ruleset an disables rules we don't use in web. This is inconsistent with how accessibility-insights-web defines its ruleset (via a combination of [axe-core's rule tags and explicit overrides](https://github.com/microsoft/accessibility-insights-web/blob/[email protected]/src/scanner/get-rule-inclusions.ts)). The difference in mechanisms led to a mistake during the axe-core 4.6.3 update where the rulesets became inconsistent between web and service. To ensure consistency between web and service, this PR replaces `rule-exclusion.ts` with a new mechanism based on the recently-added `@accessibility-insights/axe-config` package in the web repo. That package builds an axe config blob based on web's `get-rule-inclusions.ts` mechanism. This update bundles a copy of the current output of that package into `axe-configuration.ts` to form the basis of service's axe configuration, and removes `rule-exclusion.ts` entirely as obsolete. Besides keeping the rules in sync, this also means adds a new axe-core config option that was previously overridden by web but not service (`"pingWaitTime": 1000` instead of the default 500ms). I decided to keep the new value from the web config for consistency with web and because both values are still very comfortably within the margin of error set by the page level timeouts used in `page-timeout-config.ts`, even assuming pages with multiple levels of iframe nesting. ##### Motivation * Make it more reliable to keep web and service rules in sync. * Disables the unintentionally-enabled `empty-table-header` best-practice rule ##### Context * Builds on @sfoslund 's work in microsoft/accessibility-insights-web#6395 * Partially implements (but doesn't complete) #571 * I considered doing a fuller implementation that would involve creating a publishing pipeline for the axe-config package, but I didn't want to tie up the fix for the rule inconsistency in service vs web with that process. This is still an incremental improvement that moves us closer to that end state without the NPM package layer. * Incorporates changes from microsoft/accessibility-insights-web#6448 and microsoft/accessibility-insights-web#6449 ##### Testing Besides the PR checklist, I validated that a local build of the scan package against test site https://www.catalog.update.microsoft.com/Home.aspx produced a report consistent with [email protected]'s results (no violations), where previously the service reported a slightly distinct set of errors (one violation of the `empty-table-header` rule, which was accidentally omitted from `rule-exclusions.ts` during the 4.6.3 update). #### Pull request checklist  - [x] Addresses an existing issue: Partially addresses #571 - [x] Added relevant unit test for your changes. (`yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] Ran precheckin (`yarn precheckin`) - [ ] Validated in an Azure resource group
microsoft · Feb 22, 2023 · 8c00499 · 8c00499
1 parent e69cb95
commit 8c00499
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 57 deletions.
diff --git a/packages/scanner-global-library/src/axe-scanner/axe-puppeteer-factory.spec.ts b/packages/scanner-global-library/src/axe-scanner/axe-puppeteer-factory.spec.ts
@@ -8,8 +8,8 @@ import { AxePuppeteer } from '@axe-core/puppeteer';
 import * as Puppeteer from 'puppeteer';
 import { IMock, Mock, MockBehavior, Times } from 'typemoq';
 import { AxePuppeteerFactory } from './axe-puppeteer-factory';
-import { RuleExclusion } from './rule-exclusion';
 import { AxeConfiguration } from './axe-configuration';
+import { AxeRunOptions } from './axe-run-options';
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
@@ -18,26 +18,30 @@ describe('AxePuppeteerFactory', () => {
     let page: IMock<Puppeteer.Page>;
     let fsMock: IMock<typeof fs>;
     let axeConfiguration: AxeConfiguration;
+    let axeRunOptions: AxeRunOptions;
 
     beforeEach(() => {
         page = Mock.ofType<Puppeteer.Page>();
         fsMock = Mock.ofInstance(fs, MockBehavior.Strict);
         axeConfiguration = { allowedOrigins: ['test origin'] };
-        testSubject = new AxePuppeteerFactory(axeConfiguration, new RuleExclusion(), fsMock.object);
+        axeRunOptions = { runOnly: ['test-rule'] };
+        testSubject = new AxePuppeteerFactory(axeConfiguration, axeRunOptions, fsMock.object);
     });
 
     it('create axe puppeteer instance', async () => {
         const axePuppeteer = await testSubject.createAxePuppeteer(page.object);
         expect(axePuppeteer).toBeDefined();
         expect(axePuppeteer).toBeInstanceOf(AxePuppeteer);
         expect((axePuppeteer as any).config).toStrictEqual(axeConfiguration);
+        expect((axePuppeteer as any).axeOptions).toStrictEqual(axeRunOptions);
     });
 
     it('create axe puppeteer instance, sourcePath is empty', async () => {
         const axePuppeteer = await testSubject.createAxePuppeteer(page.object, '');
         expect(axePuppeteer).toBeDefined();
         expect(axePuppeteer).toBeInstanceOf(AxePuppeteer);
         expect((axePuppeteer as any).config).toStrictEqual(axeConfiguration);
+        expect((axePuppeteer as any).axeOptions).toStrictEqual(axeRunOptions);
     });
 
     it('create axe puppeteer instance, sourcePath is not empty', async () => {
@@ -52,12 +56,14 @@ describe('AxePuppeteerFactory', () => {
         expect(axePuppeteer).toBeDefined();
         expect(axePuppeteer).toBeInstanceOf(AxePuppeteer);
         expect((axePuppeteer as any).config).toStrictEqual(axeConfiguration);
+        expect((axePuppeteer as any).axeOptions).toStrictEqual(axeRunOptions);
     });
 
     it('create axe puppeteer instance, legacyMode is true', async () => {
         const axePuppeteer = await testSubject.createAxePuppeteer(page.object, '', true);
         expect(axePuppeteer).toBeDefined();
         expect(axePuppeteer).toBeInstanceOf(AxePuppeteer);
         expect((axePuppeteer as any).config).toStrictEqual(axeConfiguration);
+        expect((axePuppeteer as any).axeOptions).toStrictEqual(axeRunOptions);
     });
 });
diff --git a/packages/scanner-global-library/src/axe-scanner/axe-puppeteer-factory.ts b/packages/scanner-global-library/src/axe-scanner/axe-puppeteer-factory.ts
@@ -7,24 +7,21 @@ import { inject, injectable } from 'inversify';
 import { isEmpty } from 'lodash';
 import * as Puppeteer from 'puppeteer';
 import { iocTypes } from '../ioc-types';
-import { RuleExclusion } from './rule-exclusion';
 import { AxeConfiguration } from './axe-configuration';
+import { AxeRunOptions } from './axe-run-options';
 
 @injectable()
 export class AxePuppeteerFactory {
     constructor(
         @inject(iocTypes.AxeConfiguration) private readonly axeConfiguration: AxeConfiguration,
-        private readonly ruleExclusion: RuleExclusion = new RuleExclusion(),
+        @inject(iocTypes.AxeRunOptions) private readonly axeRunOptions: AxeRunOptions,
         private readonly fileSystemObj: typeof fs = fs,
     ) {}
 
     public async createAxePuppeteer(page: Puppeteer.Page, contentSourcePath?: string, legacyMode: boolean = false): Promise<AxePuppeteer> {
         const axeSource = await this.getAxeSource(contentSourcePath);
 
-        return new AxePuppeteer(page, axeSource)
-            .configure(this.axeConfiguration)
-            .disableRules(this.ruleExclusion.accessibilityRuleExclusionList)
-            .setLegacyMode(legacyMode);
+        return new AxePuppeteer(page, axeSource).configure(this.axeConfiguration).options(this.axeRunOptions).setLegacyMode(legacyMode);
     }
 
     private async getAxeSource(contentSourcePath?: string): Promise<string> {

diff --git a/packages/scanner-global-library/src/axe-scanner/axe-run-options.ts b/packages/scanner-global-library/src/axe-scanner/axe-run-options.ts
@@ -0,0 +1,77 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import type { RunOptions } from 'axe-core';
+
+export type AxeRunOptions = RunOptions;
+
+// This is copied verbatim from the built version of the @accessibility-insights/axe-config
+// package in microsoft/accessibility-insights-web. It must be updated with each axe-core update
+// and also to pick up any out-of-band rule customizations.
+//
+// In the future, it would be nice to publish that package and pick it up via NPM instead of
+// copying it here. See https://github.com/microsoft/accessibility-insights-service/issues/571
+export const webAxeRunOptions: AxeRunOptions = {
+    runOnly: {
+        type: 'rule',
+        values: [
+            'area-alt',
+            'aria-allowed-attr',
+            'aria-allowed-role',
+            'aria-command-name',
+            'aria-hidden-body',
+            'aria-hidden-focus',
+            'aria-input-field-name',
+            'aria-meter-name',
+            'aria-progressbar-name',
+            'aria-required-attr',
+            'aria-required-children',
+            'aria-required-parent',
+            'aria-roledescription',
+            'aria-roles',
+            'aria-toggle-field-name',
+            'aria-tooltip-name',
+            'aria-valid-attr-value',
+            'aria-valid-attr',
+            'audio-caption',
+            'autocomplete-valid',
+            'avoid-inline-spacing',
+            'blink',
+            'button-name',
+            'color-contrast',
+            'definition-list',
+            'dlitem',
+            'document-title',
+            'duplicate-id-active',
+            'duplicate-id-aria',
+            'frame-focusable-content',
+            'frame-title',
+            'html-has-lang',
+            'html-lang-valid',
+            'html-xml-lang-mismatch',
+            'image-alt',
+            'input-button-name',
+            'input-image-alt',
+            'label',
+            'link-in-text-block',
+            'link-name',
+            'list',
+            'listitem',
+            'marquee',
+            'meta-refresh',
+            'meta-viewport',
+            'nested-interactive',
+            'object-alt',
+            'presentation-role-conflict',
+            'role-img-alt',
+            'select-name',
+            'server-side-image-map',
+            'svg-img-alt',
+            'td-headers-attr',
+            'th-has-data-cells',
+            'valid-lang',
+            'video-caption',
+        ],
+    },
+    pingWaitTime: 1000,
+};
diff --git a/packages/scanner-global-library/src/axe-scanner/rule-exclusion.ts b/packages/scanner-global-library/src/axe-scanner/rule-exclusion.ts
diff --git a/packages/scanner-global-library/src/ioc-types.ts b/packages/scanner-global-library/src/ioc-types.ts
@@ -3,5 +3,6 @@
 
 export const iocTypes = {
     AxeConfiguration: 'AxeConfiguration',
+    AxeRunOptions: 'AxeRunOptions',
     AzureAuthClientCredentialProvider: 'azureAuthClientCredentialProvider',
 };
diff --git a/packages/scanner-global-library/src/setup-scanner-container.spec.ts b/packages/scanner-global-library/src/setup-scanner-container.spec.ts
@@ -7,6 +7,7 @@ import { Container } from 'inversify';
 import { AxePuppeteerFactory } from './axe-scanner/axe-puppeteer-factory';
 import { setupCloudScannerContainer, setupLocalScannerContainer } from './setup-scanner-container';
 import { cloudAxeConfiguration, localAxeConfiguration } from './axe-scanner/axe-configuration';
+import { webAxeRunOptions } from './axe-scanner/axe-run-options';
 import { iocTypes } from './ioc-types';
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
@@ -27,6 +28,10 @@ describe(setupCloudScannerContainer, () => {
         expect(container.get(iocTypes.AxeConfiguration)).toBe(cloudAxeConfiguration);
     });
 
+    it('should use webAxeRunOptions', () => {
+        expect(container.get(iocTypes.AxeRunOptions)).toBe(webAxeRunOptions);
+    });
+
     function verifySingletonResolution(key: any): void {
         expect(container.get(key)).toBeDefined();
         expect(container.get(key)).toBe(container.get(key));
@@ -44,4 +49,8 @@ describe(setupLocalScannerContainer, () => {
     it('should use localAxeConfiguration', () => {
         expect(container.get(iocTypes.AxeConfiguration)).toBe(localAxeConfiguration);
     });
+
+    it('should use webAxeRunOptions', () => {
+        expect(container.get(iocTypes.AxeRunOptions)).toBe(webAxeRunOptions);
+    });
 });
diff --git a/packages/scanner-global-library/src/setup-scanner-container.ts b/packages/scanner-global-library/src/setup-scanner-container.ts
@@ -3,18 +3,21 @@
 
 import * as inversify from 'inversify';
 import { cloudAxeConfiguration, localAxeConfiguration } from './axe-scanner/axe-configuration';
+import { webAxeRunOptions } from './axe-scanner/axe-run-options';
 import { AxePuppeteerFactory } from './axe-scanner/axe-puppeteer-factory';
 import { iocTypes } from './ioc-types';
 
 export function setupCloudScannerContainer(container: inversify.Container): inversify.Container {
     container.bind(iocTypes.AxeConfiguration).toConstantValue(cloudAxeConfiguration);
+    container.bind(iocTypes.AxeRunOptions).toConstantValue(webAxeRunOptions);
     container.bind(AxePuppeteerFactory).toSelf().inSingletonScope();
 
     return container;
 }
 
 export function setupLocalScannerContainer(container: inversify.Container): inversify.Container {
     container.bind(iocTypes.AxeConfiguration).toConstantValue(localAxeConfiguration);
+    container.bind(iocTypes.AxeRunOptions).toConstantValue(webAxeRunOptions);
 
     return container;
 }