fix: resolve snapshot -C and screenshot --annotate hang over WSS (#842)

* fix: resolve snapshot -C and screenshot --annotate hang over WSS (#841)

Root cause: sequential CDP round-trips per element in
find_cursor_interactive_elements() and collect_annotations() caused
timeouts over high-latency WSS connections (~200ms × 200+ elements
exceeds the 30s CDP timeout).

Fix:
- snapshot -C: Replace per-element CDP calls with a single JS eval
  that detects cursor:pointer/onclick/tabindex elements in-browser,
  then batch-resolve via DOM.querySelectorAll + concurrent
  DOM.describeNode calls using join_all
- screenshot --annotate: Replace sequential DOM.resolveNode +
  getRect calls with concurrent join_all, matching v0.19.0's
  Promise.all() pattern

Behavioral parity with v0.19.0 (Node.js/Playwright):
- cursor:pointer detection via getComputedStyle
- Inherited cursor:pointer dedup (skip children of pointer parents)
- interactiveTags and interactive ARIA roles exclusion
- Role differentiation: clickable vs focusable
- Text dedup against ARIA tree ref names and quoted strings
- Edge case: -i -C shows cursor elements even when ARIA tree is empty

Tests:
- 5 unit tests for build_dedup_set() helper
- 3 e2e regression tests: cursor-interactive detection, annotation
  scaling to 50 elements, cursor scaling to 100 elements

* fix: add hidden/aria-hidden filtering, contentEditable support, and cleanup robustness

- Restore hidden/aria-hidden element filtering in cursor-interactive JS
  (was present in old code, dropped during rewrite)
- Add contentEditable detection with 'editable' role and hint
- Replace fire-and-forget cleanup with warning on failure
- Simplify build_dedup_set to use ref_map only (eliminates fragile
  tree-text quote parsing; ref_map already has all ref-bearing names)

---------

Co-authored-by: ctate <366502+ctate@users.noreply.github.com>
This commit is contained in:
Chris Tate
2026-03-15 20:41:30 -05:00
committed by GitHub
co-authored by ctate
parent 5b74604a5c
commit 285eab46df
3 changed files with 590 additions and 105 deletions
+292 -88
View File
@@ -4,8 +4,7 @@ use serde_json::Value;
use super::cdp::client::CdpClient;
use super::cdp::types::{
AXNode, AXProperty, AXValue, CallFunctionOnParams, EvaluateParams, EvaluateResult,
GetFullAXTreeResult,
AXNode, AXProperty, AXValue, EvaluateParams, EvaluateResult, GetFullAXTreeResult,
};
use super::element::RefMap;
@@ -294,6 +293,22 @@ pub async fn take_snapshot(
}
let mut trimmed = output.trim().to_string();
let tree_is_empty = trimmed.is_empty();
if options.cursor {
let cursor_section = find_cursor_interactive_elements(client, session_id, ref_map).await?;
if !cursor_section.is_empty() {
// v0.19.0 parity: when interactive tree is empty but cursor elements exist,
// the cursor elements replace the empty message (no separator).
if tree_is_empty {
trimmed = cursor_section;
} else {
trimmed.push_str("\n# Cursor-interactive elements:\n");
trimmed.push_str(&cursor_section);
}
}
}
if trimmed.is_empty() {
if options.interactive {
return Ok("(no interactive elements)".to_string());
@@ -301,14 +316,6 @@ pub async fn take_snapshot(
return Ok("(empty page)".to_string());
}
if options.cursor {
let cursor_section = find_cursor_interactive_elements(client, session_id, ref_map).await?;
if !cursor_section.is_empty() {
trimmed.push_str("\n# Cursor-interactive elements:\n");
trimmed.push_str(&cursor_section);
}
}
Ok(trimmed)
}
@@ -317,30 +324,74 @@ async fn find_cursor_interactive_elements(
session_id: &str,
ref_map: &mut RefMap,
) -> Result<String, String> {
// Single JS evaluation that matches the v0.19.0 Node.js findCursorInteractiveElements():
// - Uses querySelectorAll('*') to walk all elements
// - Checks getComputedStyle(el).cursor === 'pointer'
// - Checks onclick attribute/handler and tabindex
// - Skips interactiveTags (a, button, input, select, textarea, details, summary)
// - Skips elements with interactive ARIA roles
// - Deduplicates inherited cursor:pointer from parent
// - Skips empty text and zero-size elements
// - Tags each matched element with data-__ab-ci for batch backendNodeId resolution
let js = r#"
(function() {
const elements = [];
const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_ELEMENT);
let node;
while (node = walker.nextNode()) {
if (node.closest && node.closest('[hidden], [aria-hidden="true"]')) continue;
const explicitRole = node.getAttribute ? node.getAttribute('role') : null;
if (explicitRole) continue;
const tag = node.tagName ? node.tagName.toLowerCase() : '';
const hasClick = node.onclick || (node.attributes && node.attributes.getNamedItem('onclick'));
const tabindex = node.getAttribute ? node.getAttribute('tabindex') : null;
const contentEditable = node.getAttribute ? node.getAttribute('contenteditable') : null;
const isInherentlyClickable =
(tag === 'a' && node.href) || tag === 'button' ||
(tag === 'input' && ['submit','button','image','reset'].indexOf((node.type||'').toLowerCase()) >= 0) ||
tag === 'summary';
const isFocusable = tabindex !== null && parseInt(tabindex, 10) >= 0;
const isEditable = contentEditable === '' || contentEditable === 'true';
if (hasClick || isInherentlyClickable || isFocusable || isEditable) {
elements.push(node);
var results = [];
if (!document.body) return results;
var interactiveRoles = {
'button':1, 'link':1, 'textbox':1, 'checkbox':1, 'radio':1, 'combobox':1, 'listbox':1,
'menuitem':1, 'menuitemcheckbox':1, 'menuitemradio':1, 'option':1, 'searchbox':1,
'slider':1, 'spinbutton':1, 'switch':1, 'tab':1, 'treeitem':1
};
var interactiveTags = {
'a':1, 'button':1, 'input':1, 'select':1, 'textarea':1, 'details':1, 'summary':1
};
var allElements = document.body.querySelectorAll('*');
for (var i = 0; i < allElements.length; i++) {
var el = allElements[i];
if (el.closest && el.closest('[hidden], [aria-hidden="true"]')) continue;
var tagName = el.tagName.toLowerCase();
if (interactiveTags[tagName]) continue;
var role = el.getAttribute('role');
if (role && interactiveRoles[role.toLowerCase()]) continue;
var computedStyle = getComputedStyle(el);
var hasCursorPointer = computedStyle.cursor === 'pointer';
var hasOnClick = el.hasAttribute('onclick') || el.onclick !== null;
var tabIndex = el.getAttribute('tabindex');
var hasTabIndex = tabIndex !== null && tabIndex !== '-1';
var ce = el.getAttribute('contenteditable');
var isEditable = ce === '' || ce === 'true';
if (!hasCursorPointer && !hasOnClick && !hasTabIndex && !isEditable) continue;
// Skip elements that only inherit cursor:pointer from an ancestor
if (hasCursorPointer && !hasOnClick && !hasTabIndex && !isEditable) {
var parent = el.parentElement;
if (parent && getComputedStyle(parent).cursor === 'pointer') continue;
}
var text = (el.textContent || '').trim().slice(0, 100);
if (!text) continue;
var rect = el.getBoundingClientRect();
if (rect.width === 0 || rect.height === 0) continue;
el.setAttribute('data-__ab-ci', String(results.length));
results.push({
text: text,
tagName: tagName,
hasOnClick: hasOnClick,
hasCursorPointer: hasCursorPointer,
hasTabIndex: hasTabIndex,
isEditable: isEditable
});
}
return elements;
return results;
})()
"#;
@@ -349,91 +400,178 @@ async fn find_cursor_interactive_elements(
"Runtime.evaluate",
&EvaluateParams {
expression: js.to_string(),
return_by_value: Some(false),
return_by_value: Some(true),
await_promise: Some(false),
},
Some(session_id),
)
.await?;
let array_object_id = match result.result.object_id {
Some(id) => id,
None => return Ok(String::new()),
};
let elements: Vec<Value> = result
.result
.value
.and_then(|v| serde_json::from_value::<Vec<Value>>(v).ok())
.unwrap_or_default();
let props_result: Value = client
if elements.is_empty() {
return Ok(String::new());
}
let mut existing_texts = build_dedup_set(ref_map);
// Batch-resolve backendNodeIds: use DOM.getDocument to get the root nodeId,
// then DOM.querySelectorAll to get all tagged elements in a single call.
let doc: Value = client
.send_command(
"Runtime.getProperties",
Some(serde_json::json!({ "objectId": array_object_id })),
"DOM.getDocument",
Some(serde_json::json!({ "depth": 0 })),
Some(session_id),
)
.await?;
let empty: Vec<Value> = Vec::new();
let result_array = props_result
.get("result")
let root_node_id = doc
.get("root")
.and_then(|r| r.get("nodeId"))
.and_then(|v| v.as_i64())
.ok_or("DOM.getDocument did not return root nodeId")?;
let query_result: Value = client
.send_command(
"DOM.querySelectorAll",
Some(serde_json::json!({
"nodeId": root_node_id,
"selector": "[data-__ab-ci]"
})),
Some(session_id),
)
.await?;
let node_ids: Vec<i64> = query_result
.get("nodeIds")
.and_then(|v| v.as_array())
.unwrap_or(&empty);
.map(|arr| arr.iter().filter_map(|v| v.as_i64()).collect())
.unwrap_or_default();
let mut indexed: Vec<(usize, String)> = Vec::new();
for prop in result_array {
let name = prop.get("name").and_then(|v| v.as_str()).unwrap_or("");
if let Ok(idx) = name.parse::<usize>() {
if let Some(obj_id) = prop
.get("value")
.and_then(|v| v.get("objectId"))
.and_then(|v| v.as_str())
{
indexed.push((idx, obj_id.to_string()));
}
}
}
indexed.sort_by_key(|(idx, _)| *idx);
let element_object_ids: Vec<String> = indexed.into_iter().map(|(_, id)| id).collect();
let mut next_ref = ref_map.next_ref_num();
let mut lines: Vec<String> = Vec::new();
let get_text_js =
r#"function(){ return (this.innerText || this.textContent || '').trim().slice(0, 100) }"#;
for object_id in &element_object_ids {
let describe: Value = client
.send_command(
// Resolve backendNodeIds for each DOM node using concurrent CDP calls.
let describe_futures: Vec<_> = node_ids
.iter()
.map(|&node_id| {
client.send_command(
"DOM.describeNode",
Some(serde_json::json!({ "objectId": object_id })),
Some(serde_json::json!({ "nodeId": node_id })),
Some(session_id),
)
.await?;
})
.collect();
let backend_node_id = describe
let describe_results = futures_util::future::join_all(describe_futures).await;
// Build a map from data-__ab-ci index to backendNodeId.
let mut idx_to_backend: HashMap<usize, i64> = HashMap::new();
for desc in describe_results.into_iter().flatten() {
let backend_id = desc
.get("node")
.and_then(|n| n.get("backendNodeId"))
.and_then(|v| v.as_i64());
let ci_attr = desc
.get("node")
.and_then(|n| n.get("attributes"))
.and_then(|a| a.as_array())
.and_then(|attrs| {
// attributes is a flat array: [name, value, name, value, ...]
attrs
.iter()
.enumerate()
.find(|(_, v)| v.as_str() == Some("data-__ab-ci"))
.and_then(|(i, _)| attrs.get(i + 1))
.and_then(|v| v.as_str())
.and_then(|s| s.parse::<usize>().ok())
});
if let (Some(bid), Some(idx)) = (backend_id, ci_attr) {
idx_to_backend.insert(idx, bid);
}
}
let text_result: EvaluateResult = client
.send_command_typed(
"Runtime.callFunctionOn",
&CallFunctionOnParams {
function_declaration: get_text_js.to_string(),
object_id: Some(object_id.clone()),
arguments: None,
return_by_value: Some(true),
await_promise: Some(false),
},
Some(session_id),
)
.await?;
// Clean up the data attributes we injected for backendNodeId resolution.
let cleanup_js =
r#"(function(){ var els = document.querySelectorAll('[data-__ab-ci]'); for (var i = 0; i < els.length; i++) els[i].removeAttribute('data-__ab-ci'); return els.length; })()"#.to_string();
if let Err(e) = client
.send_command_typed::<EvaluateParams, EvaluateResult>(
"Runtime.evaluate",
&EvaluateParams {
expression: cleanup_js,
return_by_value: Some(true),
await_promise: Some(false),
},
Some(session_id),
)
.await
{
eprintln!("[agent-browser] Warning: failed to clean up data-__ab-ci attributes: {e}");
}
let text = text_result
.result
.value
.as_ref()
// Build refs and output lines with v0.19.0-compatible format.
let mut next_ref = ref_map.next_ref_num();
let mut lines: Vec<String> = Vec::new();
for (i, elem) in elements.iter().enumerate() {
let text = elem
.get("text")
.and_then(|v| v.as_str())
.unwrap_or("")
.trim()
.to_string();
let kind = "clickable";
// Text dedup: skip if this text already appears in the ARIA tree refs (v0.19.0 parity)
let text_lower = text.to_lowercase();
if existing_texts.contains(&text_lower) {
continue;
}
existing_texts.insert(text_lower);
let backend_node_id = idx_to_backend.get(&i).copied();
// Role differentiation: v0.19.0 uses 'clickable' for cursor:pointer or onclick,
// 'focusable' for tabindex-only elements.
let has_cursor_pointer = elem
.get("hasCursorPointer")
.and_then(|v| v.as_bool())
.unwrap_or(false);
let has_on_click = elem
.get("hasOnClick")
.and_then(|v| v.as_bool())
.unwrap_or(false);
let has_tab_index = elem
.get("hasTabIndex")
.and_then(|v| v.as_bool())
.unwrap_or(false);
let is_editable = elem
.get("isEditable")
.and_then(|v| v.as_bool())
.unwrap_or(false);
let kind = if has_cursor_pointer || has_on_click {
"clickable"
} else if is_editable {
"editable"
} else {
"focusable"
};
let mut hints: Vec<&str> = Vec::new();
if has_cursor_pointer {
hints.push("cursor:pointer");
}
if has_on_click {
hints.push("onclick");
}
if has_tab_index {
hints.push("tabindex");
}
if is_editable {
hints.push("contenteditable");
}
let ref_id = format!("e{}", next_ref);
next_ref += 1;
@@ -443,7 +581,15 @@ async fn find_cursor_interactive_elements(
.replace('\\', "\\\\")
.replace('"', "\\\"")
.replace(['\n', '\r'], " ");
lines.push(format!("[ref={}] ({}) \"{}\"", ref_id, kind, escaped));
// v0.19.0 output format: - clickable "text" [ref=eN] [cursor:pointer, onclick]
lines.push(format!(
"- {} \"{}\" [ref={}] [{}]",
kind,
escaped,
ref_id,
hints.join(", ")
));
}
ref_map.set_next_ref_num(next_ref);
@@ -763,6 +909,20 @@ fn extract_properties(props: &Option<Vec<AXProperty>>) -> NodeProperties {
(level, checked, expanded, selected, disabled, required)
}
/// Build the set of texts to de-duplicate cursor-interactive elements against.
///
/// All ref-bearing ARIA tree nodes have their names stored in `ref_map` during
/// tree construction, so the ref-map entries are the single source of truth.
/// This avoids fragile parsing of the rendered tree text.
fn build_dedup_set(ref_map: &RefMap) -> std::collections::HashSet<String> {
ref_map
.entries_sorted()
.into_iter()
.filter(|(_, entry)| !entry.name.is_empty())
.map(|(_, entry)| entry.name.to_lowercase())
.collect()
}
/// Recursively collect all `backendNodeId` values from a CDP DOM node tree
/// (as returned by `DOM.describeNode` with `depth: -1`).
fn collect_backend_node_ids(node: &Value, ids: &mut std::collections::HashSet<i64>) {
@@ -835,4 +995,48 @@ mod tests {
assert!(dups.contains_key("button:Submit"));
assert!(!dups.contains_key("button:Cancel"));
}
// -----------------------------------------------------------------------
// Cursor-interactive text dedup (Issue #841 regression guard)
// -----------------------------------------------------------------------
#[test]
fn test_dedup_set_from_ref_map_names() {
let mut ref_map = RefMap::new();
ref_map.add("e1".to_string(), Some(1), "link", "Example Link", None);
ref_map.add("e2".to_string(), Some(2), "button", "Submit", None);
let set = build_dedup_set(&ref_map);
assert!(set.contains("example link"));
assert!(set.contains("submit"));
assert!(!set.contains("other text"));
}
#[test]
fn test_dedup_set_case_insensitive() {
let mut ref_map = RefMap::new();
ref_map.add("e1".to_string(), Some(1), "button", "Submit Form", None);
let set = build_dedup_set(&ref_map);
assert!(set.contains("submit form"));
assert!(!set.contains("Submit Form"));
}
#[test]
fn test_dedup_set_empty_inputs() {
let ref_map = RefMap::new();
let set = build_dedup_set(&ref_map);
assert!(set.is_empty());
}
#[test]
fn test_dedup_set_skips_empty_names() {
let mut ref_map = RefMap::new();
ref_map.add("e1".to_string(), Some(1), "generic", "", None);
ref_map.add("e2".to_string(), Some(2), "button", "OK", None);
let set = build_dedup_set(&ref_map);
assert_eq!(set.len(), 1);
assert!(set.contains("ok"));
}
}