Skip to content

Commit b216072

Browse files
seanmcguire12seanmcguire12
authored
Improve reliability of radio button clicks (#448)
* rm commented code * handle clicks explicitly * click labels when possible * version bump in pkg lock * changeset
1 parent 16837ec commit b216072

File tree

2 files changed

+174
-124
lines changed

2 files changed

+174
-124
lines changed

Diff for: ‎.changeset/rich-pans-lay.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@browserbasehq/stagehand": patch
3+
---
4+
5+
improve handling of radio button clicks

Diff for: ‎lib/handlers/actHandler.ts

+169-124
Original file line numberDiff line numberDiff line change
@@ -346,11 +346,11 @@ export class StagehandActHandler {
346346

347347
throw new PlaywrightCommandException(e.message);
348348
}
349-
} else if (typeof locator[method as keyof typeof locator] === "function") {
350-
// Log current URL before action
349+
} else if (method === "click") {
350+
// Log the URL before clicking
351351
this.logger({
352352
category: "action",
353-
message: "page URL before action",
353+
message: "page URL before click",
354354
level: 2,
355355
auxiliary: {
356356
url: {
@@ -360,17 +360,59 @@ export class StagehandActHandler {
360360
},
361361
});
362362

363-
// Perform the action
363+
// if the element is a radio button, we should try to click the label instead
364364
try {
365-
await (
366-
locator[method as keyof Locator] as unknown as (
367-
...args: string[]
368-
) => Promise<void>
369-
)(...args.map((arg) => arg?.toString() || ""));
365+
const isRadio = await locator.evaluate((el) => {
366+
return el instanceof HTMLInputElement && el.type === "radio";
367+
});
368+
369+
const clickArg = args.length ? args[0] : undefined;
370+
371+
if (isRadio) {
372+
// if it’s a radio button, try to find a label to click
373+
const inputId = await locator.evaluate((el) => el.id);
374+
let labelLocator;
375+
376+
if (inputId) {
377+
// if the radio button has an ID, try label[for="thatId"]
378+
labelLocator = this.stagehandPage.page.locator(
379+
`label[for="${inputId}"]`,
380+
);
381+
}
382+
if (!labelLocator || (await labelLocator.count()) < 1) {
383+
// if no label was found or the label doesn’t exist, check if
384+
// there is an ancestor <label>
385+
labelLocator = this.stagehandPage.page
386+
.locator(`xpath=${xpath}/ancestor::label`)
387+
.first();
388+
}
389+
if ((await labelLocator.count()) < 1) {
390+
// if still no label, try checking for a following-sibling or preceding-sibling label
391+
labelLocator = locator
392+
.locator(`xpath=following-sibling::label`)
393+
.first();
394+
if ((await labelLocator.count()) < 1) {
395+
labelLocator = locator
396+
.locator(`xpath=preceding-sibling::label`)
397+
.first();
398+
}
399+
}
400+
if ((await labelLocator.count()) > 0) {
401+
// if we found a label, click it
402+
await labelLocator.click(clickArg);
403+
} else {
404+
// otherwise, just click the radio button itself
405+
await locator.click(clickArg);
406+
}
407+
} else {
408+
// here we just do a normal click if it's not a radio input
409+
const clickArg = args.length ? args[0] : undefined;
410+
await locator.click(clickArg);
411+
}
370412
} catch (e) {
371413
this.logger({
372414
category: "action",
373-
message: "error performing method",
415+
message: "error performing click",
374416
level: 1,
375417
auxiliary: {
376418
error: {
@@ -400,106 +442,150 @@ export class StagehandActHandler {
400442
}
401443

402444
// Handle navigation if a new page is opened
403-
if (method === "click") {
445+
this.logger({
446+
category: "action",
447+
message: "clicking element, checking for page navigation",
448+
level: 1,
449+
auxiliary: {
450+
xpath: {
451+
value: xpath,
452+
type: "string",
453+
},
454+
},
455+
});
456+
457+
// NAVIDNOTE: Should this happen before we wait for locator[method]?
458+
const newOpenedTab = await Promise.race([
459+
new Promise<Page | null>((resolve) => {
460+
// TODO: This is a hack to get the new page
461+
// We should find a better way to do this
462+
this.stagehandPage.context.once("page", (page) => resolve(page));
463+
setTimeout(() => resolve(null), 1_500);
464+
}),
465+
]);
466+
467+
this.logger({
468+
category: "action",
469+
message: "clicked element",
470+
level: 1,
471+
auxiliary: {
472+
newOpenedTab: {
473+
value: newOpenedTab ? "opened a new tab" : "no new tabs opened",
474+
type: "string",
475+
},
476+
},
477+
});
478+
479+
if (newOpenedTab) {
404480
this.logger({
405481
category: "action",
406-
message: "clicking element, checking for page navigation",
482+
message: "new page detected (new tab) with URL",
407483
level: 1,
408484
auxiliary: {
409-
xpath: {
410-
value: xpath,
485+
url: {
486+
value: newOpenedTab.url(),
411487
type: "string",
412488
},
413489
},
414490
});
415-
// try {
416-
// // Force-click here
417-
// await locator.click({ force: true });
418-
// } catch (e) {
419-
// // handle/log exception
420-
// throw new PlaywrightCommandException(e.message);
421-
// }
422-
423-
// NAVIDNOTE: Should this happen before we wait for locator[method]?
424-
const newOpenedTab = await Promise.race([
425-
new Promise<Page | null>((resolve) => {
426-
// TODO: This is a hack to get the new page
427-
// We should find a better way to do this
428-
this.stagehandPage.context.once("page", (page) => resolve(page));
429-
setTimeout(() => resolve(null), 1_500);
430-
}),
431-
]);
491+
await newOpenedTab.close();
492+
await this.stagehandPage.page.goto(newOpenedTab.url());
493+
await this.stagehandPage.page.waitForLoadState("domcontentloaded");
494+
await this.stagehandPage._waitForSettledDom(domSettleTimeoutMs);
495+
}
432496

497+
await Promise.race([
498+
this.stagehandPage.page.waitForLoadState("networkidle"),
499+
new Promise((resolve) => setTimeout(resolve, 5_000)),
500+
]).catch((e) => {
433501
this.logger({
434502
category: "action",
435-
message: "clicked element",
503+
message: "network idle timeout hit",
436504
level: 1,
437505
auxiliary: {
438-
newOpenedTab: {
439-
value: newOpenedTab ? "opened a new tab" : "no new tabs opened",
506+
trace: {
507+
value: e.stack,
508+
type: "string",
509+
},
510+
message: {
511+
value: e.message,
440512
type: "string",
441513
},
442514
},
443515
});
516+
});
444517

445-
if (newOpenedTab) {
446-
this.logger({
447-
category: "action",
448-
message: "new page detected (new tab) with URL",
449-
level: 1,
450-
auxiliary: {
451-
url: {
452-
value: newOpenedTab.url(),
453-
type: "string",
454-
},
455-
},
456-
});
457-
await newOpenedTab.close();
458-
await this.stagehandPage.page.goto(newOpenedTab.url());
459-
await this.stagehandPage.page.waitForLoadState("domcontentloaded");
460-
await this.stagehandPage._waitForSettledDom(domSettleTimeoutMs);
461-
}
518+
this.logger({
519+
category: "action",
520+
message: "finished waiting for (possible) page navigation",
521+
level: 1,
522+
});
462523

463-
await Promise.race([
464-
this.stagehandPage.page.waitForLoadState("networkidle"),
465-
new Promise((resolve) => setTimeout(resolve, 5_000)),
466-
]).catch((e) => {
467-
this.logger({
468-
category: "action",
469-
message: "network idle timeout hit",
470-
level: 1,
471-
auxiliary: {
472-
trace: {
473-
value: e.stack,
474-
type: "string",
475-
},
476-
message: {
477-
value: e.message,
478-
type: "string",
479-
},
524+
if (this.stagehandPage.page.url() !== initialUrl) {
525+
this.logger({
526+
category: "action",
527+
message: "new page detected with URL",
528+
level: 1,
529+
auxiliary: {
530+
url: {
531+
value: this.stagehandPage.page.url(),
532+
type: "string",
480533
},
481-
});
534+
},
482535
});
536+
}
537+
} else if (typeof locator[method as keyof typeof locator] === "function") {
538+
// Fallback: any other locator method
539+
// Log current URL before action
540+
this.logger({
541+
category: "action",
542+
message: "page URL before action",
543+
level: 2,
544+
auxiliary: {
545+
url: {
546+
value: this.stagehandPage.page.url(),
547+
type: "string",
548+
},
549+
},
550+
});
483551

552+
// Perform the action
553+
try {
554+
await (
555+
locator[method as keyof Locator] as unknown as (
556+
...args: string[]
557+
) => Promise<void>
558+
)(...args.map((arg) => arg?.toString() || ""));
559+
} catch (e) {
484560
this.logger({
485561
category: "action",
486-
message: "finished waiting for (possible) page navigation",
562+
message: "error performing method",
487563
level: 1,
564+
auxiliary: {
565+
error: {
566+
value: e.message,
567+
type: "string",
568+
},
569+
trace: {
570+
value: e.stack,
571+
type: "string",
572+
},
573+
xpath: {
574+
value: xpath,
575+
type: "string",
576+
},
577+
method: {
578+
value: method,
579+
type: "string",
580+
},
581+
args: {
582+
value: JSON.stringify(args),
583+
type: "object",
584+
},
585+
},
488586
});
489587

490-
if (this.stagehandPage.page.url() !== initialUrl) {
491-
this.logger({
492-
category: "action",
493-
message: "new page detected with URL",
494-
level: 1,
495-
auxiliary: {
496-
url: {
497-
value: this.stagehandPage.page.url(),
498-
type: "string",
499-
},
500-
},
501-
});
502-
}
588+
throw new PlaywrightCommandException(e.message);
503589
}
504590
} else {
505591
this.logger({
@@ -547,47 +633,6 @@ export class StagehandActHandler {
547633
});
548634

549635
const outerHtml = clone.outerHTML;
550-
551-
// const variables = {
552-
// // Replace with your actual variables and their values
553-
// // Example:
554-
// username: "JohnDoe",
555-
// email: "[email protected]",
556-
// };
557-
558-
// // Function to replace variable values with variable names
559-
// const replaceVariables = (element: Element) => {
560-
// if (element instanceof HTMLElement) {
561-
// for (const [key, value] of Object.entries(variables)) {
562-
// if (value) {
563-
// element.innerText = element.innerText.replace(
564-
// new RegExp(value, "g"),
565-
// key,
566-
// );
567-
// }
568-
// }
569-
// }
570-
571-
// if (
572-
// element instanceof HTMLInputElement ||
573-
// element instanceof HTMLTextAreaElement
574-
// ) {
575-
// for (const [key, value] of Object.entries(variables)) {
576-
// if (value) {
577-
// element.value = element.value.replace(
578-
// new RegExp(value, "g"),
579-
// key,
580-
// );
581-
// }
582-
// }
583-
// }
584-
// };
585-
586-
// // Replace variables in the cloned element
587-
// replaceVariables(clone);
588-
589-
// // Replace variables in all child elements
590-
// clone.querySelectorAll("*").forEach(replaceVariables);
591636
return outerHtml.trim().replace(/\s+/g, " ");
592637
});
593638
}

0 commit comments

Comments
 (0)