From c51b995d25035dace0f6cba6582ec521b655a52d Mon Sep 17 00:00:00 2001
From: Shunguo <shunguoy@us.ibm.com>
Date: Fri, 25 Oct 2024 17:56:11 -0500
Subject: [PATCH] update target spacing rule #1974

---
 .../src/v2/aria/ARIADefinitions.ts            |  4 +-
 .../src/v2/dom/DOMMapper.ts                   | 27 +++++---
 .../src/v4/rules/target_spacing_sufficient.ts | 26 ++++----
 .../src/v4/util/CommonUtil.ts                 | 42 ++++++------
 .../src/v4/util/VisUtil.ts                    | 64 +++++++++++++++++--
 .../element_clip.html                         | 34 ++++++++++
 6 files changed, 148 insertions(+), 49 deletions(-)
 create mode 100644 accessibility-checker-engine/test/v2/checker/accessibility/rules/target_spacing_sufficient_ruleunit/element_clip.html

diff --git a/accessibility-checker-engine/src/v2/aria/ARIADefinitions.ts b/accessibility-checker-engine/src/v2/aria/ARIADefinitions.ts
index c24e7e9d9..d9f4e9ac1 100644
--- a/accessibility-checker-engine/src/v2/aria/ARIADefinitions.ts
+++ b/accessibility-checker-engine/src/v2/aria/ARIADefinitions.ts
@@ -1814,7 +1814,7 @@ export class ARIADefinitions {
         },
         "button": {
             implicitRole: ["button"],
-            validRoles: ["checkbox", "combobox", "gridcell", "link", "menuitem", "menuitemcheckbox", "menuitemradio", "option", "radio", "slider", "switch", "tab", "treeitem"],
+            validRoles: ["checkbox", "combobox", "gridcell", "link", "menuitem", "menuitemcheckbox", "menuitemradio", "option", "radio","separator", "slider", "switch", "tab", "treeitem"],
             globalAriaAttributesValid: true
         },
         "canvas": {
@@ -1927,7 +1927,7 @@ export class ARIADefinitions {
             globalAriaAttributesValid: false
         },
         "hgroup": {
-            implicitRole: ["generic"],
+            implicitRole: ["group"],
             validRoles: ["any"],
             globalAriaAttributesValid: true
         },
diff --git a/accessibility-checker-engine/src/v2/dom/DOMMapper.ts b/accessibility-checker-engine/src/v2/dom/DOMMapper.ts
index bfb8bf41f..4ffa6264a 100644
--- a/accessibility-checker-engine/src/v2/dom/DOMMapper.ts
+++ b/accessibility-checker-engine/src/v2/dom/DOMMapper.ts
@@ -16,6 +16,7 @@
 
 import { CommonMapper } from "../common/CommonMapper";
 import { Bounds } from "../api/IMapper";
+import { CacheUtil } from "../../v4/util/CacheUtil";
 
 export class DOMMapper extends CommonMapper {
     getRole(node: Node) : string {
@@ -42,7 +43,10 @@ export class DOMMapper extends CommonMapper {
      * @returns 
      */
     getBounds(node: Node) : Bounds {
-        if (node.nodeType === 1 /*Node.ELEMENT_NODE*/) {
+        if (node.nodeType !== 1 /*Node.ELEMENT_NODE*/) return null;
+
+        const bunds = CacheUtil.getCache(node as Element, "DOMMapper_Bounds", undefined);
+        if (bunds === undefined) {
             let adjustment = 1;
             if (node.ownerDocument && node.ownerDocument.defaultView && node.ownerDocument.defaultView.devicePixelRatio) {
                 adjustment = node.ownerDocument.defaultView.devicePixelRatio;
@@ -53,16 +57,18 @@ export class DOMMapper extends CommonMapper {
             if (bounds) {
                 let scrollX = node && node.ownerDocument && node.ownerDocument.defaultView && node.ownerDocument.defaultView.scrollX || 0;
                 let scrollY = node && node.ownerDocument && node.ownerDocument.defaultView && node.ownerDocument.defaultView.scrollY || 0;
-                return {
+                const ret = {
                     "left": Math.ceil((bounds.left + scrollX) * adjustment),
                     "top": Math.ceil((bounds.top + scrollY) * adjustment),
                     "height": Math.ceil(bounds.height * adjustment),
                     "width": Math.ceil(bounds.width * adjustment)
                 };
+                CacheUtil.setCache(node as Element, "DOMMapper_Bounds", ret);
+                return ret;
             }
+            return null;
         }
-
-        return null;
+        return bunds;
     }
 
     /**
@@ -71,21 +77,26 @@ export class DOMMapper extends CommonMapper {
      * @returns 
      */
     getUnadjustedBounds(node: Node) : Bounds {
-        if (node.nodeType === 1 /*Node.ELEMENT_NODE*/) {
+        if (node.nodeType !== 1 /*Node.ELEMENT_NODE*/) return null;
+
+        const bunds = CacheUtil.getCache(node as Element, "DOMMapper_UnadjustedBounds", undefined);
+        if (bunds === undefined) {
             const bounds = (node as Element).getBoundingClientRect();
             // adjusted for scroll if any
             if (bounds) {
                 let scrollX = node && node.ownerDocument && node.ownerDocument.defaultView && node.ownerDocument.defaultView.scrollX || 0;
                 let scrollY = node && node.ownerDocument && node.ownerDocument.defaultView && node.ownerDocument.defaultView.scrollY || 0;
-                return {
+                const ret = {
                     "left": Math.ceil(bounds.left + scrollX),
                     "top": Math.ceil(bounds.top + scrollY),
                     "height": Math.ceil(bounds.height),
                     "width": Math.ceil(bounds.width)
                 };
+                CacheUtil.setCache(node as Element, "DOMMapper_UnadjustedBounds", ret);
+                return ret;
             }
+            return null;
         }
-
-        return null;
+        return bunds;
     }
 }
diff --git a/accessibility-checker-engine/src/v4/rules/target_spacing_sufficient.ts b/accessibility-checker-engine/src/v4/rules/target_spacing_sufficient.ts
index 3977aff41..c132532aa 100644
--- a/accessibility-checker-engine/src/v4/rules/target_spacing_sufficient.ts
+++ b/accessibility-checker-engine/src/v4/rules/target_spacing_sufficient.ts
@@ -64,13 +64,9 @@
         run: (context: RuleContext, options?: {}, contextHierarchies?: RuleContextHierarchy): RuleResult | RuleResult[] => {
             const ruleContext = context["dom"].node as HTMLElement;
             const nodeName = ruleContext.nodeName.toLocaleLowerCase(); 
-            //ignore certain elements
-            if (CommonUtil.getAncestor(ruleContext, ["svg", "pre", "code", "script", "meta", 'head']) !== null 
-                || nodeName === "body" || nodeName === "html" )
-                return null;
             
             // ignore hidden, non-target, or inline element without text in the same line
-            if (!VisUtil.isNodeVisible(ruleContext) || !CommonUtil.isTarget(ruleContext))
+            if (!CommonUtil.isTarget(ruleContext))
                 return null;
 
             // check inline element: without text in the same line
@@ -103,23 +99,23 @@
             if (!zindex || isNaN(Number(zindex)))
                 zindex = "0";
             
-            var elems = doc.querySelectorAll('body *:not(script)');
+            //select all elements except itself and descendants
+            var elems = doc.querySelectorAll('body *:not(script):not(style)');
             if (!elems || elems.length === 0)
                 return;
             
             const mapper : DOMMapper = new DOMMapper();
             const bounds = mapper.getUnadjustedBounds(ruleContext); //context["dom"].bounds;    
-            if (!bounds || bounds['height'] === 0 || bounds['width'] === 0 ) 
+            if (!bounds) 
                 return null;
-                
-                    
+                        
             let before = true;
             let minX = 24;
             let minY = 24;
             let adjacentX = null;
             let adjacentY = null;
             let checked = []; //contains a list of elements that have been checked so their descendants don't need to be checked again
-            for (let i=0; i < elems.length; i++) { 
+            for (let i=0; i < elems.length; i++) { //console.log("target="+nodeName +", target id="+ ruleContext.getAttribute("id") +" elem="+elems[i].nodeName +", id="+elems[i].getAttribute("id"));
                 const elem = elems[i] as HTMLElement;
                 /**
                  *  the nodes returned from querySelectorAll is in document order
@@ -130,12 +126,14 @@
                     //the next node in elems will be after the target node (ruleContext). 
                     before = false;
                     continue;
-                }    
-                if (!VisUtil.isNodeVisible(elem) || !CommonUtil.isTarget(elem) || elem.contains(ruleContext) 
-                   || checked.some(item => item.contains(elem))) continue;
+                }
+                // ignore ascendants of the element, not a target, or itself or its ascendant already checked   
+                if (elem.contains(ruleContext)  || !CommonUtil.isTarget(elem) 
+                   || checked.some(item => item.contains(elem))) 
+                    continue;
 
                 const bnds = mapper.getUnadjustedBounds(elem);
-                if (bnds.height === 0 || bnds.width === 0) continue;
+                if (!bnds) continue;
                 
                 var zStyle = getComputedStyle(elem); 
                 let z_index = '0';
diff --git a/accessibility-checker-engine/src/v4/util/CommonUtil.ts b/accessibility-checker-engine/src/v4/util/CommonUtil.ts
index 2c648373c..52ce6d4ab 100644
--- a/accessibility-checker-engine/src/v4/util/CommonUtil.ts
+++ b/accessibility-checker-engine/src/v4/util/CommonUtil.ts
@@ -458,29 +458,33 @@ export class CommonUtil {
          * 
          */
     public static isTarget(element) {
-        if (!element) return false;
-
-        if (element.hasAttribute("tabindex") || CommonUtil.isTabbable(element)) return true;
-
-        const roles = AriaUtil.getRoles(element, true);
-        if (!roles && roles.length === 0)
-            return false;
-
-        let tagProperty = AriaUtil.getElementAriaProperty(element);
-        let allowedRoles = AriaUtil.getAllowedAriaRoles(element, tagProperty);
-        if (!allowedRoles || allowedRoles.length === 0)
+        if (!element || element.nodeType !== 1 
+            || ["html", "body"].includes(element.nodeName.toLowerCase())
+            || CommonUtil.getAncestor(element, ["svg", "pre", "code", "script", "meta", 'head']) !== null 
+            || !VisUtil.isNodeVisible(element) || VisUtil.isNodeVisuallyHidden(element) 
+            || CommonUtil.isNodeDisabled(element) || VisUtil.isElementOffscreen(element))
             return false;
 
-        let parent = element.parentElement;
-        // datalist, fieldset, optgroup, etc. may be just used for grouping purpose, so go up to the parent
-        while (parent && roles.some(role => role === 'group'))
-            parent = parent.parentElement;
+        if (element.hasAttribute("tabindex") || CommonUtil.isTabbable(element)) 
+            return true;
 
-        if (parent && (parent.hasAttribute("tabindex") || CommonUtil.isTabbable(parent))) {
-            const target_roles = ["listitem", "menuitem", "menuitemcheckbox", "menuitemradio", "option", "radio", "switch", "treeitem"];
-            if (allowedRoles.includes('any') || roles.some(role => target_roles.includes(role)))
+        const role = AriaUtil.getResolvedRole(element);
+        if (!role) return false;
+
+        const target_roles = ["listitem", "menuitem", "menuitemcheckbox", "menuitemradio", "option", "radio", "switch", "treeitem"];
+        if (target_roles.includes(role)) {
+            // find the proper parent elements
+            let parent = element.parentElement;
+            if (parent) {
+                const parent_role = AriaUtil.getResolvedRole(parent);
+                // datalist, fieldset, optgroup, etc. may be just used for grouping purpose, so go up to the parent
+                if (parent_role === 'group')
+                    parent = parent.parentElement;
+            }
+            
+            if (parent && CommonUtil.isTarget(parent))
                 return true;
-        }
+        }    
         return false;
     }
 
diff --git a/accessibility-checker-engine/src/v4/util/VisUtil.ts b/accessibility-checker-engine/src/v4/util/VisUtil.ts
index 26f43f9a5..7dec0285e 100644
--- a/accessibility-checker-engine/src/v4/util/VisUtil.ts
+++ b/accessibility-checker-engine/src/v4/util/VisUtil.ts
@@ -16,6 +16,7 @@ import { DOMUtil } from "../../v2/dom/DOMUtil";
 import { DOMWalker } from "../../v2/dom/DOMWalker";
 import { DOMMapper } from "../../v2/dom/DOMMapper";
 import { AriaUtil } from "./AriaUtil";
+import { CSSUtil } from "./CSSUtil";
 
 export class VisUtil {
     // This list contains a list of element tags which can not be hidden, when hidden is
@@ -231,6 +232,56 @@ export class VisUtil {
         return true;
     }
 
+    /**
+     * This function is responsible for checking if the node that is visually hidden by clipping or opaq:
+     *    1. Check if the current node is visually hidden:
+     *       CSS --> clip: rect(0px, 0px, 0px, 0px)
+     *       CSS --> opacity: 0
+     *
+     *    Note: If either current node or any of the parent nodes are visually hidden then this
+     *          function will return true (node is not visually hidden).
+     *
+     *    Note: nodes with CSS properties clip: rect(0px, 0px, 0px, 0px) or opacity:0 or filter:opacity(0%), or similar SVG mechanisms: 
+     *      They are not considered hidden to an AT. Text hidden with these methods can still be selected or copied, 
+     *      and user agents still expose it in their accessibility trees.  
+     * 
+     * @parm {element} node The node which should be checked if it is visually hidden or not.
+     * @return {bool} true if the node is visually hidden, false otherwise
+     *
+     * @memberOf VisUtil
+     */
+    public static isNodeVisuallyHidden(node: Node) : boolean {
+        if (!node || node.nodeType !== 1) return false;
+        
+        let elem = node as HTMLElement;
+        // Set PT_NODE_HIDDEN to false for all the nodes, before the check and this will be changed to
+        // true when we detect that the node is hidden. We have to set it to false so that we know
+        // the rules has already been checked.
+        const hidden = CacheUtil.getCache(elem, "PT_NODE_VISUALLY_HIDDEN", undefined);
+        if (hidden === undefined) {
+            // defined styles only give the styles that changed
+            const defined_styles = CSSUtil.getDefinedStyles(elem);
+            if ((defined_styles['position']==='absolute' && defined_styles['clip'] && defined_styles['clip'].replaceAll(' ', '')==='rect(0px,0px,0px,0px)')
+                || (defined_styles['opacity'] && parseFloat(defined_styles['opacity']) < 0.1))  {
+                CacheUtil.setCache(elem, "PT_NODE_VISUALLY_HIDDEN", true);
+                return true;
+            }
+
+            // Get the parentNode for this node, becuase we have to check all parents to make sure they do not have
+            // the hidden CSS, property or attribute. Only keep checking until we are all the way back to the parentNode
+            // element.
+            let parentElement = DOMWalker.parentElement(elem);
+            if (!parentElement)
+                return false;
+
+            // Check upwards recursively
+            const hid = VisUtil.isNodeVisuallyHidden(parentElement);
+            CacheUtil.setCache(elem, "PT_NODE_VISUALLY_HIDDEN", hid);
+            return hid;
+        }
+        return hidden;
+    }
+
     /**
      * return true if the node or its ancestor is hidden by CSS content-visibility:hidden
      * At this time, CSS content-visibility is partially supported by Chrome & Edge, but not supported by Firefox
@@ -273,19 +324,20 @@ export class VisUtil {
      * @param node
      */
     public static isElementOffscreen(node: HTMLElement) : boolean {
-        if (!node) return false;
+        if (!node) return true;
+        if (node.nodeType !== 1) return false;
+        
         const vis = CacheUtil.getCache(node , "PT_NODE_Offscreen", undefined);
         if (vis !== undefined) return vis;  
 
         const mapper : DOMMapper = new DOMMapper();
-        const bounds = mapper.getUnadjustedBounds(node);;    
-        
+        const bounds = mapper.getUnadjustedBounds(node); 
         if (!bounds) {
-            CacheUtil.setCache(node, "PT_NODE_Offscreen", false); 
-            return false;
+            CacheUtil.setCache(node, "PT_NODE_Offscreen", true); 
+            return true;
         }
         
-        if (bounds['height'] === 0 || bounds['width'] === 0 || bounds['top'] < 0 || bounds['left'] < 0) {
+        if (bounds['height'] === 0 || bounds['width'] === 0 || (bounds['top']+bounds['height']) <= 0 || (bounds['left']+bounds['width']) <= 0) {
             CacheUtil.setCache(node, "PT_NODE_Offscreen", true);
             return true;
         } 
diff --git a/accessibility-checker-engine/test/v2/checker/accessibility/rules/target_spacing_sufficient_ruleunit/element_clip.html b/accessibility-checker-engine/test/v2/checker/accessibility/rules/target_spacing_sufficient_ruleunit/element_clip.html
new file mode 100644
index 000000000..c486a1510
--- /dev/null
+++ b/accessibility-checker-engine/test/v2/checker/accessibility/rules/target_spacing_sufficient_ruleunit/element_clip.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html lang="en-us">
+
+<head>
+  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+</head>
+
+<body id="reference_zsr_1lb_vt">
+  <main role="main">
+      <article tabindex="0">
+        <div style="position: absolute; clip: rect(0, 0, 0, 0);">
+          <div>
+            <div tabindex="-1">
+        <span>
+          <a target="_blank"
+            href="https://www.ibm.com/investor/events/earnings-3q24" tabindex="-1">Replay: IBM
+            third-quarter 2024 earnings presentation
+          </a>
+        </span>
+      </div>
+    </div>
+        </div>
+      </article>
+      <script>
+        UnitTest = {
+          ruleIds: ["target_spacing_sufficient"],
+          results: [
+
+          ]
+        }
+      </script>
+</body>
+
+</html>
\ No newline at end of file