Merge pull request #2686 from johanrd/day_fix/template-require-context-role

Post-merge-review: Fix template-require-context-role: align aria-hidden scope and report oc with upstream

Author: NullVoxPopuli

lib/rules/template-require-context-role.js

@@ -49,13 +49,20 @@ module.exports = {
 
         if (role && ROLES_REQUIRING_CONTEXT[role]) {
           // Skip check if at root level (no parent elements — context may be external)
-          if (elementStack.length > 1 && !isInsideAriaHidden(elementStack)) {
-            const parentRole = getAccessibleParentRole(elementStack);
+          if (elementStack.length > 1) {
+            const parentContext = getParentContext(elementStack);
+            if (parentContext.ariaHidden) {
+              // aria-hidden on the effective parent (or a transparent wrapper
+              // walked through on the way up) — upstream suppresses the rule.
+              return;
+            }
+            const parentRole = parentContext.role;
             if (parentRole === undefined) {
               // No non-transparent parent found (effectively root) — skip
             } else if (!parentRole || !ROLES_REQUIRING_CONTEXT[role].includes(parentRole)) {
+              const roleAttr = node.attributes?.find((a) => a.name === 'role');
               context.report({
-                node,
+                node: roleAttr || node,
                 messageId: 'missingContext',
                 data: {
                   role,
@@ -82,46 +89,37 @@ function getRoleFromNode(node) {
   return null;
 }
 
-/**
- * Check if any ancestor element in the stack has aria-hidden="true".
- */
-function isInsideAriaHidden(elementStack) {
-  // Check ancestors (all elements except the current one)
-  for (let i = elementStack.length - 2; i >= 0; i--) {
-    const node = elementStack[i];
-    const ariaHidden = node.attributes?.find((a) => a.name === 'aria-hidden');
-    if (ariaHidden?.value?.type === 'GlimmerTextNode' && ariaHidden.value.chars === 'true') {
-      return true;
-    }
-  }
-  return false;
+function hasAriaHiddenTrue(node) {
+  const attr = node.attributes?.find((a) => a.name === 'aria-hidden');
+  return attr?.value?.type === 'GlimmerTextNode' && attr.value.chars === 'true';
 }
 
 /**
- * Get the role of the nearest non-transparent ancestor element.
- * Transparent elements are those with role="presentation"/"none" or named blocks (tag starts with ':').
- * Returns:
- *   - a role string if a non-transparent ancestor with a role is found
- *   - null if a non-transparent ancestor WITHOUT a role is found (breaks context)
- *   - undefined if no non-transparent ancestor exists (root level)
+ * Walk up the ancestor chain through transparent wrappers (named-block slots,
+ * `<template>`, role="presentation"/"none") checking `aria-hidden` at each
+ * layer. Returns { ariaHidden, role } where:
+ *   - `ariaHidden` is true if aria-hidden="true" was seen on any traversed
+ *     element (including transparent wrappers) up to and including the first
+ *     non-transparent parent — matches upstream's semantics.
+ *   - `role` is the role of the first non-transparent parent: a role string,
+ *     null (element with no role), or undefined (no non-transparent parent).
  */
-function getAccessibleParentRole(elementStack) {
+function getParentContext(elementStack) {
   for (let i = elementStack.length - 2; i >= 0; i--) {
     const node = elementStack[i];
-
-    // Named blocks (e.g. <:content>) and <template> wrapper are transparent
+    if (hasAriaHiddenTrue(node)) {
+      return { ariaHidden: true, role: undefined };
+    }
+    // Named blocks (`<:content>`) and the `<template>` wrapper are transparent
     if (node.tag && (node.tag.startsWith(':') || node.tag === 'template')) {
       continue;
     }
-
     const role = getRoleFromNode(node);
-
-    // Presentation/none roles are transparent in the accessibility tree
+    // presentation/none roles are transparent in the accessibility tree
     if (role === 'presentation' || role === 'none') {
       continue;
     }
-
-    return role; // could be null (element with no role) or a role string
+    return { ariaHidden: false, role };
   }
-  return undefined; // no non-transparent ancestor found
+  return { ariaHidden: false, role: undefined };
 }

tests/lib/rules/template-require-context-role.js

@@ -89,7 +89,7 @@ ruleTester.run('template-require-context-role', rule, {
         {
           message:
             'Role "listitem" must be contained in an element with one of these roles: group, list',
-          type: 'GlimmerElementNode',
+          type: 'GlimmerAttrNode',
         },
       ],
     },
@@ -103,7 +103,7 @@ ruleTester.run('template-require-context-role', rule, {
       errors: [
         {
           message: 'Role "tab" must be contained in an element with one of these roles: tablist',
-          type: 'GlimmerElementNode',
+          type: 'GlimmerAttrNode',
         },
       ],
     },
@@ -118,7 +118,7 @@ ruleTester.run('template-require-context-role', rule, {
         {
           message:
             'Role "menuitem" must be contained in an element with one of these roles: group, menu, menubar',
-          type: 'GlimmerElementNode',
+          type: 'GlimmerAttrNode',
         },
       ],
     },
@@ -275,6 +275,18 @@ ruleTester.run('template-require-context-role', rule, {
         },
       ],
     },
+    {
+      // aria-hidden on a non-immediate ancestor must NOT suppress the rule
+      // (upstream only honors aria-hidden on the immediate parent)
+      code: '<template><div aria-hidden="true"><div><div role="listitem">Item</div></div></div></template>',
+      output: null,
+      errors: [
+        {
+          message:
+            'Role "listitem" must be contained in an element with one of these roles: group, list',
+        },
+      ],
+    },
   ],
 });
 
@@ -482,5 +494,16 @@ hbsRuleTester.run('template-require-context-role', rule, {
         },
       ],
     },
+    {
+      // aria-hidden on a non-immediate ancestor must NOT suppress the rule
+      code: '<div aria-hidden="true"><div><div role="listitem">Item</div></div></div>',
+      output: null,
+      errors: [
+        {
+          message:
+            'Role "listitem" must be contained in an element with one of these roles: group, list',
+        },
+      ],
+    },
   ],
 });