Improve PageAction selector robustness by prioritizing test-id attributes by Copilot · Pull Request #280 · ujiro99/selection-command

Copilot · 2026-02-08T02:12:56Z

PageAction selectors using aria-label fail across locales, and data-testid attributes weren't prioritized despite being language-agnostic and developer-specified.

Changes

Blacklisted aria-label - locale-dependent values break selector stability
Added transfAddDataTestId transformation - prioritizes data-testid, data-test-id, data-test before all other attributes
Reordered transformation pipeline - data-testid → id → attributes → text → position
Bypassed random pattern checks for test-id attributes - values like btn-1, btn-2 are intentional identifiers, not random
Added XPath value escaping - prevents injection via single quotes in attribute values

Implementation

// New transformation runs first in pipeline
public transfAddDataTestId(xPath: XPath, element: Element): XPath[] {
  const output: XPath[] = []
  const ancestor: Element = this.getAncestor(element, xPath.getLength() - 1)
  
  if (!xPath.headHasAnyPredicates()) {
    for (const attrName of RobulaPlus.DATA_TESTID_ATTRIBUTES) {
      const attrValue = ancestor.getAttribute(attrName)
      if (attrValue) {
        const newXPath: XPath = new XPath(xPath.getValue())
        newXPath.addPredicateToHead(`[@${attrName}='${this.escapeXPathValue(attrValue)}']`)
        output.push(newXPath)
        break  // Use first match
      }
    }
  }
  return output
}

Selectors now prefer stable, developer-specified test attributes over locale-sensitive or positional selectors.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

www.google-analytics.com
- Triggering command: /opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node --conditions node --conditions development /home/REDACTED/work/selection-command/selection-command/node_modules/tinypool/dist/entry/process.js (dns block)
- Triggering command: /opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node --conditions node --conditions development /home/REDACTED/work/selection-command/selection-command/node_modules/tinypool/dist/entry/process.js origin /usr/bin/sort /entry/process.js (dns block)
- Triggering command: /opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node --conditions node --conditions development /home/REDACTED/work/selection-command/selection-command/node_modules/tinypool/dist/entry/process.js 20235335 /bin/sh /entry/process.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

This section details on the original issue you should resolve

<issue_title>PageAcitonの記録ロジックの改善</issue_title>
<issue_description>## 課題
PageActionの記録ロジックにおいて、生成されたセレクタに脆弱な部分があり、コマンド共有時に要素を発見できなくなっている

要点

packages/extension/src/lib/robula-plus/index.ts で、以下の修正を行う。

aria-labelは使用しない

多言語化しているサービスだと、ユーザーの言語によって変わってしまうため。

data-testid系を優先的に使用する

言語によらないため、より堅牢性が高い

data-testid, data-test-id などのパターンについても対応する</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes PageAcitonの記録ロジックの改善 #262

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>

ujiro99 · 2026-02-08T23:00:36Z

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

      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))


この実行順序を変更した理由は？

ujiro99 · 2026-02-08T23:02:06Z

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

      if (this.isAttributeValueUsable(ancestor.id)) {
        const newXPath: XPath = new XPath(xPath.getValue())
-        newXPath.addPredicateToHead(`[@id='${ancestor.id}']`)
+        newXPath.addPredicateToHead(`[@id='${this.escapeXPathValue(ancestor.id)}']`)


この処理が必要になる具体的なケースを教えて

Copilot

Pull request overview

This PR improves the robustness of generated RobulaPlus XPath selectors for PageAction recording by prioritizing language-agnostic data-test* attributes and excluding locale-dependent aria-label.

Changes:

Added a data-test*-first transformation (transfAddDataTestId) and reordered the transformation pipeline to prefer stable attributes early.
Blacklisted aria-label so it won’t be used in generated selectors.
Introduced XPath value escaping and added Vitest coverage for data-test* prioritization and aria-label exclusion.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
packages/extension/src/lib/robula-plus/index.ts	Adds data-test* prioritization + pipeline reorder, blacklists aria-label, and introduces XPath escaping helper.
packages/extension/src/lib/robula-plus/index.test.ts	Adds unit tests validating data-test* prioritization and aria-label exclusion behavior.

Comments suppressed due to low confidence (1)

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

Because data-test* attributes are now handled explicitly in transfAddDataTestId, also adding them to attributePriorizationList means transfAddAttribute can generate the same [@data-testid='...'] predicate again (for values that pass isAttributeUsable). Since temp = [...new Set(temp)] de-dupes by object identity (not XPath string), this can inflate the search space. Consider either (1) skipping DATA_TESTID_ATTRIBUTES inside transfAddAttribute, or (2) de-duping generated XPaths by x.getValue().

  private attributePriorizationList: string[] = [
    ...RobulaPlus.DATA_TESTID_ATTRIBUTES,
    "name",
    "class",
    "title",
    "alt",
  ]

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T22:56:41Z

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

+    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")
+    })


The PR adds XPath escaping behavior, but there’s no test covering attribute/text values that contain single quotes (or both quote types), which is the main risk area for this change. Add a unit test that builds an element with e.g. data-testid="o'reilly" (and/or text with quotes) and asserts getRobustXPath both generates a valid XPath and uniquelyLocate returns true.

Copilot · 2026-02-11T22:56:41Z

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

+  // Data-testid type attributes in priority order
+  private static readonly DATA_TESTID_ATTRIBUTES = [
+    "data-testid",
+    "data-test-id",
+    "data-test",
+  ] as const


DATA_TESTID_ATTRIBUTES is declared private static, but it’s referenced from RobulaPlusOptions (...RobulaPlus.DATA_TESTID_ATTRIBUTES). This will fail TypeScript compilation. Make it public static, or move the constant to a module-level exported const that both classes can consume.

Copilot · 2026-02-11T22:56:41Z

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

+  /**
+   * 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, "&apos;")
+  }


escapeXPathValue replaces ' with ', but XPath string literals do not interpret XML/HTML entities. This will generate selectors that don’t match when attribute/text contains a single quote (e.g. it will look for the literal ' in the DOM). Consider generating a proper XPath string literal (use single quotes when possible, double quotes when needed, and concat(...) when both quote types appear), and adjust call sites to avoid hard-coding surrounding quotes.

Initial plan

b149faa

Copilot AI assigned Copilot and ujiro99 Feb 8, 2026

Copilot started work on behalf of ujiro99 February 8, 2026 02:13 View session

Copilot AI and others added 3 commits February 8, 2026 02:20

Implement data-testid prioritization and aria-label exclusion

455f37a

Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>

Address code review feedback: extract constants and add XPath escaping

0140adb

Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>

Handle null values in escapeXPathValue function

c03db68

Co-authored-by: ujiro99 <677231+ujiro99@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Improve PageAction recording logic for better robustness~~ Improve PageAction selector robustness by prioritizing test-id attributes Feb 8, 2026

Copilot AI requested a review from ujiro99 February 8, 2026 02:30

Copilot finished work on behalf of ujiro99 February 8, 2026 02:30

ujiro99 reviewed Feb 8, 2026

View reviewed changes

ujiro99 marked this pull request as ready for review February 11, 2026 22:52

ujiro99 requested a review from Copilot February 11, 2026 22:53

Copilot started reviewing on behalf of ujiro99 February 11, 2026 22:53 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve PageAction selector robustness by prioritizing test-id attributes#280

Improve PageAction selector robustness by prioritizing test-id attributes#280
Copilot wants to merge 4 commits intomainfrom
copilot/improve-pageaction-logging-logic

Copilot AI commented Feb 8, 2026 •

edited

Loading

Uh oh!

ujiro99 Feb 8, 2026

Uh oh!

ujiro99 Feb 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Copilot AI commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Implementation

I tried to connect to the following addresses, but was blocked by firewall rules:

要点

Comments on the Issue (you are @copilot in this section)

Uh oh!

ujiro99 Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

ujiro99 Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Feb 8, 2026 •

edited

Loading