fix(aria-allowed-role): landmark roles banner on header and contentin…

…fo on footer to only report on top-level rule (#3142)

Co-authored-by: Wilco Fiers <wilco.fiers@deque.com>
Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>

axe.d.ts

@@ -264,13 +264,16 @@ declare namespace axe {
     results: PartialRuleResult[];
     environmentData?: EnvironmentData;
   }
-  type PartialResults = Array<PartialResult | null>
+  type PartialResults = Array<PartialResult | null>;
   interface FrameContext {
     frameSelector: CrossTreeSelector;
     frameContext: ContextObject;
   }
   interface Utils {
-    getFrameContexts: (context?: ElementContext, options?: RunOptions) => FrameContext[];
+    getFrameContexts: (
+      context?: ElementContext,
+      options?: RunOptions
+    ) => FrameContext[];
     shadowSelect: (selector: CrossTreeSelector) => Element | null;
   }
   interface EnvironmentData {

doc/frame-messenger.md

@@ -110,7 +110,7 @@ The `message` passed to responder may be an `Error`. If axe-core passes an `Erro
 
 When axe-core tests frames, it first sends a ping to that frame, to check that the frame has a compatible version of axe-core in it that can respond to the message. If it gets no response, that frame will be skipped in the test. Axe-core does this to avoid a situation where it waits the full frame timeout, just to find out the frame didn't have axe-core in it in the first place.
 
-In situations where communication between frames can be slow, it may be necessary to increase the ping timeout. This can be done with the `pingWaitTime` option. By default, this is 500ms. This can be configured in the following way: 
+In situations where communication between frames can be slow, it may be necessary to increase the ping timeout. This can be done with the `pingWaitTime` option. By default, this is 500ms. This can be configured in the following way:
 
 ```js
 const results = await axe.run(context, { pingWaitTime: 1000 }));

doc/run-partial.md

@@ -7,9 +7,7 @@ To use these methods, call `axe.runPartial()` in the top window, and in all nest
 This results in code that looks something like the following. The `context` and `options` arguments used are the same as would be passed to `axe.run`. See [API.md](api.md) for details.
 
 ```js
-const partialResults = await Promise.all(
-  runPartialRecursive(context, options)
-)
+const partialResults = await Promise.all(runPartialRecursive(context, options));
 const axeResults = await axe.finishRun(partialResults, options);
 ```
 
@@ -25,10 +23,10 @@ function runPartialRecursive(context, options = {}, win = window) {
   // Find all frames in context, and determine what context object to use in that frame
   const frameContexts = axe.utils.getFrameContexts(context, options);
   // Run the current context, in the current window.
-  const promiseResults = [ axe.runPartial(context, options) ];
+  const promiseResults = [axe.runPartial(context, options)];
 
   // Loop over all frames in context
-  frameContexts.forEach(({ frameSelector, frameContext }) =>  {
+  frameContexts.forEach(({ frameSelector, frameContext }) => {
     // Find the window of the frame
     const frame = axe.utils.shadowSelect(frameSelector);
     const frameWin = frame.contentWindow;
@@ -37,7 +35,7 @@ function runPartialRecursive(context, options = {}, win = window) {
     promiseResults.push(...frameResults);
   });
   return promiseResults;
-};
+}
 ```
 
 **important**: The order in which these methods are called matters for performance. Internally, axe-core constructs a flattened tree when `axe.utils.getFrameContexts` is called. This is fairly slow, and so should not happen more than once per frame. When `axe.runPartial` is called, that tree will be used if it still exists. Since this tree can get out of sync with the actual DOM, it is important to call `axe.runPartial` immediately after `axe.utils.getFrameContexts`.
@@ -55,7 +53,7 @@ The `axe.finishRun` method does two things: It calls the `after` methods of chec
 //   - frame_1a
 // - frame_2
 // The partial results are passed in the following order:
-axe.finishRun([ top, frame_1, frame_1a, frame_2 ])
+axe.finishRun([top, frame_1, frame_1a, frame_2]);
 ```
 
 If for some reason `axe.runPartial` fails to run, the integration API **must** include `null` in the data in place of the results object, so that axe-core knows to skip over it. If a frame fails to run, results from any descending frames **must be omitted**. To illustrate this, consider the following:
@@ -68,9 +66,7 @@ If for some reason `axe.runPartial` fails to run, the integration API **must** i
 // - frame_2
 
 // If axe.runPartial throws an error, the results must be passed to finishRun like this:
-axe.finishRun([
-  top, null, /* nothing for frame 1a, */ frame_2
-])
+axe.finishRun([top, null, /* nothing for frame 1a, */ frame_2]);
 ```
 
 **important**: Since `axe.finishRun` may have access to cross-origin information, it should only be called in an environment that is known not to have third-party scripts. When using a browser driver, this can for example by done in a blank page.

lib/checks/aria/deprecatedrole-evaluate.js

@@ -15,7 +15,7 @@ export default function deprecatedroleEvaluate(node, options, virtualNode) {
   if (!roleDefinition?.deprecated) {
     return false;
   }
-  
+
   this.data(role);
   return true;
 }

lib/commons/aria/get-element-unallowed-roles.js

@@ -2,12 +2,7 @@ import isValidRole from './is-valid-role';
 import getImplicitRole from './implicit-role';
 import getRoleType from './get-role-type';
 import isAriaRoleAllowedOnElement from './is-aria-role-allowed-on-element';
-import {
-  tokenList,
-  isHtmlElement,
-  matchesSelector,
-  getNodeFromTree
-} from '../../core/utils';
+import { tokenList, isHtmlElement, getNodeFromTree } from '../../core/utils';
 import AbstractVirtuaNode from '../../core/base/virtual-node/abstract-virtual-node';
 
 // dpub roles which are subclassing roles that are implicit on some native
@@ -22,6 +17,11 @@ const dpubRoles = [
   'doc-noteref'
 ];
 
+const landmarkRoles = {
+  header: 'banner',
+  footer: 'contentinfo'
+};
+
 /**
  * Returns all roles applicable to element in a list
  *
@@ -33,7 +33,6 @@ const dpubRoles = [
 
 function getRoleSegments(vNode) {
   let roles = [];
-
   if (!vNode) {
     return roles;
   }
@@ -44,9 +43,7 @@ function getRoleSegments(vNode) {
   }
 
   // filter invalid roles
-  roles = roles.filter(role => isValidRole(role));
-
-  return roles;
+  return roles.filter(role => isValidRole(role));
 }
 
 /**
@@ -59,50 +56,32 @@ function getRoleSegments(vNode) {
 function getElementUnallowedRoles(node, allowImplicit = true) {
   const vNode =
     node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node);
-  const { nodeName } = vNode.props;
-
   // by pass custom elements
   if (!isHtmlElement(vNode)) {
     return [];
   }
+  // allow landmark roles to use their implicit role inside another landmark
+  // @see https://github.com/dequelabs/axe-core/pull/3142
+  const { nodeName } = vNode.props;
+  const implicitRole = getImplicitRole(vNode) || landmarkRoles[nodeName];
 
   const roleSegments = getRoleSegments(vNode);
-  const implicitRole = getImplicitRole(vNode);
-
-  // stores all roles that are not allowed for a specific element most often an element only has one explicit role
-  const unallowedRoles = roleSegments.filter(role => {
-    // if role and implicit role are same, when allowImplicit: true
-    // ignore as it is a redundant role
-    if (allowImplicit && role === implicitRole) {
-      return false;
-    }
-
-    // if role is a dpub role make sure it's used on an element with a valid
-    // implicit role fallback
-    if (allowImplicit && dpubRoles.includes(role)) {
-      const roleType = getRoleType(role);
-      if (implicitRole !== roleType) {
-        return true;
-      }
-    }
-
-    // Edge case:
-    // setting implicit role row on tr element is allowed when child of table[role='grid']
-    if (
-      !allowImplicit &&
-      !(
-        role === 'row' &&
-        nodeName === 'tr' &&
-        matchesSelector(vNode, 'table[role="grid"] > tr')
-      )
-    ) {
-      return true;
-    }
-    // check if role is allowed on element
-    return !isAriaRoleAllowedOnElement(vNode, role);
+  return roleSegments.filter(role => {
+    return !roleIsAllowed(role, vNode, allowImplicit, implicitRole);
   });
+}
 
-  return unallowedRoles;
+function roleIsAllowed(role, vNode, allowImplicit, implicitRole) {
+  if (allowImplicit && role === implicitRole) {
+    return true;
+  }
+  // if role is a dpub role make sure it's used on an element with a valid
+  // implicit role fallback
+  if (dpubRoles.includes(role) && getRoleType(role) !== implicitRole) {
+    return false;
+  }
+  // check if role is allowed on element
+  return isAriaRoleAllowedOnElement(vNode, role);
 }
 
 export default getElementUnallowedRoles;

lib/commons/aria/is-aria-role-allowed-on-element.js

@@ -15,17 +15,17 @@ function isAriaRoleAllowedOnElement(node, role) {
     node instanceof AbstractVirtuaNode ? node : getNodeFromTree(node);
   const implicitRole = getImplicitRole(vNode);
 
-  // always allow the explicit role to match the implicit role
-  if (role === implicitRole) {
-    return true;
-  }
-
   const spec = getElementSpec(vNode);
 
   if (Array.isArray(spec.allowedRoles)) {
     return spec.allowedRoles.includes(role);
   }
 
+  // By default, ARIA in HTML does not allow implicit roles to be the same as explicit ones
+  // aria-allowed-roles has an `allowedImplicit` option to bypass this.
+  if (role === implicitRole) {
+    return false;
+  }
   return !!spec.allowedRoles;
 }
 

lib/core/base/audit.js

@@ -592,11 +592,10 @@ class Audit {
       } else if (['tag', 'tags', undefined].includes(only.type)) {
         only.type = 'tag';
 
-        const unmatchedTags = only.values.filter(tag => (
-          !tags.includes(tag) && 
-          !/wcag2[1-3]a{1,3}/.test(tag)
-        ));
-        if (unmatchedTags.length !== 0)  {
+        const unmatchedTags = only.values.filter(
+          tag => !tags.includes(tag) && !/wcag2[1-3]a{1,3}/.test(tag)
+        );
+        if (unmatchedTags.length !== 0) {
           axe.log('Could not find tags `' + unmatchedTags.join('`, `') + '`');
         }
       } else {
@@ -622,7 +621,7 @@ class Audit {
       application: this.application
     };
     if (typeof branding === 'string') {
-      this.application = branding
+      this.application = branding;
     }
     if (
       branding &&

lib/core/utils/send-command-to-frame.js

@@ -2,16 +2,15 @@ import getSelector from './get-selector';
 import respondable from './respondable';
 import log from '../log';
 
-
 /**
  * Sends a command to an instance of axe in the specified frame
  * @param  {Element}  node       The frame element to send the message to
  * @param  {Object}   parameters Parameters to pass to the frame
  * @param  {Function} callback   Function to call when results from the frame has returned
  */
- export default function sendCommandToFrame(node, parameters, resolve, reject) {
+export default function sendCommandToFrame(node, parameters, resolve, reject) {
   const win = node.contentWindow;
-  const pingWaitTime = parameters.options?.pingWaitTime ?? 500
+  const pingWaitTime = parameters.options?.pingWaitTime ?? 500;
   if (!win) {
     log('Frame does not have a content window', node);
     resolve(null);

lib/rules/aria-roles.json

@@ -10,10 +10,10 @@
   "all": [],
   "any": [],
   "none": [
-    "fallbackrole", 
-    "invalidrole", 
-    "abstractrole", 
-    "unsupportedrole", 
+    "fallbackrole",
+    "invalidrole",
+    "abstractrole",
+    "unsupportedrole",
     "deprecatedrole"
   ]
 }

test/checks/aria/aria-allowed-role.js

@@ -30,11 +30,11 @@ describe('aria-allowed-role', function() {
     var options = {
       allowImplicit: false
     };
-    var actual = axe.testUtils
+    var outcome = axe.testUtils
       .getCheckEvaluate('aria-allowed-role')
       .call(checkContext, null, options, vNode);
-    var expected = false;
-    assert.equal(actual, expected);
+
+    assert.isFalse(outcome);
     assert.deepEqual(checkContext._data, ['row']);
   });
 

test/checks/aria/deprecatedrole.js

@@ -4,10 +4,10 @@ describe('deprecatedrole', function() {
   var checkContext = axe.testUtils.MockCheckContext();
   var checkSetup = axe.testUtils.checkSetup;
   var checkEvaluate = axe.testUtils.getCheckEvaluate('deprecatedrole');
-  afterEach(function () {
+  afterEach(function() {
     checkContext.reset();
     axe.reset();
-  })
+  });
 
   it('returns true if applied to a deprecated role', function() {
     axe.configure({
@@ -36,7 +36,9 @@ describe('deprecatedrole', function() {
         }
       }
     });
-    var params = checkSetup('<div id="target" role="doc-fizzbuzz">Contents</div>');
+    var params = checkSetup(
+      '<div id="target" role="doc-fizzbuzz">Contents</div>'
+    );
     assert.isTrue(checkEvaluate.apply(checkContext, params));
     assert.deepEqual(checkContext._data, 'doc-fizzbuzz');
   });
@@ -57,8 +59,8 @@ describe('deprecatedrole', function() {
     assert.isNull(checkContext._data);
   });
 
-  describe('with fallback roles', function () {
-    it('returns true if the deprecated role is the first valid role', function () {
+  describe('with fallback roles', function() {
+    it('returns true if the deprecated role is the first valid role', function() {
       axe.configure({
         standards: {
           ariaRoles: {
@@ -69,12 +71,14 @@ describe('deprecatedrole', function() {
           }
         }
       });
-      var params = checkSetup('<div id="target" role="foo widget melon button">Contents</div>');
+      var params = checkSetup(
+        '<div id="target" role="foo widget melon button">Contents</div>'
+      );
       assert.isTrue(checkEvaluate.apply(checkContext, params));
       assert.deepEqual(checkContext._data, 'melon');
-    })
+    });
 
-    it('returns false if the deprecated role is not the first valid role', function () {
+    it('returns false if the deprecated role is not the first valid role', function() {
       axe.configure({
         standards: {
           ariaRoles: {
@@ -85,7 +89,9 @@ describe('deprecatedrole', function() {
           }
         }
       });
-      var params = checkSetup('<div id="target" role="button melon widget">Contents</div>');
+      var params = checkSetup(
+        '<div id="target" role="button melon widget">Contents</div>'
+      );
       assert.isFalse(checkEvaluate.apply(checkContext, params));
       assert.isNull(checkContext._data);
     });