diff --git a/crates/obscura-cdp/src/domains/dom.rs b/crates/obscura-cdp/src/domains/dom.rs index ed9e1b4..a61bf8d 100644 --- a/crates/obscura-cdp/src/domains/dom.rs +++ b/crates/obscura-cdp/src/domains/dom.rs @@ -1,8 +1,36 @@ +use obscura_browser::Page; use obscura_dom::{DomTree, NodeData, NodeId}; use serde_json::{json, Value}; use crate::dispatch::CdpContext; +/// Resolve a DOM `nodeId` from CDP params. Honors `nodeId`, `backendNodeId`, +/// and `objectId` in that order. Playwright commonly passes only `objectId` +/// (returned by a prior `DOM.resolveNode`); without this fallback those +/// requests silently default to node 0 and click the wrong element. +fn resolve_node_id(page: &mut Page, params: &Value) -> Result { + if let Some(nid) = params.get("nodeId").and_then(|v| v.as_u64()) { + return Ok(nid); + } + if let Some(nid) = params.get("backendNodeId").and_then(|v| v.as_u64()) { + return Ok(nid); + } + if let Some(oid) = params.get("objectId").and_then(|v| v.as_str()) { + let code = format!( + "(function() {{ var o = globalThis.__obscura_objects && globalThis.__obscura_objects['{}']; \ + return (o && typeof o._nid === 'number') ? o._nid : -1; }})()", + oid.replace('\'', "\\'") + ); + let result = page.evaluate(&code); + let nid = result.as_f64().map(|n| n as i64).unwrap_or(-1); + if nid < 0 { + return Err(format!("objectId {oid} could not be resolved to a node")); + } + return Ok(nid as u64); + } + Err("nodeId, backendNodeId, or objectId required".to_string()) +} + pub async fn handle( method: &str, params: &Value, @@ -138,10 +166,8 @@ pub async fn handle( "setAttributeValue" => Ok(json!({})), "removeNode" => Ok(json!({})), "getBoxModel" => { - let node_id = params.get("nodeId").and_then(|v| v.as_u64()) - .or_else(|| params.get("backendNodeId").and_then(|v| v.as_u64())) - .unwrap_or(0); let page = ctx.get_session_page_mut(session_id).ok_or("No page")?; + let node_id = resolve_node_id(page, params)?; let code = format!( "(function() {{\ var el = globalThis._wrap && globalThis._wrap({0});\ @@ -175,10 +201,8 @@ pub async fn handle( })) } "getContentQuads" => { - let node_id = params.get("nodeId").and_then(|v| v.as_u64()) - .or_else(|| params.get("backendNodeId").and_then(|v| v.as_u64())) - .unwrap_or(0); let page = ctx.get_session_page_mut(session_id).ok_or("No page")?; + let node_id = resolve_node_id(page, params)?; let code = format!( "(function() {{\ var el = globalThis._wrap && globalThis._wrap({0});\ diff --git a/crates/obscura-cdp/src/domains/page.rs b/crates/obscura-cdp/src/domains/page.rs index a3d9145..9b26601 100644 --- a/crates/obscura-cdp/src/domains/page.rs +++ b/crates/obscura-cdp/src/domains/page.rs @@ -313,6 +313,17 @@ pub async fn handle( .to_string(), ) } + "captureScreenshot" | "captureSnapshot" => { + // Same story as printToPDF: rasterising a page needs a layout and + // paint pipeline that Obscura intentionally does not have. Reply + // with a clear error so clients can fail fast instead of waiting + // on the generic "Unknown Page method" reply. + Err(format!( + "Page.{method} is not supported by Obscura: no layout or paint engine. \ + For visual snapshots, drive a real headless Chromium for the \ + screenshot leg of your pipeline and use Obscura for the scraping leg." + )) + } _ => Err(format!("Unknown Page method: {}", method)), } } @@ -398,4 +409,32 @@ mod tests { "error must point to a workaround: {err}" ); } + + /// Regression for #45: same idea as printToPDF for captureScreenshot. + /// Playwright's `page.screenshot()` calls Page.captureScreenshot via CDP; + /// without an explicit arm, clients see "Unknown Page method" and have + /// no idea why their screenshot request failed. + #[tokio::test] + async fn capture_screenshot_returns_descriptive_unsupported_error() { + let mut ctx = CdpContext::new(); + let err = handle("captureScreenshot", &json!({}), &mut ctx, &None) + .await + .expect_err("captureScreenshot must error until a real paint exists"); + assert!( + !err.contains("Unknown Page method"), + "captureScreenshot must NOT fall through to the catch-all: {err}" + ); + assert!( + err.contains("not supported by Obscura"), + "error must clearly state screenshot is unsupported: {err}" + ); + // Same for the MHTML snapshot sibling method. + let err2 = handle("captureSnapshot", &json!({}), &mut ctx, &None) + .await + .expect_err("captureSnapshot must error until a real renderer exists"); + assert!( + !err2.contains("Unknown Page method"), + "captureSnapshot must NOT fall through: {err2}" + ); + } } diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index 2d85610..d2a9d59 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -746,6 +746,29 @@ class Element extends Node { return {x:8,y:8,width:100,height:20,top:8,right:108,bottom:28,left:8,toJSON(){return this;}}; } getClientRects() { return [this.getBoundingClientRect()]; } + // No layout engine: a stub that always returns true unblocks Playwright's + // actionability polling. With a real layout we'd check display, visibility, + // opacity and rect dimensions per spec. + checkVisibility(opts) { return true; } + // ARIA reflection properties. Without an accessibility tree we expose the + // raw aria-* attributes so Playwright's getByRole / getByLabel locators can + // at least find elements that author them explicitly. + get role() { return this.getAttribute('role'); } + set role(v) { if (v == null) this.removeAttribute('role'); else this.setAttribute('role', String(v)); } + get ariaLabel() { return this.getAttribute('aria-label'); } + set ariaLabel(v) { if (v == null) this.removeAttribute('aria-label'); else this.setAttribute('aria-label', String(v)); } + get ariaRoleDescription() { return this.getAttribute('aria-roledescription'); } + set ariaRoleDescription(v) { if (v == null) this.removeAttribute('aria-roledescription'); else this.setAttribute('aria-roledescription', String(v)); } + get ariaChecked() { return this.getAttribute('aria-checked'); } + set ariaChecked(v) { if (v == null) this.removeAttribute('aria-checked'); else this.setAttribute('aria-checked', String(v)); } + get ariaDisabled() { return this.getAttribute('aria-disabled'); } + set ariaDisabled(v) { if (v == null) this.removeAttribute('aria-disabled'); else this.setAttribute('aria-disabled', String(v)); } + get ariaExpanded() { return this.getAttribute('aria-expanded'); } + set ariaExpanded(v) { if (v == null) this.removeAttribute('aria-expanded'); else this.setAttribute('aria-expanded', String(v)); } + get ariaHidden() { return this.getAttribute('aria-hidden'); } + set ariaHidden(v) { if (v == null) this.removeAttribute('aria-hidden'); else this.setAttribute('aria-hidden', String(v)); } + get ariaSelected() { return this.getAttribute('aria-selected'); } + set ariaSelected(v) { if (v == null) this.removeAttribute('aria-selected'); else this.setAttribute('aria-selected', String(v)); } scrollIntoView() { globalThis.__obscura_click_target = this; } animate(keyframes, options) { const duration = typeof options === 'number' ? options : (options?.duration || 0); @@ -2164,6 +2187,7 @@ globalThis.Range = class Range { setStart(){} setEnd(){} collapse(){} selectNode Element.prototype.getElementsByTagName, Element.prototype.getElementsByClassName, Element.prototype.matches, Element.prototype.closest, Element.prototype.getBoundingClientRect, Element.prototype.getClientRects, + Element.prototype.checkVisibility, Element.prototype.addEventListener, Element.prototype.removeEventListener, Element.prototype.dispatchEvent, Element.prototype.click, Element.prototype.focus, Element.prototype.blur, diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index 8b5b527..c7df9eb 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -1811,4 +1811,66 @@ mod tests { ); } + // ── Issue #45 (Playwright actionability) regression tests ──────────────── + // Kept at the end of the module so they don't share textual context with + // unrelated test additions in other branches (avoids spurious merge + // conflicts when both this branch and an unrelated bootstrap.js change + // add tests near the start of `mod tests`). + + /// Playwright >= 1.25 calls `element.checkVisibility(...)` before every + /// input event. If the method isn't defined Playwright retries until its + /// action timeout fires. Without a layout engine we can't compute it + /// properly, so the stub always returns true — still strictly better + /// than the undefined path. + #[test] + fn element_check_visibility_is_callable() { + let mut rt = setup_runtime(r#"
x
"#); + let result = rt + .evaluate("document.getElementById('x').checkVisibility({checkOpacity: true})") + .unwrap(); + assert_eq!(result, serde_json::json!(true)); + + let typeof_method = rt + .evaluate("typeof document.getElementById('x').checkVisibility") + .unwrap(); + assert_eq!(typeof_method, serde_json::json!("function")); + } + + /// Playwright's `getByRole` / `getByLabel` locators resolve via ARIA + /// reflection properties. Without the getters those locators always + /// fail. Reflect the underlying aria-* attributes. + #[test] + fn element_aria_reflection_properties_read_aria_attrs() { + let mut rt = setup_runtime( + r#""#, + ); + let result = rt + .evaluate( + r#" + const el = document.getElementById('b'); + return [el.role, el.ariaLabel, el.ariaSelected]; + "#, + ) + .unwrap(); + assert_eq!(result, serde_json::json!(["tab", "Settings", "true"])); + } + + /// Setting an ARIA reflection property must write through to the + /// underlying attribute so frameworks that toggle state via + /// `el.ariaExpanded = 'true'` actually update the DOM. + #[test] + fn element_aria_reflection_setters_write_through() { + let mut rt = setup_runtime(r#"
"#); + let result = rt + .evaluate( + r#" + const el = document.getElementById('d'); + el.role = 'menu'; + el.ariaExpanded = 'true'; + return [el.getAttribute('role'), el.getAttribute('aria-expanded')]; + "#, + ) + .unwrap(); + assert_eq!(result, serde_json::json!(["menu", "true"])); + } }