fix(cdp,js): playwright actionability stubs and objectId resolve (#178)
Playwright fails locator actions on Obscura for two avoidable reasons that are separate from the missing layout engine. Both have trivial fixes that unblock the actionability polling loop. - Element.checkVisibility(opts) was undefined. Playwright >= 1.25 calls it before every input event and retries until its action timeout if the method isn't a function. Add a stub that returns true. Without a layout pass we can't compute it for real, but a permissive stub still beats the undefined-function path. - ARIA reflection properties were missing on Element (role, ariaLabel, ariaSelected, ariaExpanded, ariaHidden, ariaChecked, ariaDisabled, ariaRoleDescription). Playwright's getByRole and getByLabel locators read them. Reflect through to the underlying aria-* attributes for both read and write. - DOM.getBoxModel and DOM.getContentQuads ignored the objectId param. Playwright passes only objectId (from a prior DOM.resolveNode), and the handlers silently defaulted to node 0, so dispatchMouseEvent always clicked the wrong element. Extract a resolve_node_id helper that honors nodeId, backendNodeId, and objectId in that order. - Page.captureScreenshot and Page.captureSnapshot now return a clear "not supported, use a real Chromium for the screenshot leg" error instead of the generic "Unknown Page method" fallback. Matches the pattern already in place for Page.printToPDF. Partial fix for #45. The architectural blocker (no layout engine, so multi-element click coordinates are wrong) is unchanged.
This commit is contained in:
1 parent
3d12f76d8e
commit
a3b21bbb70
4 files changed
+155
-6
No files matched your search
@@ -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<u64, String> {
|
||||
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});\
|
||||
|
||||
@@ -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}"
|
||||
);
|
||||
}
|
||||
}
|
||||
Vendored
+24
@@ -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,
|
||||
|
||||
@@ -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#"<div id="x">x</div>"#);
|
||||
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#"<button id="b" role="tab" aria-label="Settings" aria-selected="true">x</button>"#,
|
||||
);
|
||||
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#"<div id="d"></div>"#);
|
||||
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"]));
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user