Skip to content

Commit

Permalink
Merge pull request #1861 from IBMa/dev-1626
Browse files Browse the repository at this point in the history
fixrule(`list_markup_review`) Reduce Needs Review false positives for a list
  • Loading branch information
ErickRenteria authored Mar 7, 2024
2 parents 0eabdbe + ccb7a15 commit 565f70c
Show file tree
Hide file tree
Showing 19 changed files with 943 additions and 1,335 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,44 @@ export class RPTUtil {
return retVal;
}

/**
* WAI-ARIA’s role attribute may have a list of values, but only the first valid and supported WAI-ARIA role is used
* https://www.w3.org/TR/wai-aria-implementation/#mapping_role_table
* This function is responsible for retrieving the resoled role for an element.
* @parm {HTMLElement} ele - element for which to find role.
*
* @return string - resolved role for the element:
* explicit role: resoled from the list of values
* implicit role: if not explicitely specified, or none of the specified role values is allowed for the element
* null: if none of the specified role values is allowed for the element, neither implicit role exists
*
* @memberOf RPTUtil
*/
public static getResolvedRole(elem: Element) : string {
if (!elem) return null;
let role = getCache(elem, "RPTUTIL_ELEMENT_RESOLVED_ROLE", null);
if (role === null) {
const roles = RPTUtil.getUserDefinedRoles(elem);
let tagProperty = RPTUtil.getElementAriaProperty(elem);
let allowedRoles = RPTUtil.getAllowedAriaRoles(elem, tagProperty);
if (roles && roles.length > 0 && allowedRoles && allowedRoles.length > 0) {
for (let i=0; i < roles.length; i++) {
if (allowedRoles.includes(roles[i])) {
role = roles[i];
break;
}
}
}

if (role === null) {
const implicitRole = RPTUtil.getImplicitRole(elem);
role = implicitRole && implicitRole.length > 0 ? implicitRole[0] : undefined;
}
setCache(elem, "RPTUTIL_ELEMENT_RESOLVED_ROLE", role);
}
return role !== undefined ? role : null;
}

/**
* This function is responsible for retrieving user defined element's roles from dom.
* @parm {HTMLElement} ele - element for which to find role.
Expand Down Expand Up @@ -1571,7 +1609,56 @@ export class RPTUtil {
walkNode = DOMWalker.parentNode(walkNode);
}
return walkNode;
}
}

/**
* return the ancestor of the given element and roles.
*
* @parm {element} element - The element to start the node walk on to find parent node
* @parm {array} roles - the role names to search for
* @parm {bool} considerImplicitRoles - true or false based on if implicit roles setting should be considered.
*
* @return {node} walkNode - A parent node of the element passed in, which has the provided role
*
* @memberOf RPTUtil
*/
public static getAncestorWithRoles(element, roleNames) {
if (!element || !roleNames || !roleNames.length || roleNames.length === 0) return null;
let walkNode = element;
while (walkNode !== null) {
let role = RPTUtil.getResolvedRole(walkNode);
if (role !== null && roleNames.includes(role))
return walkNode;
walkNode = DOMWalker.parentNode(walkNode);
}
return null;
}

/**
* return the roles with given role type.
*
* @parm {element} element - The element to start the node walk on to find parent node
* @parm {array} roleTyples - role types, such as 'widget', 'structure' etc.
*
* @return {array} roles - A parent node of the element passed in, which has the provided role
*
* @memberOf RPTUtil
*/
public static getRolesWithTypes(element, types: string[]) {
if (!element || !types || !types.length || types.length ===0) return null;

let roles = getCache(element.ownerDocument, "roles_with_given_types", null);
if (!roles || roles.length === 0) {
roles = [];
Object.entries(ARIADefinitions.designPatterns).forEach(([key, value]) => {
if (types.includes(value.roleType))
roles.push(key);
});
setCache(element.ownerDocument, "roles_with_given_types", roles);
}
return roles;
}

/**
* return the ancestor with the given style properties.
*
Expand Down
46 changes: 32 additions & 14 deletions accessibility-checker-engine/src/v4/rules/list_markup_review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import { Rule, RuleResult, RuleFail, RuleContext, RulePotential, RuleManual, RulePass, RuleContextHierarchy } from "../api/IRule";
import { eRulePolicy, eToolkitLevel } from "../api/IRule";
import { NodeWalker, RPTUtil } from "../../v2/checker/accessibility/util/legacy";
import { VisUtil } from "../../v2/dom/VisUtil";

export let list_markup_review: Rule = {
id: "list_markup_review",
Expand All @@ -25,16 +26,16 @@ export let list_markup_review: Rule = {
},
help: {
"en-US": {
"Pass_0": "list_markup_review.html",
"Potential_1": "list_markup_review.html",
"pass": "list_markup_review.html",
"potential_list": "list_markup_review.html",
"group": "list_markup_review.html"
}
},
messages: {
"en-US": {
"Pass_0": "Rule Passed",
"Potential_1": "Verify whether this is a list that should use HTML list elements",
"group": "Use proper HTML list elements to create lists"
"pass": "Proper HTML elements are used to create a list",
"potential_list": "Verify this is a list and if so, modify to use proper HTML elements for the list",
"group": "Proper HTML elements should be used to create a list"
}
},
rulesets: [{
Expand All @@ -46,14 +47,37 @@ export let list_markup_review: Rule = {
act: [],
run: (context: RuleContext, options?: {}, contextHierarchies?: RuleContextHierarchy): RuleResult | RuleResult[] => {
const ruleContext = context["dom"].node as Element;

// Extract the nodeName of the context node
let nodeName = ruleContext.nodeName.toLowerCase();

//skip the check if the element is hidden or disabled
if (RPTUtil.isNodeDisabled(ruleContext) || VisUtil.hiddenByDefaultElements.includes(nodeName))
return null;

// Don't trigger if we're not in the body or if we're in a script
if (RPTUtil.getAncestor(ruleContext, ["body"]) === null)
return null;

// ignore script, label and their child elements
if (RPTUtil.getAncestor(ruleContext, ["script", 'label']) !== null)
return null;

// ignore all widgets and their children, and certain structure roles
let roles = RPTUtil.getRolesWithTypes(ruleContext, ["widget"]);
// add some structure roles
RPTUtil.concatUniqueArrayItemList(["caption", "code", "columnheader", "figure", "list", "listitem", "math", "meter", "columnheader", "rowheader"], roles);
if (RPTUtil.getAncestorWithRoles(ruleContext, roles) !== null)
return null;

let passed = true;
let walkNode = ruleContext.firstChild as Node;
while (passed && walkNode) {
// Comply to the Check Hidden Content setting will be done by default as this rule triggers on each element
// and for each element it only checks that single elements text nodes and nothing else. So all inner elements will be
// covered on their own. Currently for this rule by default Check Hidden Content will work, as we are doing
// a node walk only on siblings so it would not get text nodes from other siblings at all.
// In the case in the future something chnges, just need to add && !RPTUtil.shouldNodeBeSkippedHidden(walkNode) to the below
// In the case in the future something changes, just need to add && !RPTUtil.shouldNodeBeSkippedHidden(walkNode) to the below
// if.
if (walkNode.nodeName == "#text") {
let txtVal = walkNode.nodeValue;
Expand All @@ -80,14 +104,8 @@ export let list_markup_review: Rule = {
walkNode = walkNode.nextSibling;
}

if (!passed) {
// Don't trigger if we're not in the body or if we're in a script
let checkAncestor = RPTUtil.getAncestor(ruleContext, ["body", "script"]);
passed = checkAncestor == null || checkAncestor.nodeName.toLowerCase() != "body";
}

if (passed) return RulePass("Pass_0");
if (!passed) return RulePotential("Potential_1");
if (passed) return null;
if (!passed) return RulePotential("potential_list");

}
}
Loading

0 comments on commit 565f70c

Please sign in to comment.