feat(axe-core-4.6): update to axe-core 4.6.3, promote link-in-text-bl…

…ock and meta-viewport (#6334) #### Details This PR updates `axe-core` to its latest version, 4.6.2, from 4.4.1. Notable code changes beyond the simple version bump include: * `link-in-text-block` failures have been promoted from "needs review" to "automated checks" (removed from needs review with explicit code change, added to automated checks implicitly due to new axe-core tagging) * `meta-viewport` failures have been promoted from "unreported" to "automated checks" (implicitly due to new axe-core tagging) * WCAG mappings for a few rules have changed (we pull these automatically from axe-core's tags, so there is no corresponding code change) * Per discussion with PM, `link-in-text-block` incomplete results are staying unreported (not being promoted to "needs review") * `frame-title-unique` and `no-autoplay-audio` are explicitly disabled. They were promoted in axe-core from experimental/best-practice to "wcag-correspondant, but needs-review-only", but we want to omit them from our needs review checks pending further investigation ##### Motivation See 1960423 ##### Context * We are considering a separate PR to pull in a temporary version of the `aria-required-children` message improvements we've suggested upstream as [axe-core#3842](dequelabs/axe-core#3842) (which would hopefully be removed later once we update to axe-core 4.7). Leaving that for a separate PR. * While testing this locally, I noticed an issue where the target page console would show an error during scans suggesting axe-core scans were being run multiple times concurrently. I also observed this is `main` without this PR - I don't think it's a regression and will be following up separately. #### Pull request checklist  - [x] Addresses an existing issue: 1960423 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS --------- Co-authored-by: JGibson2019 <jacqueline.gibson@microsoft.com> Co-authored-by: SB <shanisebarona@gmail.com>
microsoft · Jan 30, 2023 · ed75fda · ed75fda
1 parent c8a46ad
commit ed75fda
Show file tree

Hide file tree

Showing 23 changed files with 135 additions and 147 deletions.
diff --git a/package.json b/package.json
@@ -183,7 +183,7 @@
         "ajv": "^8.12.0",
         "android-device-list": "^1.2.9",
         "appium-adb": "^9.10.23",
-        "axe-core": "4.4.1",
+        "axe-core": "4.6.3",
         "classnames": "^2.3.2",
         "electron": "22.0.0",
         "electron-log": "^4.4.8",

diff --git a/packages/report/package.json b/packages/report/package.json
@@ -1,6 +1,6 @@
 {
     "name": "accessibility-insights-report",
-    "version": "4.4.1",
+    "version": "4.6.3",
     "description": "Accessibility Insights Report",
     "license": "MIT",
     "files": [
@@ -19,7 +19,7 @@
     },
     "dependencies": {
         "@fluentui/react": "^8.96.1",
-        "axe-core": "4.4.1",
+        "axe-core": "4.6.3",
         "classnames": "^2.3.2",
         "lodash": "^4.17.21",
         "luxon": "^3.2.1",

diff --git a/packages/ui/package.json b/packages/ui/package.json
@@ -18,7 +18,7 @@
     },
     "dependencies": {
         "@fluentui/react": "^8.96.1",
-        "axe-core": "4.4.1",
+        "axe-core": "4.6.3",
         "classnames": "^2.3.2",
         "lodash": "^4.17.21",
         "luxon": "^3.2.1",

diff --git a/src/common/components/cards/rich-resolution-content.tsx b/src/common/components/cards/rich-resolution-content.tsx
@@ -267,28 +267,6 @@ export const RichResolutionContent = NamedFC<RichResolutionContentProps>(
                     </div>
                 );
             }
-            case 'web/link-in-text-block': {
-                return (
-                    <ul className={styles.multiLineTextNoBullet}>
-                        <li>
-                            Manually verify that the link text EITHER has a contrast ratio of at
-                            least 3:1 compared to surrounding text OR has a distinct visual style
-                            (such as underlined, bolded, or italicized).
-                        </li>
-                        <li>
-                            To measure contrast, use{' '}
-                            <LinkComponent href="https://go.microsoft.com/fwlink/?linkid=2075365">
-                                Accessibility Insights for Windows
-                            </LinkComponent>{' '}
-                            (or the{' '}
-                            <LinkComponent href="https://developer.paciellogroup.com/resources/contrastanalyser/">
-                                Colour Contrast Analyser
-                            </LinkComponent>{' '}
-                            if you're testing on a Mac).
-                        </li>
-                    </ul>
-                );
-            }
             case 'web/th-has-data-cells': {
                 return (
                     <div>

diff --git a/src/injected/analyzers/filter-results.ts b/src/injected/analyzers/filter-results.ts
@@ -12,9 +12,7 @@ export const filterNeedsReviewResults = (results: ScanResults): ScanResults => {
             v.id !== 'color-contrast' &&
             v.id !== 'th-has-data-cells',
     );
-    results.incomplete = results.incomplete.filter(
-        i => i.id !== 'link-in-text-block' && i.id !== 'label-content-name-mismatch',
-    );
+    results.incomplete = results.incomplete.filter(i => i.id !== 'label-content-name-mismatch');
 
     return results;
 };
diff --git a/src/scanner/get-rule-inclusions.ts b/src/scanner/get-rule-inclusions.ts
@@ -11,37 +11,45 @@ export type RuleIncluded =
     | { status: 'excluded'; reason: string };
 
 export const explicitRuleOverrides: DictionaryStringTo<RuleIncluded> = {
+    'aria-allowed-role': {
+        status: 'included',
+        reason: 'best practice rule that was investigated with no known false positives, implemented as an automated check.',
+    },
     'audio-caption': {
         status: 'included',
         reason: 'for parity with video-caption, which axe-core includes by default',
     },
+    'duplicate-id': {
+        status: 'excluded',
+        reason: 'Based on feedback from users we tested to check the user impact of duplicate ID failures on static elements. We found no user impact on the experience with any of the ATs including Narrator, JAWS, NVDA, Dragon, and Windows Speech Recognition. After further discussions with the Tooling Advisory Board, we decided to make this a best practice rule. See #4102.',
+    },
     'empty-table-header': {
         status: 'excluded',
         reason: "only reports needs-review results, but we haven't implemented needs-review content for it yet",
     },
-    'form-field-multiple-labels': {
+    'frame-tested': {
+        status: 'included-always',
+        reason: 'Tests for unresponsive frames, enables iframe skipped warning',
+    },
+    'frame-title-unique': {
         status: 'excluded',
         reason: "only reports needs-review results, but we haven't implemented needs-review content for it yet",
     },
-    'duplicate-id': {
+    'form-field-multiple-labels': {
         status: 'excluded',
-        reason: 'Based on feedback from users we tested to check the user impact of duplicate ID failures on static elements. We found no user impact on the experience with any of the ATs including Narrator, JAWS, NVDA, Dragon, and Windows Speech Recognition. After further discussions with the Tooling Advisory Board, we decided to make this a best practice rule. See #4102.',
+        reason: "only reports needs-review results, but we haven't implemented needs-review content for it yet",
     },
-    'scrollable-region-focusable': {
+    'no-autoplay-audio': {
         status: 'excluded',
-        reason: 'only reports to needs-review results due to potential false-positives',
-    },
-    'aria-allowed-role': {
-        status: 'included',
-        reason: 'best practice rule that was investigated with no known false positives, implemented as an automated check.',
+        reason: "only reports needs-review results, but we haven't implemented needs-review content for it yet",
     },
     'presentation-role-conflict': {
         status: 'included',
         reason: 'best practice rule that was investigated with no known false positives, implemented as an automated check.',
     },
-    'frame-tested': {
-        status: 'included-always',
-        reason: 'Tests for unresponsive frames, enables iframe skipped warning',
+    'scrollable-region-focusable': {
+        status: 'excluded',
+        reason: 'only reports to needs-review results due to potential false-positives',
     },
 };
 
@@ -50,7 +58,6 @@ export const needsReviewRules = [
     'aria-input-field-name',
     'color-contrast',
     'th-has-data-cells',
-    'link-in-text-block',
     'scrollable-region-focusable',
     'label-content-name-mismatch',
     'p-as-heading',

diff --git a/src/tests/common/false-positive-violations.ts b/src/tests/common/false-positive-violations.ts
@@ -0,0 +1,35 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+import { Result } from 'axe-core';
+import { AxeInfo } from '../../common/axe-info';
+
+const axeInfo = AxeInfo.Default;
+
+// this is a method to remove violations tied to rules with known false-positives and was introduced
+// Jan 25 2023 to remove aria-required-children failures introduced by axe-core 4.6.1
+// we should keep this in until Deque introduces the fix for the issues tracked here
+// https://github.com/dequelabs/axe-core/issues/3850
+// the axe-core bug causes a failure for the FluentUI v8 DetailsList component
+// The FluentUI tracking issue can be found here:
+// https://github.com/microsoft/fluentui/issues/26330
+export function falsePositiveRemoval(violations: Result[]): Result[] {
+    // Re-evaluate if the false positive is still present in future axe-core versions
+    if (axeInfo.version !== '4.6.3') {
+        throw new Error('Axe Core version has changed. Please check if this is still needed');
+    }
+    let newViolations = violations.map(function (violation) {
+        if (violation.id === 'aria-required-children') {
+            const newNodes = violation.nodes.filter(
+                node =>
+                    !(
+                        node.html.includes('ms-DetailsHeader') ||
+                        node.html.includes('ms-DetailsRow')
+                    ),
+            );
+            violation.nodes = newNodes;
+        }
+        return violation;
+    });
+    newViolations = newViolations.filter(violation => violation.nodes.length > 0);
+    return newViolations;
+}
diff --git a/src/tests/electron/common/scan-for-accessibility-issues.ts b/src/tests/electron/common/scan-for-accessibility-issues.ts
@@ -4,6 +4,7 @@ import * as path from 'path';
 import { Result } from 'axe-core';
 import { Page } from 'playwright';
 import { getNeedsReviewRulesConfig } from 'scanner/get-rule-inclusions';
+import { falsePositiveRemoval } from 'tests/common/false-positive-violations';
 import { AppController } from 'tests/electron/common/view-controllers/app-controller';
 
 import { screenshotOnError as screenshot } from '../../end-to-end/common/screenshot-on-error';
@@ -44,6 +45,7 @@ async function runAxeScan(client: Page, selector?: string): Promise<Result[]> {
         },
         { selector, axeRunOptions },
     );
+    axeResults.violations = falsePositiveRemoval(axeResults.violations);
     return axeResults.violations;
 }
 

diff --git a/src/tests/end-to-end/common/page-controllers/details-view-page.ts b/src/tests/end-to-end/common/page-controllers/details-view-page.ts
@@ -230,6 +230,13 @@ export class DetailsViewPage extends Page {
         await detailsViewPage.clickSelector(tabStopsSelectors.instanceRemoveButton);
         await detailsViewPage.waitForSelectorToDisappear(tabStopsSelectors.failedInstancesSection);
     }
+
+    public async scrollToInstanceTable(): Promise<void> {
+        await this.underlyingPage
+            .locator(detailsViewSelectors.instanceTableTextContent)
+            .first()
+            .scrollIntoViewIfNeeded();
+    }
 }
 
 export function detailsViewRelativeUrl(targetTabId: number): string {

diff --git a/src/tests/end-to-end/common/pretty-print-axe-violations.ts b/src/tests/end-to-end/common/pretty-print-axe-violations.ts
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 import { AxeResults } from 'axe-core';
+import { normalizeOfficeFabricClassName } from 'tests/common/element-snapshot-formatter';
 
 export interface PrintableAxeResultNode {
     selector: string[];
@@ -12,12 +13,23 @@ export interface PrintableAxeResult {
     nodes: PrintableAxeResultNode[];
 }
 
+// This normalizes any selectors that are based on dynamically-generated FluentUI IDs
+// to ensure that snapshots are derived consistently between test runs/test machines
+export function normalizeSelector(selector: string): string {
+    let output = selector;
+    if (/^[#.][a-zA-Z0-9-]+$/.test(output)) {
+        const identifier = output.slice(1);
+        output = `${output[0]}${normalizeOfficeFabricClassName(identifier)}`;
+    }
+    return output;
+}
+
 export function prettyPrintAxeViolations(axeResults: AxeResults): PrintableAxeResult[] {
     const violations = axeResults.violations;
     const printableViolations: PrintableAxeResult[] = violations.map(result => {
         const nodeResults: PrintableAxeResultNode[] = result.nodes.map(node => {
             return {
-                selector: node.target,
+                selector: node.target.map(normalizeSelector),
                 failureSummary: node.failureSummary,
             } as PrintableAxeResultNode;
         });

diff --git a/src/tests/end-to-end/common/scan-for-accessibility-issues.ts b/src/tests/end-to-end/common/scan-for-accessibility-issues.ts
@@ -2,8 +2,9 @@
 // Licensed under the MIT License.
 import * as path from 'path';
 import { AxeResults, ElementContext } from 'axe-core';
-
 import { getNeedsReviewRulesConfig } from 'scanner/get-rule-inclusions';
+
+import { falsePositiveRemoval } from '../../common/false-positive-violations';
 import { Page } from './page-controllers/page';
 import { prettyPrintAxeViolations, PrintableAxeResult } from './pretty-print-axe-violations';
 
@@ -17,16 +18,14 @@ export async function scanForAccessibilityIssues(
     await injectAxeIfUndefined(page);
     const axeResults = (await page.evaluate(
         options => {
-            return axe.run(
-                { include: [options.selector] } as ElementContext,
-                {
-                    runOnly: { type: 'tag', values: ['wcag2a', 'wcag21a', 'wcag2aa', 'wcag21aa'] },
-                    rules: options.rules,
-                } as ElementContext,
-            );
+            return axe.run({ include: [options.selector] } as ElementContext, {
+                runOnly: { type: 'tag', values: ['wcag2a', 'wcag21a', 'wcag2aa', 'wcag21aa'] },
+                rules: options.rules,
+            });
         },
         { selector, rules: getNeedsReviewRulesConfig() },
     )) as AxeResults;
+    axeResults.violations = falsePositiveRemoval(axeResults.violations);
     return prettyPrintAxeViolations(axeResults);
 }
 

diff --git a/src/tests/end-to-end/tests/details-view/__snapshots__/headings.test.ts.snap b/src/tests/end-to-end/tests/details-view/__snapshots__/headings.test.ts.snap
@@ -9,7 +9,7 @@ exports[`Details View -> Assessment -> Headings Requirement page should pass acc
         "failureSummary": "Fix any of the following:
   Text inside the element is not included in the accessible name",
         "selector": [
-          "#header66-visualizationButton",
+          "#header000-visualizationButton",
         ],
       },
     ],
@@ -26,7 +26,7 @@ exports[`Details View -> Assessment -> Headings Requirement page should pass acc
         "failureSummary": "Fix any of the following:
   Text inside the element is not included in the accessible name",
         "selector": [
-          "#header66-visualizationButton",
+          "#header000-visualizationButton",
         ],
       },
     ],

diff --git a/src/tests/end-to-end/tests/details-view/__snapshots__/instructions.test.ts.snap b/src/tests/end-to-end/tests/details-view/__snapshots__/instructions.test.ts.snap
@@ -1,35 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Details View -> Quick Assess -> Instructions Requirement page should pass accessibility validation with highContrastMode=false 1`] = `
-[
-  {
-    "id": "label-content-name-mismatch",
-    "nodes": [
-      {
-        "failureSummary": "Fix any of the following:
-  Text inside the element is not included in the accessible name",
-        "selector": [
-          "#header61-visualizationButton",
-        ],
-      },
-    ],
-  },
-]
-`;
+exports[`Details View -> Quick Assess -> Instructions Requirement page should pass accessibility validation with highContrastMode=false 1`] = `[]`;
 
-exports[`Details View -> Quick Assess -> Instructions Requirement page should pass accessibility validation with highContrastMode=true 1`] = `
-[
-  {
-    "id": "label-content-name-mismatch",
-    "nodes": [
-      {
-        "failureSummary": "Fix any of the following:
-  Text inside the element is not included in the accessible name",
-        "selector": [
-          "#header61-visualizationButton",
-        ],
-      },
-    ],
-  },
-]
-`;
+exports[`Details View -> Quick Assess -> Instructions Requirement page should pass accessibility validation with highContrastMode=true 1`] = `[]`;
diff --git a/src/tests/end-to-end/tests/details-view/__snapshots__/settings-panel.test.ts.snap b/src/tests/end-to-end/tests/details-view/__snapshots__/settings-panel.test.ts.snap
@@ -8,15 +8,17 @@ exports[`Details View -> Settings Panel should pass accessibility validation wit
     "id": "link-in-text-block",
     "nodes": [
       {
-        "failureSummary": "Fix all of the following:
-  Links need to be distinguished from surrounding text in some way other than by color",
+        "failureSummary": "Fix any of the following:
+  The link has insufficient color contrast of 1.07:1 with the surrounding text. (Minimum contrast is 3:1, link text: #ffff00, surrounding text: #ffffff)
+  The link has no styling (such as underline) to distinguish it from the surrounding text",
         "selector": [
           "p:nth-child(1) > .root-294.insights-link[target="_blank"]",
         ],
       },
       {
-        "failureSummary": "Fix all of the following:
-  Links need to be distinguished from surrounding text in some way other than by color",
+        "failureSummary": "Fix any of the following:
+  The link has insufficient color contrast of 1.07:1 with the surrounding text. (Minimum contrast is 3:1, link text: #ffff00, surrounding text: #ffffff)
+  The link has no styling (such as underline) to distinguish it from the surrounding text",
         "selector": [
           ".toggle-description--ieTgg > .root-294.insights-link[target="_blank"]",
         ],

diff --git a/src/tests/end-to-end/tests/details-view/headings.test.ts b/src/tests/end-to-end/tests/details-view/headings.test.ts
@@ -42,11 +42,16 @@ describe('Details View -> Assessment -> Headings', () => {
                 await browser.setHighContrastMode(highContrastMode);
                 await headingsPage.waitForHighContrastMode(highContrastMode);
 
+                // Workaround for axe-core #3883 to ensure the label-content-name-mismatch false
+                // positive below is detected consistently
+                await headingsPage.scrollToInstanceTable();
+
                 const results = await scanForAccessibilityIssues(
                     headingsPage,
                     detailsViewSelectors.mainContent,
                 );
-                // this results object has a failure for label-content-name-misatch
+
+                // this results object has a failure for label-content-name-mismatch
                 // where the "show all visualizations" label does not match the content (a checkbox)
                 // this is a false positive because the checkbox is symbolic, so this criteria
                 // does not apply

diff --git a/src/tests/end-to-end/tests/popup/__snapshots__/launchpad.test.ts.snap b/src/tests/end-to-end/tests/popup/__snapshots__/launchpad.test.ts.snap
@@ -247,7 +247,7 @@ exports[`Popup -> Launch Pad content should match snapshot when quick assess fea
                 Navigate to axe-core npm page
               </div>
             </div>
-             4.4.1
+             4.6.3
           </div>
         </div>
       </div>
@@ -545,7 +545,7 @@ exports[`Popup -> Launch Pad content should match snapshot when quick assess fea
                 Navigate to axe-core npm page
               </div>
             </div>
-             4.4.1
+             4.6.3
           </div>
         </div>
       </div>