Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixrule(list_markup_review) Reduce Needs Review false positives for a list #1861

Merged
merged 16 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading