Skip to content

Commit 730165b

Browse files
quanruquanru
committed
fix(shared): include svg tag index in xpath cache for multiple icons
Previously, when multiple SVG icons existed under the same parent element (e.g., td[34]/svg[1], td[34]/svg[2], td[34]/svg[4]), the XPath cache would skip all SVG elements and only store the parent element's path (td[34]). This caused cache misses when trying to locate specific SVG icons. Changes: - Modified getElementXpath() to include top-level <svg> tags with indices - SVG child elements (path, circle, rect) skip to nearest <svg> container - Allows proper distinction between multiple SVG icons in same parent Example: - Before: Clicking td[34]/svg[4] caches as /html/body/.../td[34] - After: Clicking td[34]/svg[4] caches as /html/body/.../td[34]/svg[4] Fixes issue where cached XPath cannot distinguish between multiple SVG icons in table cells or action columns.
1 parent 7269c7f commit 730165b

3 files changed

Lines changed: 196 additions & 8 deletions

File tree

‎packages/shared/src/extractor/locator.ts‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,35 @@ export const getElementXpath = (
8787
if (el === document.documentElement) return '/html';
8888
if (el === document.body) return '/html/body';
8989

90-
// if the element is any SVG element, find the nearest non-SVG ancestor
90+
// if the element is any SVG element, handle based on tag type
9191
if (isSvgElement(el)) {
92+
const tagName = el.nodeName.toLowerCase();
93+
94+
// For top-level <svg> tag, include it in the path to distinguish between multiple SVG icons
95+
// This is important when there are multiple SVG elements under the same parent (e.g., td[34]/svg[1], td[34]/svg[2])
96+
if (tagName === 'svg') {
97+
// Include the <svg> tag with its index in the XPath
98+
return buildCurrentElementXpath(el, isOrderSensitive, isLeafElement);
99+
}
100+
101+
// For SVG child elements (path, circle, rect, etc.), skip to the nearest <svg> ancestor
102+
// These internal elements are usually decorative and can change frequently
92103
let parent = el.parentNode;
93104
while (parent && parent.nodeType === Node.ELEMENT_NODE) {
94-
if (!isSvgElement(parent)) {
95-
return getElementXpath(parent, isOrderSensitive, isLeafElement);
105+
const parentEl = parent as Element;
106+
if (isSvgElement(parentEl)) {
107+
const parentTag = parentEl.nodeName.toLowerCase();
108+
if (parentTag === 'svg') {
109+
// Found the <svg> container, return its path
110+
return getElementXpath(parentEl, isOrderSensitive, isLeafElement);
111+
}
112+
} else {
113+
// Found a non-SVG ancestor, return its path
114+
return getElementXpath(parentEl, isOrderSensitive, isLeafElement);
96115
}
97116
parent = parent.parentNode;
98117
}
99-
// fallback if no non-SVG parent found
118+
// fallback if no suitable parent found
100119
return getElementXpath(el.parentNode!, isOrderSensitive, isLeafElement);
101120
}
102121

‎packages/shared/tests/unit-test/locator-svg-multiple.test.ts‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { beforeEach, describe, expect, it } from 'vitest';
2+
import { getXpathsByPoint } from '../../src/extractor/locator';
3+
4+
// Mock DOM environment for testing
5+
class MockElement {
6+
nodeName: string;
7+
nodeType: number;
8+
namespaceURI?: string;
9+
parentNode: MockElement | null;
10+
previousElementSibling: MockElement | null;
11+
textContent: string;
12+
13+
constructor(
14+
nodeName: string,
15+
textContent = '',
16+
namespaceURI?: string,
17+
parentNode: MockElement | null = null,
18+
) {
19+
this.nodeName = nodeName;
20+
this.nodeType = 1; // ELEMENT_NODE
21+
this.namespaceURI = namespaceURI;
22+
this.textContent = textContent;
23+
this.parentNode = parentNode;
24+
this.previousElementSibling = null;
25+
}
26+
}
27+
28+
// Mock SVG element that extends SVGElement
29+
class MockSVGElement extends MockElement {
30+
constructor(
31+
nodeName: string,
32+
textContent = '',
33+
parentNode: MockElement | null = null,
34+
) {
35+
super(nodeName, textContent, 'http://www.w3.org/2000/svg', parentNode);
36+
}
37+
}
38+
39+
// Mock global objects needed by locator functions
40+
const setupMockDOM = () => {
41+
global.Node = {
42+
ELEMENT_NODE: 1,
43+
TEXT_NODE: 3,
44+
} as any;
45+
46+
// Mock SVGElement for SVG handling
47+
global.SVGElement = MockSVGElement as any;
48+
49+
global.document = {
50+
documentElement: new MockElement('html'),
51+
body: new MockElement('body'),
52+
elementFromPoint: () => null,
53+
} as any;
54+
55+
global.window = {} as any;
56+
global.HTMLElement = MockElement as any;
57+
};
58+
59+
describe('locator - multiple SVG icons', () => {
60+
beforeEach(() => {
61+
setupMockDOM();
62+
});
63+
64+
it('should distinguish between multiple SVG icons in the same parent (user issue)', () => {
65+
// Simulate the user's scenario: td[34] with multiple svg children
66+
// The user has: td[34]/svg[1], td[34]/svg[2], td[34]/svg[3], td[34]/svg[4]
67+
// Clicking on svg[4] should return xpath ending with /svg[4], not just /td[34]
68+
69+
global.document.elementFromPoint = (x: number, y: number) => {
70+
if (x === 100 && y === 100) {
71+
// Simulate clicking on the 4th SVG icon (the edit icon)
72+
const tr = new MockElement('tr');
73+
const td = new MockElement('td');
74+
75+
// Create 4 SVG icons (like in the user's table)
76+
const svg1 = new MockSVGElement('svg', '');
77+
const svg2 = new MockSVGElement('svg', '');
78+
const svg3 = new MockSVGElement('svg', '');
79+
const svg4 = new MockSVGElement('svg', ''); // The edit icon
80+
81+
// Create internal path elements for each SVG
82+
const path4 = new MockSVGElement('path', '');
83+
84+
// Set up parent chain
85+
tr.parentNode = global.document.body as any;
86+
td.parentNode = tr;
87+
svg1.parentNode = td;
88+
svg2.parentNode = td;
89+
svg3.parentNode = td;
90+
svg4.parentNode = td;
91+
path4.parentNode = svg4;
92+
93+
// Set up sibling chain
94+
svg1.previousElementSibling = null;
95+
svg2.previousElementSibling = svg1;
96+
svg3.previousElementSibling = svg2;
97+
svg4.previousElementSibling = svg3;
98+
99+
// Return the path inside svg4 (simulating clicking on the edit icon)
100+
return path4 as any;
101+
}
102+
return null;
103+
};
104+
105+
const point = { left: 100, top: 100 };
106+
const xpaths = getXpathsByPoint(point, true);
107+
108+
expect(xpaths).toBeDefined();
109+
expect(xpaths).toHaveLength(1);
110+
111+
// Should include svg[4] to distinguish from other SVG icons
112+
expect(xpaths?.[0]).toMatch(/svg\[4\]/);
113+
114+
// Should not include the internal path element
115+
expect(xpaths?.[0]).not.toMatch(/path/);
116+
117+
// Should include the td and svg with proper indices
118+
expect(xpaths?.[0]).toMatch(/\/td\[1\]\/svg\[4\]$/);
119+
120+
console.log('XPath for svg[4]:', xpaths?.[0]);
121+
});
122+
123+
it('should distinguish between different SVG icons in the same cell', () => {
124+
// Test that we can generate different xpaths for different SVG icons
125+
global.document.elementFromPoint = (x: number, y: number) => {
126+
const tr = new MockElement('tr');
127+
const td = new MockElement('td');
128+
129+
const svg1 = new MockSVGElement('svg', '');
130+
const svg2 = new MockSVGElement('svg', '');
131+
const svg3 = new MockSVGElement('svg', '');
132+
133+
tr.parentNode = global.document.body as any;
134+
td.parentNode = tr;
135+
svg1.parentNode = td;
136+
svg2.parentNode = td;
137+
svg3.parentNode = td;
138+
139+
svg1.previousElementSibling = null;
140+
svg2.previousElementSibling = svg1;
141+
svg3.previousElementSibling = svg2;
142+
143+
// Return different SVG based on coordinates
144+
if (x === 100) return svg1 as any;
145+
if (x === 200) return svg2 as any;
146+
if (x === 300) return svg3 as any;
147+
return null;
148+
};
149+
150+
const xpath1 = getXpathsByPoint({ left: 100, top: 100 }, true);
151+
const xpath2 = getXpathsByPoint({ left: 200, top: 100 }, true);
152+
const xpath3 = getXpathsByPoint({ left: 300, top: 100 }, true);
153+
154+
// All xpaths should be different
155+
expect(xpath1?.[0]).not.toBe(xpath2?.[0]);
156+
expect(xpath2?.[0]).not.toBe(xpath3?.[0]);
157+
expect(xpath1?.[0]).not.toBe(xpath3?.[0]);
158+
159+
// Each should have the correct index
160+
expect(xpath1?.[0]).toMatch(/svg\[1\]$/);
161+
expect(xpath2?.[0]).toMatch(/svg\[2\]$/);
162+
expect(xpath3?.[0]).toMatch(/svg\[3\]$/);
163+
164+
console.log('XPath for svg[1]:', xpath1?.[0]);
165+
console.log('XPath for svg[2]:', xpath2?.[0]);
166+
console.log('XPath for svg[3]:', xpath3?.[0]);
167+
});
168+
});

‎packages/shared/tests/unit-test/locator.test.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,13 @@ describe('locator', () => {
201201

202202
expect(xpaths).toBeDefined();
203203
expect(xpaths).toHaveLength(1);
204-
// Should return the xpath of the button (non-SVG ancestor), not the path element
204+
// Should include the <svg> tag in the xpath to distinguish between multiple SVG icons
205+
// but skip internal SVG child elements (path)
205206
expect(xpaths?.[0]).toMatch(/button/);
207+
expect(xpaths?.[0]).toMatch(/svg/);
206208
expect(xpaths?.[0]).not.toMatch(/path/);
207-
expect(xpaths?.[0]).not.toMatch(/svg/);
208-
// Should be the button's xpath
209-
expect(xpaths?.[0]).toBe('/html/body/button[1]');
209+
// Should include the svg tag with its index
210+
expect(xpaths?.[0]).toBe('/html/body/button[1]/svg[1]');
210211
});
211212

212213
it('should handle text nodes with order-sensitive mode', () => {

0 commit comments

Comments
 (0)