Skip to content

Commit 3d4974a

Browse files
committed
fix(driver): stop aria_click reporting success when nothing was clicked
aria_click activated elements with DOM.focus + Enter/Space and returned True whenever the CDP calls did not raise. That is a silent lie on any widget whose handler listens for `click` rather than a key event — React/Vue components bound to a div, custom comboboxes, label-wrapped visually hidden radios. The caller sees ok:true, the page never changed, and the failure only surfaces much later as inexplicably wrong state. Arm a one-shot click listener on the element, try the keyboard path, then check whether a click event actually fired. If it did not, fall back to el.click() and re-check. The fallback dispatches no pointer coordinates, so the module's zero-mouse-telemetry guarantee is intact. ok:false now means the element genuinely did not activate. Add the pieces that were missing around it, each one something a caller otherwise hand-rolls in `evaluate`: - set_field / set_fields: type-aware, framework-safe value setting. Writes through the native prototype setter and fires input+change, which is what React's controlled inputs require; plain `el.value =` is swallowed. Handles <select> by option value OR visible text, checkbox/radio booleans, contenteditable, and every input type. set_fields applies a whole map in one JS pass — a ten-field form becomes one round-trip instead of ten. - element_rect: ARIA idx → viewport-correct {cx,cy}, scrolled into view, so idx-based discovery can feed the coordinate tools directly. - find_all_by_text: ranked candidates instead of find_by_text's single silent pick, for pages with several identically-labelled controls. Screenshots now save to disk and return a path, with base64 opt-in via include_b64. A full-page capture is routinely 100k+ base64 characters, enough to exhaust an agent's context in one tool result for an image it must read back anyway. Documents that only viewport captures map 1:1 onto click_at. list_tabs reports alive:false with a reason instead of url:"?" when Chrome is gone, and prunes the dead entries so later calls fail with a clear message rather than a bare ConnectionRefusedError. Adds _is_dead_connection to tell a dead browser apart from a page that merely misbehaved. Surface batching in the tool guide. The batch tool already existed but nothing pointed at it from the single-action tools, so the obvious path was N sequential calls. Tests cover the DOM behaviour that motivated the change (native setter events, select-by-text, partial-failure reporting) plus dead-connection detection. The two test_fingerprint failures are pre-existing and reproduce on main.
1 parent 27d7d7c commit 3d4974a

4 files changed

Lines changed: 681 additions & 28 deletions

File tree

‎src/umbra/driver/aria.py‎

Lines changed: 183 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from __future__ import annotations
1919

2020
import asyncio
21+
import contextlib
2122
import logging
2223
from dataclasses import dataclass
2324
from typing import Any
@@ -224,18 +225,67 @@ async def _resolve_node(self, idx: int) -> Any:
224225
))
225226
return result
226227

227-
async def click(self, idx: int) -> bool:
228-
"""Activate element by DOM.focus + keyboard. Zero mouse events.
228+
async def _object_id(self, idx: int) -> str | None:
229+
"""RemoteObjectId for the DOM node behind an AX index, or None."""
230+
node = self._index.get(idx)
231+
if not node or not node.backend_node_id:
232+
return None
233+
try:
234+
res = await self.tab.send(self._cdp.dom.resolve_node(
235+
backend_node_id=self._cdp.dom.BackendNodeId(node.backend_node_id),
236+
))
237+
except Exception as e: # noqa: BLE001
238+
log.debug("resolve_node failed at idx=%d: %s", idx, e)
239+
return None
240+
# nodriver returns a RemoteObject (or a (RemoteObject, exc) tuple).
241+
obj = res[0] if isinstance(res, tuple) else res
242+
return getattr(obj, "object_id", None)
243+
244+
async def _call_on(self, object_id: str, fn: str) -> Any:
245+
"""Runtime.callFunctionOn(fn, object_id) → plain Python value."""
246+
res = await self.tab.send(self._cdp.runtime.call_function_on(
247+
function_declaration=fn, object_id=object_id, return_by_value=True,
248+
))
249+
obj = res[0] if isinstance(res, tuple) else res
250+
return getattr(obj, "value", None)
229251

230-
Direct CDP `DOM.focus(backend_node_id=…)` — no Runtime.callFunctionOn
231-
round-trip (was buggy w/ object_id type wrapping). Press Enter for
232-
most roles, Space for checkbox/radio/switch.
252+
async def click(self, idx: int) -> bool:
253+
"""Activate element, verifying the activation actually landed.
254+
255+
Zero mouse telemetry either way — the fallback is `el.click()`, which
256+
dispatches a trusted-shaped click event without emitting any pointer
257+
coordinates, so behavioral fingerprinting still sees nothing.
258+
259+
Why the verification exists: `DOM.focus` + Enter/Space activates native
260+
controls, but a large class of real-world widgets (React/Vue components
261+
that bind onClick to a div, custom comboboxes, label-wrapped visually
262+
hidden radios) ignore synthetic key events entirely. The old
263+
implementation returned True in exactly that case — reporting success
264+
while the page never changed, which is worse than failing loudly.
265+
266+
Strategy: arm a one-shot click listener on the element, try the
267+
keyboard path, then check whether a click event actually fired. If it
268+
did not, call `el.click()` directly and re-check.
233269
"""
234270
node = self._index.get(idx)
235271
if not node or not node.backend_node_id:
236272
return False
237273
is_toggle = node.role in {"checkbox", "radio", "switch"}
238274
key = "Space" if is_toggle else "Enter"
275+
276+
object_id = await self._object_id(idx)
277+
278+
# Arm the probe. If we can't resolve the node, fall through to the
279+
# legacy keyboard-only path rather than failing the call outright.
280+
if object_id:
281+
with contextlib.suppress(Exception):
282+
await self._call_on(object_id, """function() {
283+
this.__umbraClicked = false;
284+
this.__umbraProbe = () => { this.__umbraClicked = true; };
285+
this.addEventListener('click', this.__umbraProbe, {capture: true});
286+
return true;
287+
}""")
288+
239289
try:
240290
await self.tab.send(self._cdp.dom.focus(
241291
backend_node_id=self._cdp.dom.BackendNodeId(node.backend_node_id),
@@ -246,10 +296,135 @@ async def click(self, idx: int) -> bool:
246296
await self.tab.send(self._cdp.input_.dispatch_key_event(
247297
type_="keyUp", key=key, code=key,
248298
))
249-
return True
299+
keyboard_ok = True
250300
except Exception as e: # noqa: BLE001
251-
log.warning("aria click failed at idx=%d: %s", idx, e)
252-
return False
301+
log.debug("aria keyboard activation failed at idx=%d: %s", idx, e)
302+
keyboard_ok = False
303+
304+
if not object_id:
305+
# No probe available — preserve old behavior.
306+
return keyboard_ok
307+
308+
fired = await self._call_on(object_id, "function() { return !!this.__umbraClicked; }")
309+
310+
if not fired:
311+
# Keyboard did nothing. Escalate to a direct DOM activation, which
312+
# is what custom widget handlers actually listen for.
313+
with contextlib.suppress(Exception):
314+
await self._call_on(object_id, "function() { this.click(); return true; }")
315+
fired = await self._call_on(
316+
object_id, "function() { return !!this.__umbraClicked; }")
317+
if fired:
318+
log.debug("aria click idx=%d needed el.click() fallback", idx)
319+
320+
with contextlib.suppress(Exception):
321+
await self._call_on(object_id, """function() {
322+
if (this.__umbraProbe)
323+
this.removeEventListener('click', this.__umbraProbe, {capture: true});
324+
delete this.__umbraProbe; delete this.__umbraClicked;
325+
return true;
326+
}""")
327+
328+
if not fired:
329+
log.warning(
330+
"aria click at idx=%d did not activate the element "
331+
"(neither key press nor el.click() produced a click event)", idx)
332+
return bool(fired)
333+
334+
async def rect(self, idx: int, *, scroll_into_view: bool = True) -> dict[str, Any] | None:
335+
"""Viewport rect + center point for an ARIA index.
336+
337+
Scrolls the element into view first so the coordinates are actually
338+
clickable — an off-screen element has a rect, but clicking it lands on
339+
whatever happens to occupy that spot. Returns None if the element is
340+
not resolvable or has no box (display:none, detached).
341+
"""
342+
object_id = await self._object_id(idx)
343+
if not object_id:
344+
return None
345+
fn = """function() {
346+
%s
347+
const r = this.getBoundingClientRect();
348+
if (!r.width && !r.height) return null;
349+
return {x: r.x, y: r.y, w: r.width, h: r.height,
350+
cx: Math.round(r.x + r.width / 2),
351+
cy: Math.round(r.y + r.height / 2)};
352+
}""" % ("this.scrollIntoView({block: 'center', inline: 'center'});"
353+
if scroll_into_view else "")
354+
try:
355+
return await self._call_on(object_id, fn)
356+
except Exception as e: # noqa: BLE001
357+
log.debug("rect failed at idx=%d: %s", idx, e)
358+
return None
359+
360+
async def selector_for(self, idx: int) -> str | None:
361+
"""Build a unique CSS selector for the element behind an ARIA index.
362+
363+
Lets index-based discovery feed selector-based tools (`set_field`)
364+
without the caller hand-writing a selector and hoping it matches the
365+
same element the snapshot showed them.
366+
"""
367+
object_id = await self._object_id(idx)
368+
if not object_id:
369+
return None
370+
fn = """function() {
371+
const esc = s => (window.CSS && CSS.escape) ? CSS.escape(s) : s;
372+
if (this.id) return '#' + esc(this.id);
373+
if (this.name && this.form)
374+
return this.tagName.toLowerCase() + '[name="' + this.name + '"]';
375+
const path = [];
376+
let el = this;
377+
while (el && el.nodeType === 1 && path.length < 6) {
378+
let part = el.tagName.toLowerCase();
379+
if (el.id) { path.unshift('#' + esc(el.id)); break; }
380+
const parent = el.parentElement;
381+
if (parent) {
382+
const sibs = Array.from(parent.children)
383+
.filter(c => c.tagName === el.tagName);
384+
if (sibs.length > 1)
385+
part += ':nth-of-type(' + (sibs.indexOf(el) + 1) + ')';
386+
}
387+
path.unshift(part);
388+
el = el.parentElement;
389+
}
390+
return path.join(' > ');
391+
}"""
392+
try:
393+
return await self._call_on(object_id, fn)
394+
except Exception as e: # noqa: BLE001
395+
log.debug("selector_for failed at idx=%d: %s", idx, e)
396+
return None
397+
398+
async def find_all_by_text(self, text: str, *, role_hint: str | None = None,
399+
limit: int = 20) -> list[dict[str, Any]]:
400+
"""Every element matching `text`, best first — not just the top hit.
401+
402+
`find_by_text` collapses to a single index, which silently picks for you
403+
when a page has several plausible matches (three "+ Add" buttons, say).
404+
This returns the candidates so the caller can disambiguate by role or
405+
surrounding name instead of guessing.
406+
"""
407+
await self.snapshot()
408+
target = text.lower().strip()
409+
scored: list[tuple[int, dict[str, Any]]] = []
410+
for n in self._index.values():
411+
if role_hint and n.role != role_hint:
412+
continue
413+
for hay in (n.name, n.value or ""):
414+
if not hay:
415+
continue
416+
hl = hay.lower()
417+
if target == hl:
418+
score = 1000
419+
elif target in hl:
420+
score = 100 - abs(len(hl) - len(target))
421+
else:
422+
continue
423+
scored.append((score, {"idx": n.idx, "role": n.role,
424+
"name": n.name[:80], "score": score}))
425+
break
426+
scored.sort(key=lambda t: -t[0])
427+
return [d for _, d in scored[:limit]]
253428

254429
async def type(self, idx: int, text: str, *, clear: bool = True, jitter: bool = True) -> bool:
255430
"""Focus, optionally clear, then type with humanized log-normal delays."""

‎src/umbra/driver/utils.py‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import asyncio
1212
import base64
13+
import json
1314
import logging
1415
import re
1516
from typing import Any
@@ -136,6 +137,144 @@ async def select_option(tab: Any, selector: str, value: str) -> bool:
136137
return bool(result)
137138

138139

140+
_SET_FIELD_JS = r"""
141+
(function(sel, raw) {
142+
const el = document.querySelector(sel);
143+
if (!el) return {ok: false, why: 'no element matches selector'};
144+
145+
// React/Vue/Angular track the previous value on the DOM node and swallow
146+
// any change where node.value was reassigned directly. Going through the
147+
// prototype's native setter defeats that, which is why a plain `el.value =`
148+
// silently no-ops on framework-controlled inputs.
149+
const nativeSet = (node, prop, v) => {
150+
const proto = Object.getPrototypeOf(node);
151+
const desc = Object.getOwnPropertyDescriptor(proto, prop)
152+
|| Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, prop);
153+
if (desc && desc.set) desc.set.call(node, v);
154+
else node[prop] = v;
155+
};
156+
const fire = (node, types) => {
157+
for (const t of types)
158+
node.dispatchEvent(new Event(t, {bubbles: true}));
159+
};
160+
161+
const tag = el.tagName.toUpperCase();
162+
const type = (el.type || '').toLowerCase();
163+
const truthy = v => v === true || v === 'true' || v === 1 || v === '1'
164+
|| v === 'on' || v === 'yes' || v === 'checked';
165+
166+
if (tag === 'SELECT') {
167+
// Accept the option's value OR its visible text, like a human would.
168+
const want = String(raw);
169+
const opts = Array.from(el.options);
170+
const hit = opts.find(o => o.value === want)
171+
|| opts.find(o => o.text.trim() === want.trim())
172+
|| opts.find(o => o.text.trim().toLowerCase() === want.trim().toLowerCase())
173+
|| opts.find(o => o.text.trim().toLowerCase().includes(want.trim().toLowerCase()));
174+
if (!hit) return {ok: false, why: 'no matching option',
175+
options: opts.slice(0, 25).map(o => o.text.trim())};
176+
nativeSet(el, 'value', hit.value);
177+
fire(el, ['input', 'change']);
178+
return {ok: true, kind: 'select', value: el.value, text: hit.text.trim()};
179+
}
180+
181+
if (type === 'checkbox' || type === 'radio') {
182+
const want = truthy(raw);
183+
const proto = Object.getPrototypeOf(el);
184+
const desc = Object.getOwnPropertyDescriptor(proto, 'checked')
185+
|| Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'checked');
186+
if (el.checked !== want) {
187+
if (desc && desc.set) desc.set.call(el, want); else el.checked = want;
188+
fire(el, ['click', 'input', 'change']);
189+
}
190+
return {ok: el.checked === want, kind: type, checked: el.checked};
191+
}
192+
193+
if (el.isContentEditable) {
194+
el.focus();
195+
el.textContent = String(raw);
196+
fire(el, ['input', 'change']);
197+
return {ok: true, kind: 'contenteditable', value: el.textContent};
198+
}
199+
200+
if (tag === 'INPUT' || tag === 'TEXTAREA') {
201+
nativeSet(el, 'value', String(raw));
202+
fire(el, ['input', 'change']);
203+
return {ok: el.value === String(raw), kind: type || tag.toLowerCase(), value: el.value};
204+
}
205+
206+
return {ok: false, why: 'element is not a form field', tag: tag};
207+
})
208+
"""
209+
210+
211+
def _json_result(raw: Any) -> dict[str, Any]:
212+
"""Parse a JSON.stringify'd evaluate() result into a plain dict.
213+
214+
`tab.evaluate` hands back nodriver's CDP-serialized shape (nested
215+
[key, {type, value}] pairs) unless the payload is already a string, so the
216+
JS side stringifies and we parse here — the same round-trip
217+
`AriaDriver.current_state` uses.
218+
"""
219+
if isinstance(raw, str):
220+
try:
221+
parsed = json.loads(raw)
222+
except (ValueError, TypeError):
223+
return {"ok": False, "why": "result was not valid JSON", "raw": raw[:200]}
224+
if isinstance(parsed, dict):
225+
return parsed
226+
return {"ok": False, "why": "result was not an object", "raw": parsed}
227+
return {"ok": False, "why": "evaluation returned no result", "raw": repr(raw)[:200]}
228+
229+
230+
async def set_field(tab: Any, selector: str, value: Any) -> dict[str, Any]:
231+
"""Set any form field's value in one call, framework-safe.
232+
233+
Handles `<select>` (matched by option value *or* visible text), checkboxes
234+
and radios (accepts booleans or truthy strings), contenteditable regions,
235+
and every `<input>`/`<textarea>` type — always writing through the native
236+
prototype setter and firing input+change so React/Vue/Angular actually
237+
register the change.
238+
239+
This exists because `aria_type` drives real keystrokes, which cannot set a
240+
checkbox, cannot pick a `<select>` option, and gets mangled by masked or
241+
otherwise controlled inputs. Reach for this when you want a *value* set;
242+
reach for `aria_type` when you want the keystrokes themselves observed.
243+
"""
244+
raw = await tab.evaluate(
245+
f"JSON.stringify(({_SET_FIELD_JS})({selector!r}, {json.dumps(value)}))")
246+
return _json_result(raw)
247+
248+
249+
async def set_fields(tab: Any, fields: dict[str, Any]) -> dict[str, Any]:
250+
"""Set many form fields in ONE round-trip. Same semantics as `set_field`.
251+
252+
`fields` maps CSS selector → value. A ten-field form is one evaluate call
253+
instead of ten, which matters more than it looks: each MCP round-trip is
254+
framing + scheduling overhead, and interleaving them with re-snapshots is
255+
how a form that should take two seconds takes thirty.
256+
257+
Returns {selector: result} plus rolled-up ok/failed lists.
258+
"""
259+
payload = json.dumps(fields)
260+
script = f"""JSON.stringify((() => {{
261+
const setOne = {_SET_FIELD_JS};
262+
const fields = {payload};
263+
const out = {{}};
264+
for (const [sel, val] of Object.entries(fields)) {{
265+
try {{ out[sel] = setOne(sel, val); }}
266+
catch (e) {{ out[sel] = {{ok: false, why: String(e)}}; }}
267+
}}
268+
return out;
269+
}})())"""
270+
result = _json_result(await tab.evaluate(script))
271+
if result.get("why"):
272+
return result
273+
ok = [s for s, r in result.items() if isinstance(r, dict) and r.get("ok")]
274+
failed = [s for s, r in result.items() if not (isinstance(r, dict) and r.get("ok"))]
275+
return {"ok": not failed, "set": ok, "failed": failed, "results": result}
276+
277+
139278
async def get_response_body(tab: Any, request_id: str) -> dict[str, Any]:
140279
"""Pull the full HTTP response body for a given request_id (from get_network_requests).
141280

0 commit comments

Comments
 (0)