Skip to content

fix: extract clean data #822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 21, 2025
Merged

fix: extract clean data #822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b645664
feat: filter atomic child elements
RohitR311 Oct 8, 2025
7645983
fix: disable postCSS processing
RohitR311 Oct 11, 2025
ce2e28a
fix: prioritize link elements
RohitR311 Oct 13, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/recorder/DOMBrowserRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,7 @@ export const DOMBrowserRenderer: React.FC<RRWebDOMBrowserRendererProps> = ({
rebuild(snapshotData.snapshot, {
doc: iframeDoc,
mirror: mirror,
hackCss: false,
cache: { stylesWithHoverClass: new Map() },
afterAppend: (node) => {
if (node.nodeType === Node.TEXT_NODE && node.textContent) {
Expand Down
191 changes: 150 additions & 41 deletions src/helpers/clientSelectorGenerator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,36 +555,23 @@ class ClientSelectorGenerator {
*/
private isMeaningfulElement(element: HTMLElement): boolean {
const tagName = element.tagName.toLowerCase();

// Fast path for common meaningful elements
if (["a", "img", "input", "button", "select"].includes(tagName)) {
return true;

if (tagName === "img") {
return element.hasAttribute("src");
}

const text = (element.textContent || "").trim();
const hasHref = element.hasAttribute("href");
const hasSrc = element.hasAttribute("src");

// Quick checks first
if (text.length > 0 || hasHref || hasSrc) {
if (tagName === "a" && element.hasAttribute("href")) {
return true;
}

const isCustomElement = tagName.includes("-");
if (element.children.length > 0) {
return false;
}

// For custom elements, be more lenient about what's considered meaningful
if (isCustomElement) {
const hasChildren = element.children.length > 0;
const hasSignificantAttributes = Array.from(element.attributes).some(
(attr) => !["class", "style", "id"].includes(attr.name.toLowerCase())
);
const text = (element.textContent || "").trim();

return (
hasChildren ||
hasSignificantAttributes ||
element.hasAttribute("role") ||
element.hasAttribute("aria-label")
);
if (text.length > 0) {
return true;
}

return false;
Expand Down Expand Up @@ -2561,12 +2548,9 @@ class ClientSelectorGenerator {

const MAX_MEANINGFUL_ELEMENTS = 300;
const MAX_NODES_TO_CHECK = 1200;
const MAX_DEPTH = 12;
const MAX_DEPTH = 20;
let nodesChecked = 0;

let adjustedMaxDepth = MAX_DEPTH;
const elementDensityThreshold = 50;

const depths: number[] = [0];
let queueIndex = 0;

Expand All @@ -2576,14 +2560,10 @@ class ClientSelectorGenerator {
queueIndex++;
nodesChecked++;

if (currentDepth <= 3 && meaningfulDescendants.length > elementDensityThreshold) {
adjustedMaxDepth = Math.max(6, adjustedMaxDepth - 2);
}

if (
nodesChecked > MAX_NODES_TO_CHECK ||
meaningfulDescendants.length >= MAX_MEANINGFUL_ELEMENTS ||
currentDepth > adjustedMaxDepth
currentDepth > MAX_DEPTH
) {
break;
}
Expand All @@ -2592,7 +2572,7 @@ class ClientSelectorGenerator {
meaningfulDescendants.push(element);
}

if (currentDepth >= adjustedMaxDepth) {
if (currentDepth >= MAX_DEPTH) {
continue;
}

Expand All @@ -2607,7 +2587,7 @@ class ClientSelectorGenerator {
}
}

if (element.shadowRoot && currentDepth < adjustedMaxDepth - 1) {
if (element.shadowRoot && currentDepth < MAX_DEPTH - 1) {
const shadowChildren = element.shadowRoot.children;
const shadowLimit = Math.min(shadowChildren.length, 20);
for (let i = 0; i < shadowLimit; i++) {
Expand Down Expand Up @@ -2716,22 +2696,46 @@ class ClientSelectorGenerator {
}

if (!addPositionToAll) {
const meaningfulAttrs = ["role", "type", "name", "src", "aria-label"];
const meaningfulAttrs = ["role", "type"];
for (const attrName of meaningfulAttrs) {
if (element.hasAttribute(attrName)) {
const value = element.getAttribute(attrName)!.replace(/'/g, "\\'");
return `${tagName}[@${attrName}='${value}']`;
const isCommonAttribute = this.isAttributeCommonAcrossLists(
element,
attrName,
value,
otherListElements
);
if (isCommonAttribute) {
return `${tagName}[@${attrName}='${value}']`;
}
}
}
}
Comment on lines 2698 to 2714
Copy link

@coderabbitai coderabbitai bot Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Cross-list attribute check maps by absolute path; compute relative path to the list root and fail closed.

isAttributeCommonAcrossLists builds a path from the document root (getElementPath) then applies it starting from other list roots. This mismaps descendants and treats “corresponding element not found” as implicitly OK, letting non-common attributes pass. Compute a path relative to the current list root and require a match in every other list.

Apply these diffs:

@@ - if (!addPositionToAll) { - const meaningfulAttrs = ["role", "type"]; + if (!addPositionToAll) { + const meaningfulAttrs = ["role", "type"]; for (const attrName of meaningfulAttrs) { if (element.hasAttribute(attrName)) { const value = element.getAttribute(attrName)!.replace(/'/g, "\\'"); - const isCommonAttribute = this.isAttributeCommonAcrossLists( - element, - attrName, - value, - otherListElements - ); + const isCommonAttribute = rootElement + ? this.isAttributeCommonAcrossLists( + element, + rootElement, + attrName, + value, + otherListElements + ) + : false; if (isCommonAttribute) { return `${tagName}[@${attrName}='${value}']`; } } } } @@ - const testId = element.getAttribute("data-testid"); + const testId = element.getAttribute("data-testid"); if (testId && !addPositionToAll) { - const isCommon = this.isAttributeCommonAcrossLists( - element, - "data-testid", - testId, - otherListElements - ); + const isCommon = rootElement + ? this.isAttributeCommonAcrossLists( + element, + rootElement, + "data-testid", + testId, + otherListElements + ) + : false; if (isCommon) { return `${tagName}[@data-testid='${testId}']`; } } @@ - if (element.id && !element.id.match(/^\d/) && !addPositionToAll) { - const isCommon = this.isAttributeCommonAcrossLists( - element, - "id", - element.id, - otherListElements - ); + if (element.id && !element.id.match(/^\d/) && !addPositionToAll) { + const isCommon = rootElement + ? this.isAttributeCommonAcrossLists( + element, + rootElement, + "id", + element.id, + otherListElements + ) + : false; if (isCommon) { return `${tagName}[@id='${element.id}']`; } } @@ - if (!addPositionToAll) { + if (!addPositionToAll) { for (const attr of Array.from(element.attributes)) { if ( attr.name.startsWith("data-") && attr.name !== "data-testid" && attr.name !== "data-mx-id" && attr.value ) { - const isCommon = this.isAttributeCommonAcrossLists( - element, - attr.name, - attr.value, - otherListElements - ); + const isCommon = rootElement + ? this.isAttributeCommonAcrossLists( + element, + rootElement, + attr.name, + attr.value, + otherListElements + ) + : false; if (isCommon) { return `${tagName}[@${attr.name}='${attr.value}']`; } } } } @@ - private isAttributeCommonAcrossLists( - targetElement: HTMLElement, - attrName: string, - attrValue: string, - otherListElements: HTMLElement[] - ): boolean { - if (otherListElements.length === 0) { - return true; - } - - const targetPath = this.getElementPath(targetElement); - - for (const otherListElement of otherListElements) { - const correspondingElement = this.findCorrespondingElement( - otherListElement, - targetPath - ); - if (correspondingElement) { - const otherValue = correspondingElement.getAttribute(attrName); - if (otherValue !== attrValue) { - return false; - } - } - } - - return true; - } + private isAttributeCommonAcrossLists( + targetElement: HTMLElement, + rootElement: HTMLElement, + attrName: string, + attrValue: string, + otherListElements: HTMLElement[] + ): boolean { + if (otherListElements.length === 0) return true; + const relPath = this.getElementPath(targetElement, rootElement); + if (!relPath) return false; // cannot map reliably relative to root + for (const otherRoot of otherListElements) { + const corresponding = this.findCorrespondingElement(otherRoot, relPath); + if (!corresponding) return false; + if (corresponding.getAttribute(attrName) !== attrValue) return false; + } + return true; + } @@ - private getElementPath(element: HTMLElement): number[] { - const path: number[] = []; - let current: HTMLElement | null = element; - - while (current && current.parentElement) { - const siblings = Array.from(current.parentElement.children); - path.unshift(siblings.indexOf(current)); - current = current.parentElement; - } - - return path; - } + private getElementPath( + element: HTMLElement, + rootElement: HTMLElement + ): number[] | null { + const path: number[] = []; + let current: HTMLElement | null = element; + // Walk up until rootElement (crossing shadow hosts if needed) + while (current && current !== rootElement) { + const parent = + current.parentElement || + ((current.getRootNode() as ShadowRoot).host as HTMLElement | null); + if (!parent) return null; + const siblings = Array.from(parent.children); + const idx = siblings.indexOf(current); + if (idx < 0) return null; + path.unshift(idx); + current = parent; + } + return current === rootElement ? path : null; + }

Also applies to: 2717-2728, 2730-2740, 2749-2759, 2926-2953, 2954-2966, 2967-2983

🤖 Prompt for AI Agents 
In src/helpers/clientSelectorGenerator.ts around lines 2699-2715 (and also apply same change to ranges 2717-2728, 2730-2740, 2749-2759, 2926-2953, 2954-2966, 2967-2983): the current isAttributeCommonAcrossLists logic builds element paths from the document root and then queries other list roots using those absolute paths, which can map to wrong descendants and treats "not found" as success; change it to compute the element path relative to the current list root (strip or build the path starting from the list root node) and when checking otherListElements require that the corresponding relative path resolves to an element and that its attribute value equals the expected value in every other list (i.e., treat missing matches as failure), returning false if any list does not have a matching element or attribute value; update all mentioned code ranges to use the relative-path lookup and strict matching.

const testId = element.getAttribute("data-testid");
if (testId && !addPositionToAll) {
return `${tagName}[@data-testid='${testId}']`;
const isCommon = this.isAttributeCommonAcrossLists(
element,
"data-testid",
testId,
otherListElements
);
if (isCommon) {
return `${tagName}[@data-testid='${testId}']`;
}
}

if (element.id && !element.id.match(/^\d/) && !addPositionToAll) {
return `${tagName}[@id='${element.id}']`;
const isCommon = this.isAttributeCommonAcrossLists(
element,
"id",
element.id,
otherListElements
);
if (isCommon) {
return `${tagName}[@id='${element.id}']`;
}
}

if (!addPositionToAll) {
Expand All @@ -2742,7 +2746,15 @@ class ClientSelectorGenerator {
attr.name !== "data-mx-id" &&
attr.value
) {
return `${tagName}[@${attr.name}='${attr.value}']`;
const isCommon = this.isAttributeCommonAcrossLists(
element,
attr.name,
attr.value,
otherListElements
);
if (isCommon) {
return `${tagName}[@${attr.name}='${attr.value}']`;
}
}
}
}
Expand Down Expand Up @@ -2906,12 +2918,70 @@ class ClientSelectorGenerator {
const result = pathParts.length > 0 ? "/" + pathParts.join("/") : null;

this.pathCache.set(targetElement, result);

return result;
}

private isAttributeCommonAcrossLists(
targetElement: HTMLElement,
attrName: string,
attrValue: string,
otherListElements: HTMLElement[]
): boolean {
if (otherListElements.length === 0) {
return true;
}

const targetPath = this.getElementPath(targetElement);

for (const otherListElement of otherListElements) {
const correspondingElement = this.findCorrespondingElement(
otherListElement,
targetPath
);
if (correspondingElement) {
const otherValue = correspondingElement.getAttribute(attrName);
if (otherValue !== attrValue) {
return false;
}
}
}

return true;
}

private getElementPath(element: HTMLElement): number[] {
const path: number[] = [];
let current: HTMLElement | null = element;

while (current && current.parentElement) {
const siblings = Array.from(current.parentElement.children);
path.unshift(siblings.indexOf(current));
current = current.parentElement;
}

return path;
}

private findCorrespondingElement(
rootElement: HTMLElement,
path: number[]
): HTMLElement | null {
let current: HTMLElement = rootElement;

for (const index of path) {
const children = Array.from(current.children);
if (index >= children.length) {
return null;
}
current = children[index] as HTMLElement;
}

return current;
}

private getCommonClassesAcrossLists(
targetElement: HTMLElement,
targetElement: HTMLElement,
otherListElements: HTMLElement[]
): string[] {
if (otherListElements.length === 0) {
Expand Down Expand Up @@ -3919,9 +3989,48 @@ class ClientSelectorGenerator {
);
if (!deepestElement) return null;

if (!this.isMeaningfulElementCached(deepestElement)) {
const atomicChild = this.findAtomicChildAtPoint(deepestElement, x, y);
if (atomicChild) {
return atomicChild;
}
}

Comment on lines +3992 to +3998
Copy link

@coderabbitai coderabbitai bot Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Atomic-child fallback misses shadow DOM descendants.

findAtomicChildAtPoint only walks light DOM children. If the meaningful descendant is in a shadow root, the fallback won’t find it.

Apply this diff:

- if (!this.isMeaningfulElementCached(deepestElement)) { - const atomicChild = this.findAtomicChildAtPoint(deepestElement, x, y); + if (!this.isMeaningfulElementCached(deepestElement)) { + const atomicChild = this.findAtomicChildAtPoint(deepestElement, x, y); if (atomicChild) { return atomicChild; } }

And enhance traversal:

 private findAtomicChildAtPoint( parent: HTMLElement, x: number, y: number ): HTMLElement | null { const stack: HTMLElement[] = [parent]; const visited = new Set<HTMLElement>(); while (stack.length > 0) { const element = stack.pop()!; if (visited.has(element)) continue; visited.add(element); if (element !== parent && this.isMeaningfulElementCached(element)) { const rect = element.getBoundingClientRect(); if (x >= rect.left && x <= rect.right && y >= rect.top && y <= rect.bottom) { return element; } } - for (let i = element.children.length - 1; i >= 0; i--) { + // Light DOM children (iterate from topmost in stacking order approximation) + for (let i = element.children.length - 1; i >= 0; i--) { const child = element.children[i] as HTMLElement; const rect = child.getBoundingClientRect(); if (x >= rect.left && x <= rect.right && y >= rect.top && y <= rect.bottom) { stack.push(child); } } + // Shadow DOM children (if any) + if ((element as HTMLElement).shadowRoot) { + const shadowChildren = (element as HTMLElement).shadowRoot!.children; + for (let i = shadowChildren.length - 1; i >= 0; i--) { + const child = shadowChildren[i] as HTMLElement; + const rect = child.getBoundingClientRect(); + if (x >= rect.left && x <= rect.right && y >= rect.top && y <= rect.bottom) { + stack.push(child); + } + } + } } return null; }

Also applies to: 4003-4034

🤖 Prompt for AI Agents 
In src/helpers/clientSelectorGenerator.ts around lines 3993-3999 (and similarly for 4003-4034), the atomic-child fallback uses findAtomicChildAtPoint which only traverses light DOM children and therefore misses meaningful descendants inside shadow roots; update the traversal to also check shadow DOM by: when examining a candidate element at the point, if it has a shadowRoot call shadowRoot.elementFromPoint(x,y) (or shadowRoot.deep element lookup) and continue descending into that shadow root’s composed tree (including following assigned nodes for slots and checking shadow-inserted children) before falling back to light DOM children; ensure the function returns the deepest atomic descendant found in either the shadowRoot or light children and mirror this shadow-aware logic in the other affected block (4003-4034).
return deepestElement;
}

private findAtomicChildAtPoint(
parent: HTMLElement,
x: number,
y: number
): HTMLElement | null {
const stack: HTMLElement[] = [parent];
const visited = new Set<HTMLElement>();

while (stack.length > 0) {
const element = stack.pop()!;
if (visited.has(element)) continue;
visited.add(element);

if (element !== parent && this.isMeaningfulElementCached(element)) {
const rect = element.getBoundingClientRect();
if (x >= rect.left && x <= rect.right && y >= rect.top && y <= rect.bottom) {
return element;
}
}

for (let i = element.children.length - 1; i >= 0; i--) {
const child = element.children[i] as HTMLElement;
const rect = child.getBoundingClientRect();
if (x >= rect.left && x <= rect.right && y >= rect.top && y <= rect.bottom) {
stack.push(child);
}
}
}

return null;
}

/**
* Helper methods used by the unified getDeepestElementFromPoint
*/
Expand Down