Skip to content

Commit 510ec0f

Browse files
authored
fix(input): stop native select option clicks from timing out (#1960)
## Summary Teach the `click` tool to handle accessibility snapshot targets that point at an `option` inside a native, single-select `<select>` element. When the target is a native select option, `click` now selects that option through the owning `<select>` element instead of asking Puppeteer to click an option node that has no clickable box while the dropdown is collapsed. Other option-like targets, including custom ARIA options, continue through the normal click path. ## Motivation Fixes #1941. The text snapshot can expose native `<option>` nodes with stable uids. This makes it natural for an agent to call `click` on an option uid after seeing the desired option in the snapshot. However, a collapsed native `<select>` does not expose its child `<option>` nodes as directly clickable page boxes. In that state, the option can exist in the DOM and accessibility tree while still having no clickable geometry. Puppeteer therefore waits for the option to become interactive and eventually times out. The existing `fill` tool already handles selecting native `<select>` options. This change keeps that guidance explicit in the `click` tool description, while also making `click(option_uid)` robust for the native select case that appears in the snapshot. ## Changes - Added a constrained native select fallback in `click`: - Only runs for single-click requests. - Only runs when the snapshot node role is `option`. - Only handles real `HTMLOptionElement` nodes owned by a native `<select>`. - Does not handle disabled selects, disabled options, disabled optgroups, or multi-selects. - Falls back to the existing locator click behavior for all other targets. - Dispatches `input` and `change` events when selecting a different native option. - Updated the `click` tool description to steer agents toward `fill` for native `<select>` option selection. - Regenerated CLI/tool reference docs with `npm run gen`. - Added regression coverage for: - Clicking an option uid in a collapsed native `<select>`. - Clicking an option uid inside a native `<optgroup>`. - Clicking a custom ARIA `role="option"` element through the normal click path. ## Test Plan Passed: ```bash npm run test -- tests/tools/input.test.ts npm run check-format ``` Also attempted: ```bash npm run test ``` The full suite hit unrelated local failures outside this change area: - `tests/tools/network.test.ts`: snapshot ordering difference for redirected requests. - `tests/tools/screenshot.test.ts`: `Page.captureScreenshot` failed for the large full-page screenshot case with `Page is too large`. The changed input tool tests pass locally. ## Risk Low. The fallback is intentionally narrow: - It is gated by the accessibility role being `option`. - It verifies the DOM node is an actual `HTMLOptionElement`. - It only applies to native, non-disabled, single-select `<select>` controls. - It does not reinterpret custom combobox/listbox implementations. - It preserves the existing locator click path for non-native option targets. The main behavior change is that `click(option_uid)` can now succeed for native collapsed dropdowns that previously timed out. ## Related Issue Closes #1941. ## Maintainer Context This targets a small but high-impact mismatch between the snapshot representation and browser interaction semantics. The snapshot correctly exposes native options because they exist in the accessibility tree. The click implementation previously treated that uid like any other clickable element, but collapsed native options do not have normal clickable layout boxes. This PR makes the native select case explicit without expanding `click` into a general custom dropdown heuristic. The tool description still recommends `fill` for native `<select>` selection so agents are guided toward the more direct tool. The code fallback exists for the common case where an agent already selected an option uid from the snapshot.
1 parent 2dcd641 commit 510ec0f

2 files changed

Lines changed: 192 additions & 0 deletions

File tree

src/tools/input.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,49 @@ function handleActionError(error: unknown, uid: string) {
4242
);
4343
}
4444

45+
async function selectNativeSelectOption(handle: ElementHandle<Element>) {
46+
const selectHandle = await handle.evaluateHandle(node => {
47+
if (!(node instanceof HTMLOptionElement)) {
48+
return null;
49+
}
50+
51+
const select = node.closest('select');
52+
if (!select || select.multiple || select.disabled || node.disabled) {
53+
return null;
54+
}
55+
56+
const parentElement = node.parentElement;
57+
if (
58+
parentElement instanceof HTMLOptGroupElement &&
59+
parentElement.disabled
60+
) {
61+
return null;
62+
}
63+
64+
return select;
65+
});
66+
try {
67+
const select = selectHandle.asElement() as ElementHandle<Element> | null;
68+
if (!select) {
69+
return false;
70+
}
71+
72+
const valueHandle = await handle.getProperty('value');
73+
try {
74+
const value = await valueHandle.jsonValue();
75+
if (typeof value !== 'string') {
76+
return false;
77+
}
78+
await select.asLocator().fill(value);
79+
} finally {
80+
void valueHandle.dispose();
81+
}
82+
return true;
83+
} finally {
84+
void selectHandle.dispose();
85+
}
86+
}
87+
4588
export const click = definePageTool({
4689
name: 'click',
4790
description: `Clicks on the provided element`,
@@ -62,8 +105,18 @@ export const click = definePageTool({
62105
handler: async (request, response) => {
63106
const uid = request.params.uid;
64107
const handle = await request.page.getElementByUid(uid);
108+
const aXNode = request.page.getAXNodeByUid(uid);
109+
const shouldSelectNativeOption =
110+
!request.params.dblClick && aXNode?.role === 'option';
65111
try {
66112
await request.page.waitForEventsAfterAction(async () => {
113+
if (
114+
shouldSelectNativeOption &&
115+
(await selectNativeSelectOption(handle))
116+
) {
117+
return;
118+
}
119+
67120
await handle.asLocator().click({
68121
count: request.params.dblClick ? 2 : 1,
69122
});

tests/tools/input.test.ts

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,145 @@ describe('input', () => {
226226
assert.notStrictEqual(response.snapshotParams, undefined);
227227
});
228228
});
229+
230+
it('selects a collapsed native select option by option uid', async () => {
231+
await withMcpContext(async (response, context) => {
232+
const page = context.getSelectedPptrPage();
233+
await page.setContent(
234+
html`<select onchange="document.body.dataset.selected = this.value">
235+
<option value="v1">one</option>
236+
<option value="v2">two</option>
237+
</select>`,
238+
);
239+
const mcpPage = context.getSelectedMcpPage();
240+
mcpPage.textSnapshot = await TextSnapshot.create(mcpPage);
241+
const optionNode = [...mcpPage.textSnapshot.idToNode.values()].find(
242+
node => node.role === 'option' && node.name === 'two',
243+
);
244+
assert.ok(optionNode);
245+
246+
await click.handler(
247+
{
248+
params: {
249+
uid: optionNode.id,
250+
},
251+
page: mcpPage,
252+
},
253+
response,
254+
context,
255+
);
256+
257+
assert.strictEqual(
258+
response.responseLines[0],
259+
'Successfully clicked on the element',
260+
);
261+
assert.deepStrictEqual(
262+
await page.evaluate(() => {
263+
const select = document.querySelector('select');
264+
return {
265+
selectedValue: select?.value,
266+
changeEventValue: document.body.dataset.selected,
267+
};
268+
}),
269+
{
270+
selectedValue: 'v2',
271+
changeEventValue: 'v2',
272+
},
273+
);
274+
});
275+
});
276+
277+
it('selects a collapsed native optgroup option by option uid', async () => {
278+
await withMcpContext(async (response, context) => {
279+
const page = context.getSelectedPptrPage();
280+
await page.setContent(
281+
html`<select onchange="document.body.dataset.selected = this.value">
282+
<optgroup label="Numbers">
283+
<option value="v1">one</option>
284+
<option value="v2">two</option>
285+
</optgroup>
286+
</select>`,
287+
);
288+
const mcpPage = context.getSelectedMcpPage();
289+
mcpPage.textSnapshot = await TextSnapshot.create(mcpPage);
290+
const optionNode = [...mcpPage.textSnapshot.idToNode.values()].find(
291+
node => node.role === 'option' && node.name === 'two',
292+
);
293+
assert.ok(optionNode);
294+
295+
await click.handler(
296+
{
297+
params: {
298+
uid: optionNode.id,
299+
},
300+
page: mcpPage,
301+
},
302+
response,
303+
context,
304+
);
305+
306+
assert.strictEqual(
307+
response.responseLines[0],
308+
'Successfully clicked on the element',
309+
);
310+
assert.deepStrictEqual(
311+
await page.evaluate(() => {
312+
const select = document.querySelector('select');
313+
return {
314+
selectedValue: select?.value,
315+
changeEventValue: document.body.dataset.selected,
316+
};
317+
}),
318+
{
319+
selectedValue: 'v2',
320+
changeEventValue: 'v2',
321+
},
322+
);
323+
});
324+
});
325+
326+
it('clicks custom ARIA option elements through the normal click path', async () => {
327+
await withMcpContext(async (response, context) => {
328+
const page = context.getSelectedPptrPage();
329+
await page.setContent(
330+
html`<div role="listbox">
331+
<div
332+
role="option"
333+
tabindex="0"
334+
onclick="document.body.dataset.clicked = this.textContent.trim()"
335+
>
336+
custom two
337+
</div>
338+
</div>`,
339+
);
340+
const mcpPage = context.getSelectedMcpPage();
341+
mcpPage.textSnapshot = await TextSnapshot.create(mcpPage);
342+
const optionNode = [...mcpPage.textSnapshot.idToNode.values()].find(
343+
node => node.role === 'option' && node.name === 'custom two',
344+
);
345+
assert.ok(optionNode);
346+
347+
await click.handler(
348+
{
349+
params: {
350+
uid: optionNode.id,
351+
},
352+
page: mcpPage,
353+
},
354+
response,
355+
context,
356+
);
357+
358+
assert.strictEqual(
359+
response.responseLines[0],
360+
'Successfully clicked on the element',
361+
);
362+
assert.strictEqual(
363+
await page.evaluate(() => document.body.dataset.clicked),
364+
'custom two',
365+
);
366+
});
367+
});
229368
});
230369

231370
describe('hover', () => {

0 commit comments

Comments
 (0)