diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94b1379..b831cae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,13 @@ jobs: components: rustfmt, clippy - run: cargo fmt --all -- --check - run: cargo test --all-targets --locked + - name: Verify native clicks with full headless Chrome + timeout-minutes: 5 + run: | + export BROWSER_CLI_TEST_CHROME="$(command -v google-chrome)" + test -n "$BROWSER_CLI_TEST_CHROME" + "$BROWSER_CLI_TEST_CHROME" --version + cargo test --locked --test page_targets_browser -- --ignored --nocapture - name: Verify tests ignore inherited proxy configuration run: >- env -u NO_PROXY -u no_proxy diff --git a/Cargo.lock b/Cargo.lock index 712fb37..b6f7ec5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1208,7 +1208,7 @@ checksum = "db13adb97ab515a3691f56e4dbab09283d0b86cb45abd991d8634a9d6f501760" [[package]] name = "lexmount-browser" -version = "1.2.3" +version = "1.2.4" dependencies = [ "base64 0.22.1", "clap", diff --git a/Cargo.toml b/Cargo.toml index 267bde6..935dc7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lexmount-browser" -version = "1.2.3" +version = "1.2.4" edition = "2024" license = "MIT" description = "Native Rust SDK and CLI for Lexmount cloud browsers" diff --git a/README.md b/README.md index c790bbb..195608a 100644 --- a/README.md +++ b/README.md @@ -29,8 +29,8 @@ write `{"ok":false,"error":"...","message":"..."}` to stderr and exit with status 1. JavaScript evaluation failures keep the `cdp_error` category, but now include the browser's error summary and, when provided, one-based line/column positions. For example, a missing selector reports `Error: selector not found` -instead of only `Uncaught`. This also applies to actions implemented with -evaluation, such as `click` and `fill`; it does not retry or fix the action. +instead of only `Uncaught`. This also applies to DOM lookup/checks in `click` +and evaluation in `fill`; it does not retry or fix the action. The summary is the first line of the exception description (up to 1024 Unicode characters plus a truncation marker), falling back to a primitive thrown value @@ -86,8 +86,8 @@ automatic action retries. These changes require a new CLI release; published Explicit page selection is introduced in version 1.2.0. Check that the installed binary's `browser-cli action --help` lists `--target-id`; the published 1.1.15 binary does not have it. The package version and both bootstrap scripts target -1.2.3 together. Merging or building this source does not publish release assets: -bootstrap can install 1.2.3 only after its binaries and checksums are published +1.2.4 together. Merging or building this source does not publish release assets: +bootstrap can install 1.2.4 only after its binaries and checksums are published to COS. Until then, use a source build for local verification. Every `action` command accepts an optional `--target-id`. Obtain the page's CDP @@ -126,11 +126,35 @@ and inspect again when there are multiple plausible pages. SDK callers can use `lexmount_browser::cdp::Cdp::connect_to_target(ws_url, page_id)`. `Cdp::connect(ws_url)` retains its existing default behavior. +### Native click input + +Starting in 1.2.4, `action click --selector CSS` (and SDK `Cdp::click`) scrolls +the element into view and sends CDP `Input.dispatchMouseEvent` move/press/release +events. This replaces JavaScript `HTMLElement.click()`, which produces an +untrusted event without user activation and can leave `window.open()` blocked. +It does not add a user gesture to arbitrary `action eval` expressions. + +Before pressing, the CLI checks that the selected element is attached, enabled, +visible and hit-testable at a client-rectangle center inside the viewport. +Disabled controls/ancestors, ARIA-disabled or inert ancestors, and overlays are +rejected. It rechecks the **same DOM object and point** after hover; a replacement, +movement away from that point, or new overlay causes a `cdp_error`, not a click on +another element. There is no automatic retry or JavaScript-click fallback. +These checks are not an atomic lock against later page mutations. + +The existing main-document CSS selector scope is unchanged: this does not add +iframe or shadow-root traversal. `{"ok":true,"data":true}` means input was +dispatched, **not** that the website completed the task or opened a popup. Site +logic/browser policy can still prevent an outcome. Inspect the page/targets and +explicitly select any new result tab as shown above; click never navigates to an +inferred URL or automatically switches tabs. A failed command may have partially +dispatched input; inspect state before retrying it. + ### Local regression tests ```bash cargo test --all-targets --locked -# Optional: use a local Chrome/Chromium executable, including chrome-headless-shell. +# Use full Chrome/Chromium to cover popup blocking; headless-shell alone is insufficient. BROWSER_CLI_TEST_CHROME=/path/to/chrome cargo test --locked --test page_targets_browser -- --ignored --nocapture ``` @@ -139,6 +163,11 @@ running the same `cargo test` command. The opt-in test launches a separate headless profile and loopback-only fixtures; it does not use a Lexmount account, real websites, or an existing browser profile. The default suite exercises all action routes and failure/no-fallback behavior with deterministic CDP fixtures. +CI also runs the real-browser suite with its installed full Google Chrome. +Tests assert trusted input/user activation as well as popup creation, and cover +scrolling, hidden/disabled/covered elements, hover changes and cleanup failures. +Some headless-shell builds allow untrusted popups; passing there alone does not +demonstrate the click fix. ## Agent Skill package @@ -170,7 +199,7 @@ overwrite an existing release with changed binaries. `skills/lexmount-browser/scripts/bootstrap.ps1` and `bootstrap.sh`. 2. Run `.github/scripts/test-release-version.ps1` with Windows PowerShell 5.1 or PowerShell 7, then `.github/scripts/verify-release-version.ps1 -ReleaseTag - v1.2.3` (substitute the intended version). Complete CI and merge the PR. + v1.2.4` (substitute the intended version). Complete CI and merge the PR. 3. Create the matching tag **on that merged commit**. Typing a new tag or release title in GitHub does not update any source version. The release workflow rejects inconsistent versions before building, signing or uploading. diff --git a/skills/lexmount-browser/SKILL.md b/skills/lexmount-browser/SKILL.md index 9980474..efaeb60 100644 --- a/skills/lexmount-browser/SKILL.md +++ b/skills/lexmount-browser/SKILL.md @@ -29,9 +29,9 @@ Do not run the binary for the other platform. Both platform binaries emit JSON. ## Setup 1. Resolve `` from this `SKILL.md` and select the matching platform paths above. -2. Run the Skill-local bootstrap script if the binary is missing. Then run `sh "/scripts/doctor.sh"` on macOS arm64 or `& "\scripts\doctor.ps1"` in Windows PowerShell. +2. Run the Skill-local bootstrap script if the binary is missing. Then run `sh "/scripts/doctor.sh"` on macOS arm64. On Windows, invoke the installed binary's `doctor` command directly, not `scripts/doctor.ps1`: PowerShell uses `& "\bin\browser-cli.exe" doctor`; Bash/Git Bash uses `"/bin/browser-cli.exe" doctor` with forward slashes in the absolute path. In WorkBuddy on Windows, prefer its Bash tool when available: its PowerShell tool can return only an exit code and omit the JSON. Keep the tool's normal permissions and sandbox. 3. If credentials are missing, run `browser-cli auth login`. Pass `--client-name ""` when the current Agent has a user-facing name; otherwise the CLI uses `Agent`. Let the user approve in their browser. Never ask them to paste an API key into chat. -4. Run `browser-cli doctor` again. Continue only when `ready_for_browser_actions` is true. +4. After changing credentials, run `browser-cli doctor` again; otherwise use the doctor result already obtained. Continue only when its actual JSON reports `ready_for_browser_actions: true`. An exit code without that JSON is not a readiness result; see [Windows diagnostic output](references/troubleshooting.md#windows-diagnostic-output). Read [authentication.md](references/authentication.md) only when login or credentials fail. Read [commands.md](references/commands.md) when selecting commands. Read [troubleshooting.md](references/troubleshooting.md) only after an error. diff --git a/skills/lexmount-browser/references/troubleshooting.md b/skills/lexmount-browser/references/troubleshooting.md index d247c73..49205f6 100644 --- a/skills/lexmount-browser/references/troubleshooting.md +++ b/skills/lexmount-browser/references/troubleshooting.md @@ -12,4 +12,29 @@ Run `browser-cli doctor` first and use the failed check's message. - Skill root unknown: resolve the directory containing the loaded `SKILL.md` with the current host's locator: Codex supplies its absolute source path in the Skill metadata, Claude Code provides `${CLAUDE_SKILL_DIR}`, and WorkBuddy/CodeBuddy provides `${CODEBUDDY_SKILL_DIR}`. Do not infer it from the working directory or search the user's home directory. - command not found after bootstrap: invoke `"/bin/browser-cli"` on macOS arm64 or `& "\bin\browser-cli.exe"` in Windows PowerShell; no PATH change or restart is required. +## Windows diagnostic output + +For an installed CLI, use its absolute `bin/browser-cli.exe` path with `doctor`. +The `doctor.ps1` helper is not required. Under PowerShell's `Restricted` execution +policy, a `.ps1` file can be rejected before its body or the CLI runs; this is not +evidence of a CLI authentication or cloud-browser failure. Do not lower execution +policy, disable the sandbox, or reinstall a working binary to run this check. If +the binary is missing and the bootstrap script is blocked, report the installation +prerequisite instead of bypassing the policy. + +WorkBuddy's Windows PowerShell tool may report `Command completed with exit code +0` (or `1`) without stdout/stderr. Do not treat that as the doctor's JSON or infer +a specific failure cause from the code alone. If the harness already provides +Bash/Git Bash, invoke the same Windows executable there; do not install another +shell or use a different-platform binary. For the read-only `version`/`doctor` +checks, retrying once to recover missing output is sufficient. Read the actual +JSON and preserve any reported failure; do not keep retrying to obtain success. + +If Bash is unavailable, capture the read-only command's output in a new task-local +file and read it with the harness's file-reading tool. If that also fails, report +the output-capture limitation rather than claiming readiness or reauthorizing. +This fallback is not permission to repeat state-changing commands such as creating +sessions: inspect their existing result/state before considering any retry. Do not +include credentials, tokens or authentication configuration in diagnostic files. + Always close a newly created temporary session when abandoning a failed task. diff --git a/skills/lexmount-browser/scripts/bootstrap.ps1 b/skills/lexmount-browser/scripts/bootstrap.ps1 index 559b88c..192e2d5 100644 --- a/skills/lexmount-browser/scripts/bootstrap.ps1 +++ b/skills/lexmount-browser/scripts/bootstrap.ps1 @@ -12,7 +12,7 @@ function Invoke-Tls12Download { } } -$version = if ($env:LEXMOUNT_BROWSER_CLI_VERSION) { $env:LEXMOUNT_BROWSER_CLI_VERSION } else { "1.2.3" } +$version = if ($env:LEXMOUNT_BROWSER_CLI_VERSION) { $env:LEXMOUNT_BROWSER_CLI_VERSION } else { "1.2.4" } $downloadBaseUrl = if ($env:LEXMOUNT_BROWSER_CLI_DOWNLOAD_BASE_URL) { $env:LEXMOUNT_BROWSER_CLI_DOWNLOAD_BASE_URL.TrimEnd('/') } else { "https://cli-bin-1377899528.cos.ap-nanjing.myqcloud.com/releases/browser-cli" } $architecture = if ($env:PROCESSOR_ARCHITEW6432) { $env:PROCESSOR_ARCHITEW6432 } else { $env:PROCESSOR_ARCHITECTURE } if ($architecture -ne "AMD64") { throw "Only Windows x64 is supported" } diff --git a/skills/lexmount-browser/scripts/bootstrap.sh b/skills/lexmount-browser/scripts/bootstrap.sh index 4bc64b8..2a1ea79 100755 --- a/skills/lexmount-browser/scripts/bootstrap.sh +++ b/skills/lexmount-browser/scripts/bootstrap.sh @@ -1,7 +1,7 @@ #!/bin/sh set -eu -version="${LEXMOUNT_BROWSER_CLI_VERSION:-1.2.3}" +version="${LEXMOUNT_BROWSER_CLI_VERSION:-1.2.4}" download_base_url="${LEXMOUNT_BROWSER_CLI_DOWNLOAD_BASE_URL:-https://cli-bin-1377899528.cos.ap-nanjing.myqcloud.com/releases/browser-cli}" repo="${download_base_url%/}/v${version}" case "$(uname -s)-$(uname -m)" in diff --git a/src/cdp.rs b/src/cdp.rs index aab8980..3ea1048 100644 --- a/src/cdp.rs +++ b/src/cdp.rs @@ -11,6 +11,7 @@ use tungstenite::Message; use crate::{Error, Result}; +mod click; mod proxy; pub struct Cdp { @@ -162,10 +163,6 @@ impl Cdp { .unwrap_or(Value::Null)) } - pub fn click(&mut self, selector: &str) -> Result { - self.evaluate(&format!("(()=>{{const e=document.querySelector({});if(!e)throw new Error('selector not found');e.scrollIntoView({{block:'center'}});e.click();return true}})()", serde_json::to_string(selector)?)) - } - pub fn fill(&mut self, selector: &str, value: &str) -> Result { self.evaluate(&format!("(()=>{{const e=document.querySelector({});if(!e)throw new Error('selector not found');const s=Object.getOwnPropertyDescriptor(Object.getPrototypeOf(e),'value')?.set;s?s.call(e,{}):e.value={};e.dispatchEvent(new Event('input',{{bubbles:true}}));e.dispatchEvent(new Event('change',{{bubbles:true}}));return true}})()", serde_json::to_string(selector)?, serde_json::to_string(value)?, serde_json::to_string(value)?)) } diff --git a/src/cdp/click.rs b/src/cdp/click.rs new file mode 100644 index 0000000..4a8b480 --- /dev/null +++ b/src/cdp/click.rs @@ -0,0 +1,136 @@ +use serde_json::{Value, json}; + +use super::{Cdp, javascript_exception_message}; +use crate::{Error, Result}; + +// Keep the original DOM object across the hover check. Re-querying the selector +// could silently click a replacement element installed by a mousemove handler. +const CLICK_POINT: &str = r#"function(point) { + const e = this; + if (!e.isConnected || e.ownerDocument !== document) + throw new Error('click target is detached'); + if (e.matches(':disabled') || e.closest( + 'button:disabled, input:disabled, select:disabled, textarea:disabled, option:disabled, optgroup:disabled, [inert], [aria-disabled="true" i]')) + throw new Error('click target is disabled or inert'); + const style = getComputedStyle(e); + if (style.display === 'none' || style.visibility === 'hidden' || style.visibility === 'collapse') + throw new Error('click target is not visible'); + if (!point) e.scrollIntoView({block: 'center', inline: 'center', behavior: 'instant'}); + const width = document.documentElement.clientWidth; + const height = document.documentElement.clientHeight; + const rects = [...e.getClientRects()].map(r => ({ + left: Math.max(0, r.left), right: Math.min(width, r.right), + top: Math.max(0, r.top), bottom: Math.min(height, r.bottom) + })).filter(r => r.right > r.left && r.bottom > r.top); + if (!rects.length) throw new Error('click target has no visible area in the viewport'); + if (point && !rects.some(r => point.x >= r.left && point.x < r.right && + point.y >= r.top && point.y < r.bottom)) + throw new Error('click target moved after hover; inspect the page and try again'); + const candidates = point ? [point] : rects.map(r => ({ + x: (r.left + r.right) / 2, y: (r.top + r.bottom) / 2 + })); + for (const candidate of candidates) { + const hit = document.elementFromPoint(candidate.x, candidate.y); + if (hit && (hit === e || e.contains(hit))) return candidate; + } + throw new Error('click target is covered or cannot receive pointer events'); +}"#; + +#[derive(Clone, Copy, PartialEq)] +struct Point { + x: f64, + y: f64, +} + +impl Cdp { + /// Click a main-document CSS selector using native CDP mouse input. + /// + /// Success means the input was dispatched, not that the site's task or + /// navigation succeeded. No JavaScript-click fallback or tab switching occurs. + pub fn click(&mut self, selector: &str) -> Result { + let response = self.command( + "Runtime.evaluate", + json!({ + "expression": format!( + "(()=>{{const e=document.querySelector({});if(!e)throw new Error('selector not found');return e}})()", + serde_json::to_string(selector)? + ), + "returnByValue": false + }), + )?; + let remote = runtime_result(&response)?; + let object_id = remote["objectId"] + .as_str() + .filter(|id| !id.is_empty()) + .ok_or_else(|| Error::Cdp("click target response missing objectId".into()))?; + + let result = self.click_object(object_id); + // Navigation may already have destroyed the context. Cleanup must not + // turn a dispatched click into failure or hide its original error. + let _ = self.command("Runtime.releaseObject", json!({"objectId": object_id})); + result + } + + fn click_object(&mut self, object_id: &str) -> Result { + let point = self.click_point(object_id, None)?; + self.mouse_event("mouseMoved", point, "none", 0, 0)?; + // Hover can move, cover, disable or replace the target. Never chase a + // changed selector or blindly press at its stale coordinates. + if self.click_point(object_id, Some(point))? != point { + return Err(Error::Cdp("click target changed after hover".into())); + } + let pressed = self.mouse_event("mousePressed", point, "left", 1, 1); + // Even if pressing returns an error, best-effort release avoids leaving + // the button down after a partially handled command. Never repeat press. + let released = self.mouse_event("mouseReleased", point, "left", 0, 1); + pressed?; + released?; + Ok(json!(true)) + } + + fn click_point(&mut self, object_id: &str, point: Option) -> Result { + let response = self.command( + "Runtime.callFunctionOn", + json!({ + "objectId": object_id, + "functionDeclaration": CLICK_POINT, + "arguments": [{"value": point.map(|p| json!({"x":p.x,"y":p.y}))}], + "returnByValue": true + }), + )?; + let value = &runtime_result(&response)?["value"]; + let coordinate = |name| { + value[name] + .as_f64() + .filter(|v| v.is_finite() && *v >= 0.0) + .ok_or_else(|| Error::Cdp("click target response has invalid coordinates".into())) + }; + Ok(Point { + x: coordinate("x")?, + y: coordinate("y")?, + }) + } + + fn mouse_event( + &mut self, + event: &str, + point: Point, + button: &str, + buttons: u8, + count: u8, + ) -> Result { + self.command( + "Input.dispatchMouseEvent", + json!({"type":event, "x":point.x, "y":point.y, + "button":button, "buttons":buttons, "clickCount":count, + "pointerType":"mouse", "modifiers":0}), + ) + } +} + +fn runtime_result(response: &Value) -> Result<&Value> { + if let Some(exception) = response.get("exceptionDetails") { + return Err(Error::Cdp(javascript_exception_message(exception))); + } + Ok(&response["result"]) +} diff --git a/tests/page_targets.rs b/tests/page_targets.rs index be41951..3859a7d 100644 --- a/tests/page_targets.rs +++ b/tests/page_targets.rs @@ -24,6 +24,15 @@ impl Peer { } fn with_exception(targets: Value, fail_attach: bool, exception: Option) -> Self { + Self::with_responses(targets, fail_attach, exception, vec![]) + } + + fn with_responses( + targets: Value, + fail_attach: bool, + exception: Option, + overrides: Vec<(&'static str, usize, Value)>, + ) -> Self { let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); listener.set_nonblocking(true).unwrap(); let url = format!("ws://{}", listener.local_addr().unwrap()); @@ -99,8 +108,26 @@ impl Peer { _ => { assert!(!attached.is_empty()); assert_eq!(request["sessionId"], format!("attached-{attached}")); + let occurrence = seen.iter().filter(|r| r["method"] == method).count(); + if let Some((_, _, response)) = overrides + .iter() + .find(|(name, count, _)| *name == method && *count == occurrence) + { + let mut response = response.clone(); + response["id"] = request["id"].clone(); + socket + .send(Message::Text(response.to_string().into())) + .unwrap(); + continue; + } match method { - "Page.enable" | "Runtime.enable" => json!({}), + "Page.enable" + | "Runtime.enable" + | "Runtime.releaseObject" + | "Input.dispatchMouseEvent" => json!({}), + "Runtime.callFunctionOn" => { + json!({"result":{"value":{"x":100,"y":80}}}) + } "Page.navigate" => json!({"frameId":"frame"}), "Page.getFrameTree" => json!({"selectedTarget":attached}), "Page.getLayoutMetrics" => { @@ -140,7 +167,11 @@ impl Peer { } _ => json!(true), }; - json!({"result":{"value":value}}) + if request["params"]["returnByValue"] == false { + json!({"result":{"type":"object","subtype":"node","objectId":"element"}}) + } else { + json!({"result":{"value":value}}) + } } _ => panic!("unexpected method: {method}"), } @@ -184,6 +215,237 @@ fn two_pages() -> Value { ]) } +fn click_fixture( + overrides: Vec<(&'static str, usize, Value)>, +) -> (std::process::Output, Vec) { + let directory = tempfile::tempdir().unwrap(); + let peer = Peer::with_responses(two_pages(), false, None, overrides); + let server = api(&peer.url); + let output = support::cli( + &server.base_url(), + directory.path(), + &[ + "action", + "click", + "--session-id", + "browser", + "--target-id", + "wanted", + "--selector", + r#"[data-name='a"b']"#, + ], + ); + (output, peer.finish()) +} + +fn click_events(seen: &[Value]) -> Vec { + seen.iter() + .filter(|r| r["method"] == "Input.dispatchMouseEvent") + .map(|r| r["params"].clone()) + .collect() +} + +#[test] +fn click_uses_native_input_and_rechecks_the_same_dom_object_after_hover() { + let (output, seen) = click_fixture(vec![]); + assert_eq!(support::data(output), true); + assert_eq!( + seen.iter() + .skip(4) + .map(|r| r["method"].as_str().unwrap()) + .collect::>(), + [ + "Runtime.evaluate", + "Runtime.callFunctionOn", + "Input.dispatchMouseEvent", + "Runtime.callFunctionOn", + "Input.dispatchMouseEvent", + "Input.dispatchMouseEvent", + "Runtime.releaseObject" + ] + ); + let lookup = &seen[4]["params"]; + assert_eq!(lookup["returnByValue"], false); + assert!( + lookup["expression"] + .as_str() + .unwrap() + .contains(&serde_json::to_string(r#"[data-name='a"b']"#).unwrap()) + ); + for request in &seen { + assert!(request["params"].get("userGesture").is_none()); + for field in ["expression", "functionDeclaration"] { + assert!( + !request["params"][field] + .as_str() + .unwrap_or("") + .contains(".click(") + ); + } + } + assert_eq!(seen[5]["params"]["objectId"], "element"); + assert_eq!(seen[5]["params"]["arguments"], json!([{"value":null}])); + assert_eq!(seen[7]["params"]["objectId"], "element"); + assert_eq!( + seen[7]["params"]["arguments"], + json!([{"value":{"x":100.0,"y":80.0}}]) + ); + let events = click_events(&seen); + for (event, (kind, button, buttons, count)) in events.iter().zip([ + ("mouseMoved", "none", 0, 0), + ("mousePressed", "left", 1, 1), + ("mouseReleased", "left", 0, 1), + ]) { + assert_eq!( + *event, + json!({"type":kind,"x":100.0,"y":80.0,"button":button,"buttons":buttons, + "clickCount":count,"pointerType":"mouse","modifiers":0}) + ); + } + assert_eq!( + seen.last().unwrap()["params"], + json!({"objectId":"element"}) + ); +} + +#[test] +fn click_probe_errors_release_the_object_without_pressing() { + for (occurrence, response, expected, expected_moves) in [ + ( + 1, + json!({"result":{"result":{"value":{"x":-1,"y":80}}}}), + "invalid coordinates", + 0, + ), + ( + 1, + json!({"result":{"result":{"value":{"x":100}}}}), + "invalid coordinates", + 0, + ), + ( + 1, + json!({"result":{"result":{"value":{"x":"NaN","y":80}}}}), + "invalid coordinates", + 0, + ), + ( + 1, + json!({"result":{"exceptionDetails":{"exception":{"description":"Error: covered\n at private-stack"}}}}), + "Error: covered", + 0, + ), + ( + 2, + json!({"result":{"exceptionDetails":{"exception":{"description":"Error: detached"}}}}), + "Error: detached", + 1, + ), + ( + 2, + json!({"error":{"code":-32000,"message":"context destroyed"}}), + "context destroyed", + 1, + ), + ( + 2, + json!({"result":{"result":{"value":{"x":200,"y":80}}}}), + "changed after hover", + 1, + ), + ] { + let (output, seen) = click_fixture(vec![("Runtime.callFunctionOn", occurrence, response)]); + assert_eq!(output.status.code(), Some(1)); + assert!(output.stdout.is_empty()); + let error: Value = serde_json::from_slice(&output.stderr).unwrap(); + assert_eq!(error["error"], "cdp_error"); + assert!( + error["message"].as_str().unwrap().contains(expected), + "{error}" + ); + assert!(!error["message"].as_str().unwrap().contains("private-stack")); + let events = click_events(&seen); + assert_eq!(events.len(), expected_moves); + assert!(events.iter().all(|e| e["type"] == "mouseMoved")); + assert_eq!(seen.last().unwrap()["method"], "Runtime.releaseObject"); + } +} + +#[test] +fn invalid_click_object_is_rejected_before_input() { + for remote in [json!({"type":"undefined"}), json!({"objectId":""})] { + let (output, seen) = click_fixture(vec![( + "Runtime.evaluate", + 1, + json!({"result":{"result":remote}}), + )]); + assert_eq!(output.status.code(), Some(1)); + assert!(String::from_utf8_lossy(&output.stderr).contains("missing objectId")); + assert!(click_events(&seen).is_empty()); + assert_eq!(seen.last().unwrap()["method"], "Runtime.evaluate"); + } +} + +#[test] +fn click_input_errors_are_propagated_without_repeating_press() { + for occurrence in 1..=3 { + let (output, seen) = click_fixture(vec![ + ( + "Input.dispatchMouseEvent", + occurrence, + json!({"error":{"code":-32000,"message":"input failure"}}), + ), + ( + "Runtime.releaseObject", + 1, + json!({"error":{"code":-32000,"message":"cleanup failure"}}), + ), + ]); + assert_eq!(output.status.code(), Some(1)); + assert!(output.stdout.is_empty()); + let error: Value = serde_json::from_slice(&output.stderr).unwrap(); + assert_eq!(error["message"], "CDP command failed: input failure"); + let events = click_events(&seen); + assert_eq!(events.len(), if occurrence == 1 { 1 } else { 3 }); + if occurrence > 1 { + assert_eq!(events[1]["type"], "mousePressed"); + assert_eq!(events[2]["type"], "mouseReleased"); + } + assert_eq!(seen.last().unwrap()["method"], "Runtime.releaseObject"); + } +} + +#[test] +fn click_keeps_the_first_input_error_when_release_also_fails() { + let (output, seen) = click_fixture(vec![ + ( + "Input.dispatchMouseEvent", + 2, + json!({"error":{"code":-32000,"message":"press failure"}}), + ), + ( + "Input.dispatchMouseEvent", + 3, + json!({"error":{"code":-32000,"message":"release failure"}}), + ), + ]); + assert_eq!(output.status.code(), Some(1)); + let error: Value = serde_json::from_slice(&output.stderr).unwrap(); + assert_eq!(error["message"], "CDP command failed: press failure"); + assert_eq!(click_events(&seen).len(), 3); +} + +#[test] +fn click_cleanup_failure_does_not_mask_successful_input() { + let (output, seen) = click_fixture(vec![( + "Runtime.releaseObject", + 1, + json!({"error":{"code":-32000,"message":"context already destroyed"}}), + )]); + assert_eq!(support::data(output), true); + assert_eq!(click_events(&seen).len(), 3); +} + #[test] fn evaluation_exception_details_reach_cli_without_changing_failure_contract() { let directory = tempfile::tempdir().unwrap(); diff --git a/tests/page_targets_browser.rs b/tests/page_targets_browser.rs index 09ca874..66d6889 100644 --- a/tests/page_targets_browser.rs +++ b/tests/page_targets_browser.rs @@ -15,8 +15,10 @@ use std::{ const HOME: &str = r#"Search fixture -"#; const RESULT: &str = r#"Result fixture @@ -70,17 +72,32 @@ impl Browser { http: String::new(), }; let deadline = Instant::now() + Duration::from_secs(20); + let client = reqwest::blocking::Client::builder() + .no_proxy() + .timeout(Duration::from_secs(2)) + .build() + .unwrap(); loop { if let Ok(port_file) = fs::read_to_string(browser._profile.path().join("DevToolsActivePort")) { let mut lines = port_file.lines(); - let port: u16 = lines.next().unwrap().parse().unwrap(); - let path = lines.next().unwrap(); - assert!(path.starts_with("/devtools/browser/")); - browser.websocket = format!("ws://127.0.0.1:{port}{path}"); - browser.http = format!("http://127.0.0.1:{port}"); - return browser; + // The file can briefly be empty/partial while Chrome starts. + if let (Some(port), Some(path)) = (lines.next(), lines.next()) + && let Ok(port) = port.parse::() + && path.starts_with("/devtools/browser/") + { + browser.websocket = format!("ws://127.0.0.1:{port}{path}"); + browser.http = format!("http://127.0.0.1:{port}"); + // Wait for the initial page too, avoiding a racing blank + // target creation by Cdp::connect during browser startup. + if let Ok(response) = client.get(format!("{}/json", browser.http)).send() + && let Ok(targets) = response.json::>() + && targets.iter().any(|t| t["type"] == "page") + { + return browser; + } + } } assert!( browser.process.try_wait().unwrap().is_none(), @@ -121,6 +138,245 @@ fn pages(cdp: &mut Cdp) -> Vec { .collect() } +#[test] +#[ignore = "requires BROWSER_CLI_TEST_CHROME pointing to a Chrome/Chromium executable"] +fn native_click_checks_actionability_and_retains_the_original_element() { + if support::isolated_test( + "native_click_checks_actionability_and_retains_the_original_element", + Duration::from_secs(120), + ) { + return; + } + let chromium = std::env::var_os("BROWSER_CLI_TEST_CHROME").unwrap(); + let browser = Browser::start(Path::new(&chromium)); + let directory = tempfile::tempdir().unwrap(); + let api = MockServer::start(); + api.mock(|when, then| { + when.method(POST).path("/instance/session"); + then.status(200) + .json_body(json!({"session_id":"browser","status":"active","ws":browser.websocket})); + }); + let mut observer = Cdp::connect(&browser.websocket).unwrap(); + let targets = pages(&mut observer); + let target = targets[0]["targetId"].as_str().unwrap(); + let cases = [ + ("", "#target", None), + ( + "", + "#target", + None, + ), + ( + "
", + "#target", + None, + ), + ( + "
", + "#target", + None, + ), + ( + "", + "#target", + None, + ), + ( + "", + r#"[data-label='a"b']"#, + None, + ), + ( + "
", + "#target", + None, + ), + ( + "", + "#target", + Some("disabled"), + ), + ( + "
", + "#target", + Some("disabled"), + ), + ( + "", + "#target", + Some("disabled"), + ), + ( + "
", + "#target", + Some("inert"), + ), + ( + "
", + "#target", + Some("disabled"), + ), + ( + "", + "#target", + Some("not visible"), + ), + ( + "", + "#target", + Some("not visible"), + ), + ( + "", + "#target", + Some("no visible area"), + ), + ( + "", + "#target", + Some("cannot receive pointer"), + ), + ( + "
Overlay
", + "#target", + Some("covered"), + ), + ( + "", + "#target", + Some("disabled"), + ), + ( + "", + "#target", + Some("detached"), + ), + ( + "", + "#target", + Some("moved after hover"), + ), + ( + "", + "#target", + Some("covered"), + ), + ("

No target

", "#target", Some("selector not found")), + ( + "", + "[", + Some("SyntaxError"), + ), + ]; + for (html, selector, expected_error) in cases { + // Reset both document and pointer so mouseover is deterministic per case. + observer + .navigate("about:blank", Duration::from_secs(5)) + .unwrap(); + observer + .command( + "Input.dispatchMouseEvent", + json!({"type":"mouseMoved","x":0,"y":0}), + ) + .unwrap(); + observer.evaluate(&format!( + "document.body.innerHTML={}; window.clicks=[]; window.presses=0; document.addEventListener('pointerdown',()=>window.presses++,true); document.addEventListener('click',e=>window.clicks.push({{id:e.target.id,trusted:e.isTrusted,active:navigator.userActivation.isActive}}),true); true", + serde_json::to_string(html).unwrap() + )).unwrap(); + let output = support::cli( + &api.base_url(), + directory.path(), + &[ + "action", + "click", + "--session-id", + "browser", + "--target-id", + target, + "--selector", + selector, + ], + ); + let events = observer + .evaluate("({clicks:window.clicks,presses:window.presses})") + .unwrap(); + if let Some(expected) = expected_error { + assert_eq!(output.status.code(), Some(1), "{html}"); + assert!(output.stdout.is_empty()); + let error: Value = serde_json::from_slice(&output.stderr).unwrap(); + assert_eq!(error["error"], "cdp_error"); + assert!( + error["message"].as_str().unwrap().contains(expected), + "{html}: {error}" + ); + assert_eq!( + events, + json!({"clicks":[],"presses":0}), + "must not click target/overlay/replacement: {html}" + ); + } else { + assert_eq!(support::data(output), true, "{html}"); + assert_eq!(events["presses"], 1, "{html}"); + let clicks = events["clicks"].as_array().unwrap(); + assert_eq!(clicks.len(), 1, "{html}"); + assert_eq!(clicks[0]["trusted"], true, "{html}"); + assert_eq!(clicks[0]["active"], true, "{html}"); + } + assert_eq!(observer.evaluate("location.href").unwrap(), "about:blank"); + assert_eq!(pages(&mut observer).len(), 1); + } + + // Arbitrary evaluation stays untrusted and gains no user activation. + observer + .navigate("about:blank", Duration::from_secs(5)) + .unwrap(); + assert_eq!(observer.evaluate( + "document.body.innerHTML=''; let evidence; const b=document.querySelector('button'); b.onclick=e=>evidence={trusted:e.isTrusted,active:navigator.userActivation.isActive}; b.click(); evidence" + ).unwrap(), json!({"trusted":false,"active":false})); + + // Same-tab navigation can destroy the remote DOM object before cleanup. + // It must still produce the unchanged successful click result. + let fixture = MockServer::start(); + fixture.mock(|when, then| { + when.method(GET).path("/next"); + then.status(200) + .body("Next page"); + }); + let next_url = fixture.url("/next"); + observer.evaluate(&format!( + "document.body.innerHTML='Next page'; document.querySelector('#target').href={}; true", + serde_json::to_string(&next_url).unwrap() + )).unwrap(); + assert_eq!( + support::data(support::cli( + &api.base_url(), + directory.path(), + &[ + "action", + "click", + "--session-id", + "browser", + "--target-id", + target, + "--selector", + "#target" + ] + )), + true + ); + let deadline = Instant::now() + Duration::from_secs(5); + while !pages(&mut observer) + .iter() + .any(|page| page["targetId"] == target && page["url"] == next_url) + { + assert!( + Instant::now() < deadline, + "same-page click did not navigate" + ); + thread::sleep(Duration::from_millis(50)); + } +} + #[test] #[ignore = "requires BROWSER_CLI_TEST_CHROME pointing to a Chrome/Chromium executable"] fn javascript_errors_are_actionable_with_a_real_browser() { @@ -295,6 +551,12 @@ fn search_popup_can_be_selected_across_cli_invocations_and_closed_safely() { "--selector", "#search", ]); + // Tab count alone is insufficient: headless-shell can allow untrusted popups. + assert_eq!( + observer.evaluate("window.clickEvidence").unwrap(), + json!({"trusted":true,"active":true,"opened":true}), + "click must deliver real browser input, not HTMLElement.click()" + ); let deadline = Instant::now() + Duration::from_secs(10); let result = loop { let targets = pages(&mut observer);