fix(desktop): keep element selectors available on loaded pages (#3187)
* fix(desktop): keep element selectors available on loaded pages Annotate element and Screenshot element returned early when a cached load signal said the guest was not ready, even if the current document was complete and executable. Let executeJavaScript own the readiness boundary and keep selector cleanup available in the same stale-signal state. Extend the real Electron browser regression to reproduce the complete-document/stale-signal mismatch through both toolbar actions. * test(desktop): await browser selector injection Poll the guest selector state through the browser bridge instead of assuming injection completes within 250 ms. This keeps the Electron regression deterministic on slower CI hosts. * Harden browser selector session lifecycle * Keep selector E2E fixtures independent * fix(desktop): harden selector and annotation lifecycle * fix(desktop): keep selector PR focused * refactor(desktop): own element selector lifecycle --------- Co-authored-by: Daniel G. Kang <180234654+dgk-dev@users.noreply.github.com> Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
co-authored by
Daniel G. Kang
Mohamed Boudra
parent
b03dc804eb
commit
770b87a15c
@@ -232,6 +232,21 @@ async function callBrowserTool(client, name, args = {}) {
|
||||
return mcpPayload(await client.callTool({ name, args }), name);
|
||||
}
|
||||
|
||||
async function waitForGuestSelector(client, browserId) {
|
||||
const deadline = Date.now() + 5_000;
|
||||
while (Date.now() < deadline) {
|
||||
const evaluated = await callBrowserTool(client, "browser_evaluate", {
|
||||
browserId,
|
||||
function: "() => Boolean(globalThis.__paseoSelector)",
|
||||
});
|
||||
if (JSON.parse(evaluated.resultJson) === true) {
|
||||
return true;
|
||||
}
|
||||
await delay(50);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
async function createCallerAgent(daemonPort) {
|
||||
const transport = new StreamableHTTPClientTransport(
|
||||
new URL(`http://127.0.0.1:${daemonPort}/mcp/agents`),
|
||||
@@ -384,7 +399,7 @@ function recordViewportMismatch(failures, label, actual, expected) {
|
||||
);
|
||||
}
|
||||
|
||||
async function runRegression({ page, client, serverId, targetUrl, callerAgentId }) {
|
||||
async function runRegression({ page, client, serverId, targetUrl, callerAgentId, artifactDir }) {
|
||||
const failures = [];
|
||||
const originalWorkspaceId = workspaceIds[0];
|
||||
const originalWorkspaceRow = page.getByTestId(
|
||||
@@ -673,16 +688,90 @@ async function runRegression({ page, client, serverId, targetUrl, callerAgentId
|
||||
responsiveViewport,
|
||||
);
|
||||
|
||||
await originalDeck.getByRole("button", { name: "Annotate element" }).click();
|
||||
await page.waitForTimeout(250);
|
||||
const selectorResult = await callBrowserTool(client, "browser_evaluate", {
|
||||
const annotateButton = originalDeck.getByRole("button", { name: "Annotate element" });
|
||||
await page.evaluate((id) => {
|
||||
const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`);
|
||||
if (!(webview instanceof HTMLElement)) throw new Error(`Browser webview ${id} was unavailable`);
|
||||
globalThis.__paseoOriginalIsLoadingDescriptor = Object.getOwnPropertyDescriptor(
|
||||
webview,
|
||||
"isLoading",
|
||||
);
|
||||
Object.defineProperty(webview, "isLoading", {
|
||||
configurable: true,
|
||||
value: () => true,
|
||||
});
|
||||
}, browserId);
|
||||
await annotateButton.click();
|
||||
await page.waitForTimeout(100);
|
||||
assert(
|
||||
!(await originalDeck.getByRole("button", { name: "Cancel element selector" }).isVisible()),
|
||||
"Element selector started while the guest reported a genuine load",
|
||||
);
|
||||
const selectorDuringLoad = await callBrowserTool(client, "browser_evaluate", {
|
||||
browserId,
|
||||
function: "() => Boolean(globalThis.__paseoSelector)",
|
||||
});
|
||||
if (JSON.parse(selectorResult.resultJson) !== true) {
|
||||
failures.push("reused loaded browser remains ready for element annotation after remount");
|
||||
assert(
|
||||
JSON.parse(selectorDuringLoad.resultJson) === false,
|
||||
"Selector injected while the guest reported a genuine load",
|
||||
);
|
||||
assert(
|
||||
await page.getByText("Wait for the page to finish loading").isVisible(),
|
||||
"Element selector loading failure was not visible",
|
||||
);
|
||||
await page.evaluate((id) => {
|
||||
const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`);
|
||||
if (!(webview instanceof HTMLElement)) throw new Error(`Browser webview ${id} was unavailable`);
|
||||
const descriptor = globalThis.__paseoOriginalIsLoadingDescriptor;
|
||||
if (descriptor) Object.defineProperty(webview, "isLoading", descriptor);
|
||||
else delete webview.isLoading;
|
||||
delete globalThis.__paseoOriginalIsLoadingDescriptor;
|
||||
}, browserId);
|
||||
|
||||
const readyStateResult = await callBrowserTool(client, "browser_evaluate", {
|
||||
browserId,
|
||||
function: "() => document.readyState",
|
||||
});
|
||||
assert(
|
||||
JSON.parse(readyStateResult.resultJson) === "complete",
|
||||
"Local browser document did not finish loading",
|
||||
);
|
||||
// Reproduce the report's mismatch: the guest is complete, but the pane's
|
||||
// last loading signal says it is not ready.
|
||||
await page.evaluate((id) => {
|
||||
const webview = document.querySelector(`[data-paseo-browser-id="${id}"]`);
|
||||
if (!(webview instanceof HTMLElement)) {
|
||||
throw new Error(`Browser webview ${id} was unavailable`);
|
||||
}
|
||||
webview.dispatchEvent(new Event("did-start-loading"));
|
||||
}, browserId);
|
||||
|
||||
await annotateButton.click();
|
||||
const annotateSelectorActive = await waitForGuestSelector(client, browserId);
|
||||
await page.screenshot({ path: path.join(artifactDir, "local-page-annotate-selector.png") });
|
||||
if (!annotateSelectorActive) {
|
||||
failures.push("loaded local browser remains ready for element annotation");
|
||||
}
|
||||
|
||||
await originalDeck.getByRole("button", { name: "Cancel element selector" }).click();
|
||||
await delay(10_000);
|
||||
const screenshotButton = originalDeck.getByRole("button", { name: "Screenshot element" });
|
||||
await screenshotButton.click();
|
||||
const screenshotSelectorActive = await waitForGuestSelector(client, browserId);
|
||||
if (!screenshotSelectorActive) {
|
||||
failures.push("loaded local browser remains ready for element screenshots");
|
||||
}
|
||||
await delay(20_500);
|
||||
const selectorAfterPriorTimeout = await callBrowserTool(client, "browser_evaluate", {
|
||||
browserId,
|
||||
function: "() => Boolean(globalThis.__paseoSelector)",
|
||||
});
|
||||
if (JSON.parse(selectorAfterPriorTimeout.resultJson) !== true) {
|
||||
failures.push("a previous selector timeout does not destroy the current selector session");
|
||||
}
|
||||
await page.screenshot({ path: path.join(artifactDir, "local-page-screenshot-selector.png") });
|
||||
await originalDeck.getByRole("button", { name: "Cancel element selector" }).click();
|
||||
|
||||
if (failures.length > 0) {
|
||||
throw new Error(`Browser viewport regressions:\n- ${failures.join("\n- ")}`);
|
||||
}
|
||||
@@ -698,6 +787,7 @@ async function runRegression({ page, client, serverId, targetUrl, callerAgentId
|
||||
list: "passed",
|
||||
snapshot: "passed",
|
||||
click: "passed",
|
||||
localPageSelectors: "passed",
|
||||
};
|
||||
}
|
||||
|
||||
@@ -796,10 +886,11 @@ async function main() {
|
||||
serverId: status.serverId,
|
||||
targetUrl: target.url,
|
||||
callerAgentId,
|
||||
artifactDir,
|
||||
});
|
||||
writeJson(path.join(artifactDir, "result.json"), report);
|
||||
console.log(
|
||||
`Browser desktop browser E2E passed: WebContents ${report.originalWebContentsId} remained ${report.finalWebContentsId}; viewport, inactive capture, focus continuity, list, snapshot, click passed.`,
|
||||
`Browser desktop browser E2E passed: WebContents ${report.originalWebContentsId} remained ${report.finalWebContentsId}; viewport, inactive capture, focus continuity, list, snapshot, click, local-page selectors passed.`,
|
||||
);
|
||||
} catch (error) {
|
||||
console.error(`Browser desktop browser E2E failed. Artifacts: ${artifactDir}`);
|
||||
|
||||
Reference in New Issue
Block a user