Improve PageAction selector robustness by prioritizing test-id attributes

packages/extension/src/lib/robula-plus/index.test.ts

@@ -0,0 +1,202 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest"
+import { RobulaPlus } from "./index"
+
+describe("RobulaPlus", () => {
+  let container: HTMLElement
+
+  beforeEach(() => {
+    // Create a fresh container for each test
+    container = document.createElement("div")
+    document.body.appendChild(container)
+  })
+
+  afterEach(() => {
+    // Clean up after each test
+    document.body.removeChild(container)
+  })
+
+  describe("Attribute prioritization", () => {
+    it("should prioritize data-testid over other attributes", () => {
+      container.innerHTML = `
+        <div>
+          <button data-testid="submit-btn" class="btn-primary" name="submit">Submit</button>
+          <button data-testid="cancel-btn" class="btn-secondary" name="cancel">Cancel</button>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(buttons[0], document)
+
+      // Should use data-testid in the XPath because there are multiple buttons
+      expect(xpath).toContain("data-testid")
+      expect(xpath).toContain("submit-btn")
+    })
+
+    it("should prioritize data-test-id over other attributes", () => {
+      container.innerHTML = `
+        <div>
+          <button data-test-id="login-btn" class="btn-secondary" name="login">Login</button>
+          <button data-test-id="logout-btn" class="btn-secondary" name="logout">Logout</button>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(buttons[0], document)
+
+      // Should use data-test-id in the XPath
+      expect(xpath).toContain("data-test-id")
+      expect(xpath).toContain("login-btn")
+    })
+
+    it("should prioritize data-test over other attributes", () => {
+      container.innerHTML = `
+        <div>
+          <button data-test="cancel-btn" class="btn-cancel" name="cancel">Cancel</button>
+          <button data-test="ok-btn" class="btn-ok" name="ok">OK</button>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(buttons[0], document)
+
+      // Should use data-test in the XPath
+      expect(xpath).toContain("data-test")
+      expect(xpath).toContain("cancel-btn")
+    })
+
+    it("should use data-testid even when name and class are present", () => {
+      container.innerHTML = `
+        <div>
+          <input data-testid="username-input" name="username" class="form-input" type="text" />
+          <input data-testid="password-input" name="password" class="form-input" type="text" />
+        </div>
+      `
+      const inputs = container.querySelectorAll("input")
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(inputs[0], document)
+
+      // Should prefer data-testid over name and class (and type which both have)
+      expect(xpath).toContain("data-testid")
+      expect(xpath).toContain("username-input")
+    })
+  })
+
+  describe("aria-label exclusion", () => {
+    it("should not use aria-label in XPath selectors", () => {
+      container.innerHTML = `
+        <div>
+          <button aria-label="Submit form" class="btn-submit">送信</button>
+        </div>
+      `
+      const button = container.querySelector("button")!
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(button, document)
+
+      // Should NOT use aria-label in the XPath
+      expect(xpath).not.toContain("aria-label")
+      expect(xpath).not.toContain("Submit form")
+    })
+
+    it("should not use aria-label even when it's the only distinctive attribute", () => {
+      container.innerHTML = `
+        <div>
+          <div aria-label="Navigation menu">
+            <button>Click me</button>
+          </div>
+          <div aria-label="Footer menu">
+            <button>Click me</button>
+          </div>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+
+      const xpath1 = robula.getRobustXPath(buttons[0], document)
+      const xpath2 = robula.getRobustXPath(buttons[1], document)
+
+      // Should NOT use aria-label anywhere in the XPath
+      expect(xpath1).not.toContain("aria-label")
+      expect(xpath2).not.toContain("aria-label")
+
+      // The XPaths should still uniquely identify each button (using position or other attributes)
+      expect(robula.uniquelyLocate(xpath1, buttons[0], document)).toBe(true)
+      expect(robula.uniquelyLocate(xpath2, buttons[1], document)).toBe(true)
+    })
+
+    it("should prefer other attributes over aria-label", () => {
+      container.innerHTML = `
+        <div>
+          <button aria-label="Submit the form" name="submit-btn">Submit</button>
+          <button aria-label="Cancel the form" name="cancel-btn">Cancel</button>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(buttons[0], document)
+
+      // Should use name instead of aria-label
+      expect(xpath).not.toContain("aria-label")
+      expect(xpath).toContain("name")
+      expect(xpath).toContain("submit-btn")
+    })
+  })
+
+  describe("Combined scenarios", () => {
+    it("should prioritize data-testid over name, class, and aria-label", () => {
+      container.innerHTML = `
+        <div>
+          <button 
+            data-testid="primary-action"
+            aria-label="Primary action button"
+            name="action"
+            class="btn-primary"
+          >
+            Click
+          </button>
+          <button 
+            data-testid="secondary-action"
+            aria-label="Secondary action button"
+            name="action"
+            class="btn-secondary"
+          >
+            Click
+          </button>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+      const xpath = robula.getRobustXPath(buttons[0], document)
+
+      // Should use data-testid and not aria-label
+      expect(xpath).toContain("data-testid")
+      expect(xpath).toContain("primary-action")
+      expect(xpath).not.toContain("aria-label")
+    })
+
+    it("should work with multiple elements with data-testid", () => {
+      container.innerHTML = `
+        <div>
+          <button data-testid="btn-1">Button</button>
+          <button data-testid="btn-2">Button</button>
+          <button data-testid="btn-3">Button</button>
+        </div>
+      `
+      const buttons = container.querySelectorAll("button")
+      const robula = new RobulaPlus()
+
+      const xpath1 = robula.getRobustXPath(buttons[0], document)
+      const xpath2 = robula.getRobustXPath(buttons[1], document)
+      const xpath3 = robula.getRobustXPath(buttons[2], document)
+
+      // Each should use its unique data-testid
+      expect(xpath1).toContain("btn-1")
+      expect(xpath2).toContain("btn-2")
+      expect(xpath3).toContain("btn-3")
+
+      // Each should uniquely identify its element
+      expect(robula.uniquelyLocate(xpath1, buttons[0], document)).toBe(true)
+      expect(robula.uniquelyLocate(xpath2, buttons[1], document)).toBe(true)
+      expect(robula.uniquelyLocate(xpath3, buttons[2], document)).toBe(true)
+    })
+  })
+})

packages/extension/src/lib/robula-plus/index.ts

@@ -11,7 +11,15 @@
  * @param options - (optional) algorithm options.
  */
 export class RobulaPlus {
+  // Data-testid type attributes in priority order
+  private static readonly DATA_TESTID_ATTRIBUTES = [
+    "data-testid",
+    "data-test-id",
+    "data-test",
+  ] as const
+
   private attributePriorizationList: string[] = [
+    ...RobulaPlus.DATA_TESTID_ATTRIBUTES,
     "name",
     "class",
     "title",
@@ -29,6 +37,7 @@ export class RobulaPlus {
     "size",
     "maxlength",
     "value",
+    "aria-label",
   ]
 
   // Flag to determine whether to detect random number patterns
@@ -93,9 +102,10 @@ export class RobulaPlus {
       const xPath: XPath = xPathList.shift()!
       let temp: XPath[] = []
       temp = temp.concat(this.transfConvertStar(xPath, element))
+      temp = temp.concat(this.transfAddDataTestId(xPath, element))
       temp = temp.concat(this.transfAddId(xPath, element))
-      temp = temp.concat(this.transfAddText(xPath, element))
       temp = temp.concat(this.transfAddAttribute(xPath, element))
+      temp = temp.concat(this.transfAddText(xPath, element))
       temp = temp.concat(this.transfAddAttributeSet(xPath, element))
       temp = temp.concat(this.transfAddPosition(xPath, element))
       temp = temp.concat(this.transfAddLevel(xPath, element))
@@ -159,6 +169,18 @@ export class RobulaPlus {
     }
   }
 
+  /**
+   * Escapes single quotes in attribute values for safe XPath generation
+   * @param value - The attribute value to escape
+   * @returns The escaped value safe for use in XPath predicates
+   */
+  private escapeXPathValue(value: string | null | undefined): string {
+    if (value === null || value === undefined) {
+      return ""
+    }
+    return value.replace(/'/g, "'")
+  }
+
   public transfConvertStar(xPath: XPath, element: Element): XPath[] {
     const output: XPath[] = []
     const ancestor: Element = this.getAncestor(element, xPath.getLength() - 1)
@@ -177,13 +199,35 @@ export class RobulaPlus {
       // Only add ID if it doesn't contain React useId patterns
       if (this.isAttributeValueUsable(ancestor.id)) {
         const newXPath: XPath = new XPath(xPath.getValue())
-        newXPath.addPredicateToHead(`[@id='${ancestor.id}']`)
+        newXPath.addPredicateToHead(`[@id='${this.escapeXPathValue(ancestor.id)}']`)
         output.push(newXPath)
       }
     }
     return output
   }
 
+  public transfAddDataTestId(xPath: XPath, element: Element): XPath[] {
+    const output: XPath[] = []
+    const ancestor: Element = this.getAncestor(element, xPath.getLength() - 1)
+
+    if (!xPath.headHasAnyPredicates()) {
+      // Check for data-testid type attributes in priority order
+      for (const attrName of RobulaPlus.DATA_TESTID_ATTRIBUTES) {
+        const attrValue = ancestor.getAttribute(attrName)
+        // For data-testid attributes, we don't check for random patterns
+        // because these are explicitly set by developers for testing purposes
+        if (attrValue) {
+          const newXPath: XPath = new XPath(xPath.getValue())
+          newXPath.addPredicateToHead(`[@${attrName}='${this.escapeXPathValue(attrValue)}']`)
+          output.push(newXPath)
+          // Return immediately after finding the first data-test* attribute
+          break
+        }
+      }
+    }
+    return output
+  }
+
   public transfAddText(xPath: XPath, element: Element): XPath[] {
     const output: XPath[] = []
     const ancestor: Element = this.getAncestor(element, xPath.getLength() - 1)
@@ -195,7 +239,7 @@ export class RobulaPlus {
     ) {
       const newXPath: XPath = new XPath(xPath.getValue())
       newXPath.addPredicateToHead(
-        `[contains(text(),'${ancestor.textContent}')]`,
+        `[contains(text(),'${this.escapeXPathValue(ancestor.textContent)}')]`,
       )
       output.push(newXPath)
     }
@@ -215,7 +259,7 @@ export class RobulaPlus {
           ) {
             const newXPath: XPath = new XPath(xPath.getValue())
             newXPath.addPredicateToHead(
-              `[@${attribute.name}='${attribute.value}']`,
+              `[@${attribute.name}='${this.escapeXPathValue(attribute.value)}']`,
             )
             output.push(newXPath)
             break
@@ -230,7 +274,7 @@ export class RobulaPlus {
         ) {
           const newXPath: XPath = new XPath(xPath.getValue())
           newXPath.addPredicateToHead(
-            `[@${attribute.name}='${attribute.value}']`,
+            `[@${attribute.name}='${this.escapeXPathValue(attribute.value)}']`,
           )
           output.push(newXPath)
         }
@@ -286,9 +330,9 @@ export class RobulaPlus {
 
       // convert to predicate
       for (const attributeSet of attributePowerSet) {
-        let predicate: string = `[@${attributeSet[0].name}='${attributeSet[0].value}'`
+        let predicate: string = `[@${attributeSet[0].name}='${this.escapeXPathValue(attributeSet[0].value)}'`
         for (let i: number = 1; i < attributeSet.length; i++) {
-          predicate += ` and @${attributeSet[i].name}='${attributeSet[i].value}'`
+          predicate += ` and @${attributeSet[i].name}='${this.escapeXPathValue(attributeSet[i].value)}'`
         }
         predicate += "]"
         const newXPath: XPath = new XPath(xPath.getValue())
@@ -575,6 +619,7 @@ export class RobulaPlusOptions {
    */
 
   public attributePriorizationList: string[] = [
+    ...RobulaPlus.DATA_TESTID_ATTRIBUTES,
     "name",
     "class",
     "title",
@@ -592,6 +637,7 @@ export class RobulaPlusOptions {
     "size",
     "maxlength",
     "value",
+    "aria-label",
   ]
   public avoidRandomPatterns?: boolean
   public randomPatterns?: RegExp[]