diff --git a/crates/ff-rdp-cli/src/cli/args.rs b/crates/ff-rdp-cli/src/cli/args.rs index f16a03a..2d70084 100644 --- a/crates/ff-rdp-cli/src/cli/args.rs +++ b/crates/ff-rdp-cli/src/cli/args.rs @@ -59,6 +59,7 @@ COMMAND REFERENCE: Accessibility: ff-rdp a11y [--depth N] [--selector SEL] [--interactive] [--critical] + ff-rdp a11y --native # opt in to Firefox's platform accessibility tree (meta.source tells you which tree you got) ff-rdp a11y contrast [--selector SEL] [--fail-only] # total=results, sampled=elements checked ff-rdp a11y summary @@ -153,6 +154,8 @@ COOKBOOK: ff-rdp a11y contrast --fail-only # total = AA failures; sampled = elements checked ff-rdp a11y contrast --fail-only --jq '{total, shown: (.results|length), sampled}' ff-rdp a11y --interactive --jq '[.. | select(.role? == \"link\") | .name]' + ff-rdp a11y --jq '.meta.source' # \"native\" or \"js-fallback\" — always present + ff-rdp a11y --native --jq '.result.role' # opt in to the real platform tree ([\"document\", ...]) # DOM and CSS inspection ff-rdp dom \"a[href]\" --text-attrs @@ -788,10 +791,11 @@ With --key: {\"results\": {\"key\": \"...\", \"value\": \"...\"}, \"total\": 1, /// Inspect accessibility tree and check WCAG compliance #[command(long_about = "Inspect accessibility tree and check WCAG compliance. -Output: {\"results\": {\"role\": \"...\", \"name\": \"...\", \"children\": [...]}, \"total\": 1, \"meta\": {...}} +Output: {\"results\": {\"role\": \"...\", \"name\": \"...\", \"children\": [...]}, \"total\": 1, \"meta\": {..., \"source\": \"native\"|\"js-fallback\"}} With a11y summary: {\"results\": [{\"role\": \"...\", \"name\": \"...\", \"level\": N}], \"total\": N, \"meta\": {...}} -With a11y contrast: {\"results\": [{\"selector\": \"...\", \"ratio\": N, \"aa_normal\": bool, ...}], \"total\": N, \"sampled\": M, \"meta\": {...}} - a11y contrast `total` = returned results (AA failures under --fail-only, else all checks); `sampled` = elements examined. Pre-iter-127 `total` reported the sample size.")] +With a11y contrast: {\"results\": [{\"selector\": \"...\", \"ratio\": N, \"aa_normal\": bool, ...}], \"total\": N, \"sampled\": M, \"meta\": {..., \"source\": \"js-fallback\"}} + a11y contrast `total` = returned results (AA failures under --fail-only, else all checks); `sampled` = elements examined. Pre-iter-127 `total` reported the sample size. + `meta.source` (iter-143) is always present on a11y/a11y --critical/a11y contrast: \"native\" means the real Firefox platform accessibility tree (roles like \"document\"/\"paragraph\"); \"js-fallback\" means a DOM-derived approximation (roles like \"generic\"), with `meta.source_reason` naming why. `a11y --native` opts in to the native tree (never the default — DEC-027) by enabling Firefox's accessibility service for the duration of the call.")] A11y(A11yArgs), /// Reload the page #[command(long_about = "Reload the page. @@ -1794,6 +1798,17 @@ pub struct A11yArgs { /// instead of the full accessibility tree; empty when nothing critical. #[arg(long, conflicts_with = "interactive")] pub critical: bool, + /// Opt in to the native platform accessibility tree: enables Firefox's + /// accessibility service (if not already running), walks the real + /// platform tree (roles like "document"/"paragraph" instead of the + /// JS-derived "generic"), then restores the service to its previous + /// state if this command was the one that turned it on. This is a + /// browser-global, process-wide change while it runs — never the + /// default (DEC-027) — and cannot be combined with `--selector`/`--ref`, + /// which always use the JS-derived path. Failure to enable surfaces as + /// an explicit error, never a silent fallback. + #[arg(long, conflicts_with_all = ["selector", "ref_id"])] + pub native: bool, } #[derive(clap::Args)] diff --git a/crates/ff-rdp-cli/src/commands/a11y.rs b/crates/ff-rdp-cli/src/commands/a11y.rs index e5f8ef4..c0b3a0b 100644 --- a/crates/ff-rdp-cli/src/commands/a11y.rs +++ b/crates/ff-rdp-cli/src/commands/a11y.rs @@ -1,5 +1,8 @@ +use std::time::Duration; + use ff_rdp_core::{ - AccessibilityActor, AccessibleNode, ActorId, WebConsoleActor, filter_interactive, + AccessibilityActor, AccessibleNode, ActorId, ProtocolError, RootActor, WebConsoleActor, + filter_interactive, }; use serde_json::{Value, json}; @@ -13,12 +16,69 @@ use crate::output_pipeline::OutputPipeline; use super::connect_tab::{ConnectedTab, connect_direct}; use super::js_helpers::{escape_selector, eval_or_bail, resolve_result}; +/// Which tree an `a11y` response came from (iter-143 Theme A). +/// +/// Reported unconditionally in `meta.source` via +/// [`crate::connection_meta::merge_source`] — see [DEC-027] and +/// `kb/iterations/iteration-143-native-a11y-tree.md`. +/// +/// [DEC-027]: ../../../../kb/decision-log.md +enum A11ySource { + /// The real Firefox platform accessibility tree (roles like `document`, + /// `paragraph`, `link`). + Native, + /// A DOM-derived approximation (roles like `generic`) built by evaluating + /// JS in the page — cannot see anything the platform computes but the DOM + /// does not expose. `reason` names why this path ran instead of native. + JsFallback(&'static str), +} + +impl A11ySource { + fn merge_into(&self, meta: &mut Value) { + match self { + Self::Native => crate::connection_meta::merge_source(meta, "native", None), + Self::JsFallback(reason) => { + crate::connection_meta::merge_source(meta, "js-fallback", Some(reason)); + } + } + } +} + +/// Purpose-specific ceiling for accessibility walker requests (iter-143 Theme +/// C). The walker's root accessor stalls — it does not error — while the +/// platform accessibility service is off (iter-136): Firefox's +/// `document-ready` promise never settles. `run_native_or_js_fallback` +/// already checks `bootstrap().state.enabled` first, but a race (the service +/// getting disabled between that check and the walk) or a future call site +/// that skips the check would otherwise stall for the full `--timeout` +/// (default 10s, but user-configurable much higher). Bounding walker requests +/// to this instead means a mistake costs a few seconds, not the caller's +/// whole configured timeout. +const A11Y_WALKER_TIMEOUT: Duration = Duration::from_secs(3); + +/// Run `f` with the transport's read timeout temporarily narrowed to +/// [`A11Y_WALKER_TIMEOUT`], restoring the previous value afterwards +/// regardless of whether `f` succeeded. +fn with_walker_timeout( + ctx: &mut ConnectedTab, + f: impl FnOnce(&mut ff_rdp_core::RdpTransport) -> Result, +) -> Result { + let previous = ctx.transport_mut().read_timeout().unwrap_or(None); + let _ = ctx + .transport_mut() + .set_read_timeout(Some(A11Y_WALKER_TIMEOUT)); + let result = f(ctx.transport_mut()); + let _ = ctx.transport_mut().set_read_timeout(previous); + result +} + pub fn run( cli: &Cli, depth: u32, max_chars: u32, selector: Option<&str>, interactive: bool, + native: bool, ) -> Result<(), AppError> { let mut ctx = connect_direct(cli)?; @@ -30,8 +90,22 @@ pub fn run( })?; // If selector is provided, use JS eval approach (similar to snapshot). - let (tree, used_js_fallback) = if let Some(sel) = selector { - (run_selector_mode(&mut ctx, sel, depth, max_chars)?, false) + // `--native` conflicts with `--selector`/`--ref` at the clap level (both + // are inherently JS-derived paths — there is no native "root at + // selector" primitive), so at most one of these two branches applies. + let (tree, source) = if let Some(sel) = selector { + ( + run_selector_mode(&mut ctx, sel, depth, max_chars)?, + A11ySource::JsFallback("selector-mode"), + ) + } else if native { + // Theme B: explicit opt-in to the platform tree. Never silently + // falls back — any failure (enable failing, bootstrap still + // reporting disabled, a stalled/erroring walker request) surfaces as + // an explicit error. + let (tree, _we_enabled) = + run_native_opt_in(&mut ctx, &accessibility_actor, depth, max_chars, cli)?; + (tree, A11ySource::Native) } else { // Use native RDP protocol with JS eval fallback for Firefox 149+ where // both `getDocument` and `getRootNode` are unrecognized on the walker. @@ -76,7 +150,16 @@ pub fn run( // agent can tell how this command executed without a // separate `daemon status` round-trip. crate::connection_meta::merge_route(&mut meta, ctx.via_daemon); - if used_js_fallback && let Some(m) = meta.as_object_mut() { + // iter-143 Theme A: always present — the only way a caller can tell + // which tree it is scoring without a separate --verbose round-trip. + source.merge_into(&mut meta); + // Legacy fields kept for existing consumers: only set for an *automatic* + // fallback (the caller asked for the native tree and didn't get it), not + // for `--selector`, which is always JS-derived by design. + if let A11ySource::JsFallback(reason) = &source + && *reason != "selector-mode" + && let Some(m) = meta.as_object_mut() + { m.insert("fallback".to_string(), json!(true)); m.insert("fallback_method".to_string(), json!("js-eval")); } @@ -159,7 +242,7 @@ fn run_native_or_js_fallback( depth: u32, max_chars: u32, cli: &Cli, -) -> Result<(AccessibleNode, bool), AppError> { +) -> Result<(AccessibleNode, A11ySource), AppError> { // Step 0: the native walker only answers while the platform accessibility // service is running; with it off, the root accessor stalls until the // socket read timeout instead of erroring (iter-136). Check first and take @@ -170,10 +253,12 @@ fn run_native_or_js_fallback( if cli.is_verbose() { eprintln!( "debug: platform accessibility service is disabled; falling back to JS eval \ - (enable it in Firefox to get the native accessibility tree)" + (enable it in Firefox to get the native accessibility tree, or pass --native \ + to opt in for this command)" ); } - return run_selector_mode(ctx, "body", depth, max_chars).map(|t| (t, true)); + return run_selector_mode(ctx, "body", depth, max_chars) + .map(|t| (t, A11ySource::JsFallback("accessibility-service-disabled"))); } // Older Firefox without `bootstrap` on the accessibility actor: try the // native path anyway. @@ -181,8 +266,12 @@ fn run_native_or_js_fallback( Err(e) => return Err(map_a11y_error(e, cli)), } - // Step 1: try to get the walker. - let walker = match AccessibilityActor::get_walker(ctx.transport_mut(), accessibility_actor) { + // Step 1: try to get the walker. Bounded to A11Y_WALKER_TIMEOUT (Theme C) + // so a race with the service being disabled after the check above stalls + // for seconds, not the full configured --timeout. + let walker = match with_walker_timeout(ctx, |t| { + AccessibilityActor::get_walker(t, accessibility_actor) + }) { Ok(w) => w, Err(e) if e.is_unrecognized_packet_type() => { if cli.is_verbose() { @@ -191,13 +280,25 @@ fn run_native_or_js_fallback( falling back to JS eval" ); } - return run_selector_mode(ctx, "body", depth, max_chars).map(|t| (t, true)); + return run_selector_mode(ctx, "body", depth, max_chars) + .map(|t| (t, A11ySource::JsFallback("walker-unrecognized"))); + } + Err(ProtocolError::Timeout) => { + if cli.is_verbose() { + eprintln!( + "debug: accessibility getWalker timed out after {}s (bounded deadline, \ + iter-143); falling back to JS eval", + A11Y_WALKER_TIMEOUT.as_secs() + ); + } + return run_selector_mode(ctx, "body", depth, max_chars) + .map(|t| (t, A11ySource::JsFallback("walker-timeout"))); } Err(e) => return Err(map_a11y_error(e, cli)), }; // Step 2: try to get the root node via the walker. - let root = match AccessibilityActor::get_root(ctx.transport_mut(), &walker) { + let root = match with_walker_timeout(ctx, |t| AccessibilityActor::get_root(t, &walker)) { Ok(r) => r, Err(e) if e.is_unrecognized_packet_type() => { // Both getDocument and getRootNode failed — Firefox 149+ protocol change. @@ -207,15 +308,145 @@ fn run_native_or_js_fallback( version (tried getDocument and getRootNode); falling back to JS eval" ); } - return run_selector_mode(ctx, "body", depth, max_chars).map(|t| (t, true)); + return run_selector_mode(ctx, "body", depth, max_chars) + .map(|t| (t, A11ySource::JsFallback("root-unrecognized"))); + } + Err(ProtocolError::Timeout) => { + if cli.is_verbose() { + eprintln!( + "debug: accessibility walker root request timed out after {}s (bounded \ + deadline, iter-143); falling back to JS eval", + A11Y_WALKER_TIMEOUT.as_secs() + ); + } + return run_selector_mode(ctx, "body", depth, max_chars) + .map(|t| (t, A11ySource::JsFallback("root-timeout"))); } Err(e) => return Err(map_a11y_error(e, cli)), }; // Step 3: walk the tree with the native protocol. - AccessibilityActor::walk_tree(ctx.transport_mut(), &walker, &root, depth, max_chars) - .map(|t| (t, false)) - .map_err(|e| map_a11y_error(e, cli)) + with_walker_timeout(ctx, |t| { + AccessibilityActor::walk_tree(t, &walker, &root, depth, max_chars) + }) + .map(|t| (t, A11ySource::Native)) + .map_err(|e| map_a11y_error(e, cli)) +} + +/// Opt-in native tree walk (iter-143 Theme B): enables the platform +/// accessibility service via the root actor's `parentAccessibilityActor` when +/// it is not already running, walks the native tree, then restores the +/// previous state afterward if — and only if — this call was the one that +/// turned it on (DEC-027: ff-rdp never leaves behind a browser-global +/// mutation the caller did not ask for, and never touches state it did not +/// create). +/// +/// Unlike [`run_native_or_js_fallback`], this never falls back to the +/// JS-derived tree: a caller who passed `--native` asked for the platform +/// tree specifically, so any failure — `enable` failing, `bootstrap` still +/// reporting disabled after `enable`, or a stalled/erroring walker request — +/// surfaces as an explicit error instead of a silent substitution. +/// +/// Returns the tree and whether this call enabled the service (`we_enabled`) +/// — surfaced so tests and `--verbose` diagnostics can confirm restoration. +fn run_native_opt_in( + ctx: &mut ConnectedTab, + accessibility_actor: &ActorId, + depth: u32, + max_chars: u32, + cli: &Cli, +) -> Result<(AccessibleNode, bool), AppError> { + let root_form = RootActor::get_root(ctx.transport_mut()).map_err(|e| map_a11y_error(e, cli))?; + let parent_actor: ActorId = root_form + .get("parentAccessibilityActor") + .and_then(Value::as_str) + .ok_or_else(|| { + AppError::User( + "--native: this Firefox's root actor does not expose \ + 'parentAccessibilityActor' — cannot enable the platform accessibility \ + service remotely. Omit --native to use the JS-derived fallback." + .to_string(), + ) + })? + .into(); + + let was_enabled = + AccessibilityActor::is_service_enabled(ctx.transport_mut(), accessibility_actor) + .map_err(|e| map_a11y_error(e, cli))?; + + let we_enabled = if was_enabled { + false + } else { + AccessibilityActor::enable_service(ctx.transport_mut(), &parent_actor).map_err(|e| { + AppError::User(format!( + "--native: failed to enable the platform accessibility service via \ + parentAccessibilityActor.enable(): {e}" + )) + })?; + let now_enabled = + AccessibilityActor::is_service_enabled(ctx.transport_mut(), accessibility_actor) + .map_err(|e| map_a11y_error(e, cli))?; + if !now_enabled { + return Err(AppError::User( + "--native: called parentAccessibilityActor.enable() but bootstrap() still \ + reports the accessibility service as disabled — refusing to walk the native \ + tree rather than silently falling back. This may mean another consumer \ + immediately disabled it, or this Firefox build doesn't honor a remote enable()." + .to_string(), + )); + } + if cli.is_verbose() { + eprintln!( + "debug: --native: platform accessibility service was off; enabled it for this \ + command and will restore it to disabled afterward" + ); + } + true + }; + + let walk_result = walk_native_tree_bounded(ctx, accessibility_actor, depth, max_chars, cli); + + if we_enabled { + // Best-effort restore: report a failure but don't let it mask the + // primary result. On Windows an active screen reader can block + // `disable` (kb/rdp/actors/accessibility.md) — that's an expected + // limitation, not a bug in ff-rdp. + if let Err(e) = AccessibilityActor::disable_service(ctx.transport_mut(), &parent_actor) { + if cli.is_verbose() { + eprintln!( + "debug: --native: failed to restore the accessibility service to disabled \ + after this opt-in run: {e}" + ); + } + } else if cli.is_verbose() { + eprintln!("debug: --native: restored the accessibility service to disabled"); + } + } + + walk_result.map(|tree| (tree, we_enabled)) +} + +/// Walk the native accessibility tree (walker → root → recursive children), +/// with each step bounded by [`A11Y_WALKER_TIMEOUT`] (Theme C). Shared by +/// [`run_native_opt_in`]; unlike [`run_native_or_js_fallback`] there is no +/// fallback branch here — every error maps straight to an [`AppError`]. +fn walk_native_tree_bounded( + ctx: &mut ConnectedTab, + accessibility_actor: &ActorId, + depth: u32, + max_chars: u32, + cli: &Cli, +) -> Result { + let walker = with_walker_timeout(ctx, |t| { + AccessibilityActor::get_walker(t, accessibility_actor) + }) + .map_err(|e| map_a11y_error(e, cli))?; + let root = with_walker_timeout(ctx, |t| AccessibilityActor::get_root(t, &walker)) + .map_err(|e| map_a11y_error(e, cli))?; + with_walker_timeout(ctx, |t| { + AccessibilityActor::walk_tree(t, &walker, &root, depth, max_chars) + }) + .map_err(|e| map_a11y_error(e, cli)) } /// Selector-based subtree extraction via JS eval. @@ -308,6 +539,13 @@ fn parse_js_a11y_tree(value: &Value) -> Option { /// Map protocol errors to user-friendly messages. fn map_a11y_error(err: ff_rdp_core::ProtocolError, cli: &Cli) -> AppError { match &err { + ProtocolError::Timeout => AppError::User( + "accessibility request timed out waiting for a reply from Firefox. If this \ + happened while walking the tree, the platform accessibility service is likely \ + off — Firefox's walker never replies in that case (iter-136). Check \ + `a11y --jq '.meta.source'`, or omit --native to use the JS-derived fallback." + .to_string(), + ), ff_rdp_core::ProtocolError::ActorError { error, .. } if error == "noSuchActor" || error == "unknownActor" => { @@ -434,6 +672,11 @@ pub fn run_critical(cli: &Cli, root_selector: Option<&str>) -> Result<(), AppErr // agent can tell how this command executed without a // separate `daemon status` round-trip. crate::connection_meta::merge_route(&mut meta, ctx.via_daemon); + // iter-143 Theme A: `--critical` has no native-tree equivalent — the + // platform accessibility service doesn't expose a WCAG-critical severity + // — so this is always JS-derived. Reported for consistency with the + // plain `a11y` tree's `meta.source` rather than as an actual fallback. + crate::connection_meta::merge_source(&mut meta, "js-fallback", Some("critical-audit-js-only")); let controls = OutputControls::from_cli(cli, SortDir::Asc); let mut items = violations; diff --git a/crates/ff-rdp-cli/src/commands/a11y_contrast.rs b/crates/ff-rdp-cli/src/commands/a11y_contrast.rs index 152e460..ccfc3ff 100644 --- a/crates/ff-rdp-cli/src/commands/a11y_contrast.rs +++ b/crates/ff-rdp-cli/src/commands/a11y_contrast.rs @@ -63,6 +63,10 @@ pub fn run(cli: &Cli, selector: Option<&str>, fail_only: bool) -> Result<(), App // agent can tell how this command executed without a // separate `daemon status` round-trip. crate::connection_meta::merge_route(&mut meta, ctx.via_daemon); + // iter-143 Theme A: contrast checking is always DOM/computed-style based + // — there is no native-actor equivalent — so this is always + // "js-fallback". Reported for consistency with `a11y`'s `meta.source`. + crate::connection_meta::merge_source(&mut meta, "js-fallback", Some("contrast-audit-js-only")); // Apply output controls (sort, limit, fields). let controls = OutputControls::from_cli(cli, SortDir::Desc); diff --git a/crates/ff-rdp-cli/src/connection_meta.rs b/crates/ff-rdp-cli/src/connection_meta.rs index d88998e..6fedfdd 100644 --- a/crates/ff-rdp-cli/src/connection_meta.rs +++ b/crates/ff-rdp-cli/src/connection_meta.rs @@ -64,6 +64,25 @@ pub fn merge_route(meta: &mut Value, via_daemon: bool) { } } +/// Merge `meta.source` — `"native"` or `"js-fallback"` — into `meta` +/// (iter-143 Theme A). +/// +/// Same unconditional treatment as [`merge_route`]: a caller scoring +/// accessibility output needs to know which tree it is looking at — the +/// native platform tree (real accessible roles like `document`/`paragraph`) +/// or the DOM-derived approximation (`generic`, …) — without a separate +/// `--verbose` round-trip. `reason` is populated only when `source` is +/// `"js-fallback"`, naming why the native path was not used (e.g. +/// `"accessibility-service-disabled"`, `"selector-mode"`). +pub fn merge_source(meta: &mut Value, source: &str, reason: Option<&str>) { + if let Some(obj) = meta.as_object_mut() { + obj.insert("source".to_string(), Value::String(source.to_owned())); + if let Some(r) = reason { + obj.insert("source_reason".to_string(), Value::String(r.to_owned())); + } + } +} + fn cached_owner(host: &str, port: u16) -> Option { // Only cache for loopback hosts. A remote port would require a different // lookup strategy entirely; we just skip the cache for those. @@ -240,6 +259,35 @@ mod tests { assert_eq!(meta["route"], "direct"); } + /// iter-143 Theme A: `merge_source` sets `source` unconditionally and + /// omits `source_reason` when none is given (the native path). + #[test] + fn merge_source_native_omits_reason() { + let mut meta = json!({}); + merge_source(&mut meta, "native", None); + assert_eq!(meta["source"], "native"); + assert!(meta.get("source_reason").is_none()); + } + + #[test] + fn merge_source_js_fallback_includes_reason() { + let mut meta = json!({}); + merge_source( + &mut meta, + "js-fallback", + Some("accessibility-service-disabled"), + ); + assert_eq!(meta["source"], "js-fallback"); + assert_eq!(meta["source_reason"], "accessibility-service-disabled"); + } + + #[test] + fn merge_source_makes_meta_non_empty() { + let mut meta = json!({}); + merge_source(&mut meta, "native", None); + assert!(!is_meta_empty(&meta), "meta with source must not be empty"); + } + #[test] fn merge_route_makes_meta_non_empty() { // Route must be visible in DEFAULT (non-verbose) output — merging it diff --git a/crates/ff-rdp-cli/src/dispatch.rs b/crates/ff-rdp-cli/src/dispatch.rs index f23539d..ccff1a2 100644 --- a/crates/ff-rdp-cli/src/dispatch.rs +++ b/crates/ff-rdp-cli/src/dispatch.rs @@ -749,6 +749,7 @@ fn dispatch_inner( ref_id, interactive, critical, + native, }) => { let resolved_selector: Option = if let Some(id) = ref_id.as_deref() { Some(resolve_ref_via_daemon(cli, id)?) @@ -772,6 +773,7 @@ fn dispatch_inner( *max_chars, effective_selector, *interactive, + *native, ) } } diff --git a/crates/ff-rdp-cli/tests/e2e/a11y.rs b/crates/ff-rdp-cli/tests/e2e/a11y.rs index ab86cf3..b266c37 100644 --- a/crates/ff-rdp-cli/tests/e2e/a11y.rs +++ b/crates/ff-rdp-cli/tests/e2e/a11y.rs @@ -79,6 +79,141 @@ fn a11y_legacy_root_server() -> MockRdpServer { .on("getDocument", load_fixture("a11y_get_root_response.json")) } +/// Build a mock server for a `bootstrap`-disabled Firefox: `a11y` must take +/// the JS-eval fallback path (iter-143 Theme A). +fn a11y_disabled_service_server() -> MockRdpServer { + MockRdpServer::new() + .on("listTabs", load_fixture("list_tabs_response.json")) + .on("getTarget", load_fixture("get_target_response.json")) + .on( + "bootstrap", + serde_json::json!({ + "from": "server1.conn0.child2/accessibilityActor12", + "state": {"enabled": false} + }), + ) + .on_with_followup( + "evaluateJSAsync", + load_fixture("eval_immediate_response.json"), + serde_json::json!({ + "from": "server1.conn0.child2/consoleActor3", + "type": "evaluationResult", + "resultID": "1775437183977.373-0", + "hasException": false, + "result": "__FF_RDP_JSON__{\"role\":\"document\",\"children\":[{\"role\":\"generic\",\"name\":\"body\"}]}", + "timestamp": 1_775_437_183_980.721 + }), + ) +} + +/// Build a mock server for `a11y --native` where the service is already +/// enabled: same protocol flow as [`a11y_server`] but with `getRoot` +/// exposing `parentAccessibilityActor` (needed even when the service is +/// already on, since `run_native_opt_in` always locates it first). +fn a11y_native_already_enabled_server() -> MockRdpServer { + MockRdpServer::new() + .on("listTabs", load_fixture("list_tabs_response.json")) + .on("getTarget", load_fixture("get_target_response.json")) + .on( + "getRoot", + serde_json::json!({ + "from": "root", + "parentAccessibilityActor": "server1.conn0.parentAccessibilityActor6" + }), + ) + .on("bootstrap", load_fixture("a11y_bootstrap_response.json")) + .on("getWalker", load_fixture("a11y_get_walker_response.json")) + .on_sequence( + "children", + vec![ + (load_fixture("a11y_walker_children_response.json"), vec![]), + (load_fixture("a11y_children_response.json"), vec![]), + (load_fixture("a11y_children_empty_response.json"), vec![]), + ], + ) +} + +/// Build a mock server for `a11y --native` where the service starts +/// *disabled*: `run_native_opt_in` must call `enable` on +/// `parentAccessibilityActor`, see `bootstrap` flip to enabled, walk the +/// tree, then call `disable` to restore the prior state. +fn a11y_native_opt_in_server() -> MockRdpServer { + MockRdpServer::new() + .on("listTabs", load_fixture("list_tabs_response.json")) + .on("getTarget", load_fixture("get_target_response.json")) + .on( + "getRoot", + serde_json::json!({ + "from": "root", + "parentAccessibilityActor": "server1.conn0.parentAccessibilityActor6" + }), + ) + .on_sequence( + "bootstrap", + vec![ + ( + serde_json::json!({ + "from": "server1.conn0.child2/accessibilityActor12", + "state": {"enabled": false} + }), + vec![], + ), + ( + serde_json::json!({ + "from": "server1.conn0.child2/accessibilityActor12", + "state": {"enabled": true} + }), + vec![], + ), + ], + ) + .on( + "enable", + serde_json::json!({"from": "server1.conn0.parentAccessibilityActor6"}), + ) + .on( + "disable", + serde_json::json!({"from": "server1.conn0.parentAccessibilityActor6"}), + ) + .on("getWalker", load_fixture("a11y_get_walker_response.json")) + .on_sequence( + "children", + vec![ + (load_fixture("a11y_walker_children_response.json"), vec![]), + (load_fixture("a11y_children_response.json"), vec![]), + (load_fixture("a11y_children_empty_response.json"), vec![]), + ], + ) +} + +/// Build a mock server where `enable` succeeds but `bootstrap` keeps +/// reporting the service disabled afterward — the "enable didn't take" +/// failure branch, which must surface as an explicit error, never a silent +/// fallback. +fn a11y_native_enable_does_not_take_server() -> MockRdpServer { + MockRdpServer::new() + .on("listTabs", load_fixture("list_tabs_response.json")) + .on("getTarget", load_fixture("get_target_response.json")) + .on( + "getRoot", + serde_json::json!({ + "from": "root", + "parentAccessibilityActor": "server1.conn0.parentAccessibilityActor6" + }), + ) + .on( + "bootstrap", + serde_json::json!({ + "from": "server1.conn0.child2/accessibilityActor12", + "state": {"enabled": false} + }), + ) + .on( + "enable", + serde_json::json!({"from": "server1.conn0.parentAccessibilityActor6"}), + ) +} + /// Build a mock server for a11y contrast (uses JS eval path like snapshot). fn a11y_contrast_server() -> MockRdpServer { MockRdpServer::new() @@ -269,6 +404,269 @@ fn a11y_with_jq_extracts_role() { assert_eq!(stdout.trim(), "\"document\""); } +// --------------------------------------------------------------------------- +// a11y: meta.source (iter-143 Theme A) +// --------------------------------------------------------------------------- + +#[test] +fn a11y_reports_native_source_when_walker_succeeds() { + let server = a11y_server(); + let port = server.port(); + let handle = std::thread::spawn(move || server.serve_one()); + + let mut args = base_args(port); + args.push("a11y".to_owned()); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + handle.join().unwrap(); + assert!( + output.status.success(), + "expected success, stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let json: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("stdout must be valid JSON"); + assert_eq!( + json["meta"]["source"], "native", + "a successful walker traversal must report meta.source = native: {json}" + ); + assert!( + json["meta"].get("source_reason").is_none(), + "the native path must not carry a fallback reason: {json}" + ); + assert!( + json["meta"].get("fallback").is_none(), + "the native path must not set the legacy fallback flag: {json}" + ); +} + +#[test] +fn a11y_reports_js_fallback_source_when_service_disabled() { + let server = a11y_disabled_service_server(); + let port = server.port(); + let handle = std::thread::spawn(move || server.serve_one()); + + let mut args = base_args(port); + args.push("a11y".to_owned()); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + handle.join().unwrap(); + assert!( + output.status.success(), + "expected success, stderr: {} stdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); + + let json: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("stdout must be valid JSON"); + assert_eq!( + json["meta"]["source"], "js-fallback", + "a disabled accessibility service must report meta.source = js-fallback: {json}" + ); + assert_eq!( + json["meta"]["source_reason"], "accessibility-service-disabled", + "the fallback reason must name why: {json}" + ); + assert_eq!( + json["meta"]["fallback"], true, + "the legacy fallback flag is kept for existing consumers: {json}" + ); +} + +#[test] +fn a11y_selector_mode_reports_js_fallback_without_legacy_fallback_flag() { + // `--selector` always runs the JS-eval selector path directly — it never + // touches `bootstrap`/the walker — so any mock exposing a role-shaped + // `evaluateJSAsync` result works; reuse the disabled-service server's. + let server = a11y_disabled_service_server(); + let port = server.port(); + let handle = std::thread::spawn(move || server.serve_one()); + + let mut args = base_args(port); + args.extend([ + "a11y".to_owned(), + "--selector".to_owned(), + "main".to_owned(), + ]); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + handle.join().unwrap(); + assert!( + output.status.success(), + "expected success, stderr: {} stdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); + + let json: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("stdout must be valid JSON"); + assert_eq!( + json["meta"]["source"], "js-fallback", + "--selector is always JS-derived: {json}" + ); + assert_eq!(json["meta"]["source_reason"], "selector-mode"); + assert!( + json["meta"].get("fallback").is_none(), + "--selector is a deliberate JS-only mode, not an automatic fallback \ + from a failed native attempt, so the legacy fallback flag must be \ + absent: {json}" + ); +} + +// --------------------------------------------------------------------------- +// a11y --native (iter-143 Theme B) +// --------------------------------------------------------------------------- + +#[test] +fn a11y_native_walks_platform_tree_when_service_already_enabled() { + let server = a11y_native_already_enabled_server(); + let port = server.port(); + let handle = std::thread::spawn(move || server.serve_one()); + + let mut args = base_args(port); + args.extend(["a11y".to_owned(), "--native".to_owned()]); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + handle.join().unwrap(); + assert!( + output.status.success(), + "expected success, stderr: {} stdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); + + let json: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("stdout must be valid JSON"); + assert_eq!(json["meta"]["source"], "native"); + assert_eq!(json["results"]["role"], "document"); +} + +#[test] +fn a11y_native_enables_walks_and_restores_service() { + let mut server = a11y_native_opt_in_server(); + let enable_calls = server.call_counter("enable"); + let disable_calls = server.call_counter("disable"); + let port = server.port(); + let handle = std::thread::spawn(move || server.serve_one()); + + let mut args = base_args(port); + args.extend(["a11y".to_owned(), "--native".to_owned()]); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + handle.join().unwrap(); + assert!( + output.status.success(), + "expected success, stderr: {} stdout: {}", + String::from_utf8_lossy(&output.stderr), + String::from_utf8_lossy(&output.stdout) + ); + + let json: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("stdout must be valid JSON"); + assert_eq!(json["meta"]["source"], "native"); + assert_eq!(json["results"]["role"], "document"); + + assert_eq!( + enable_calls.load(std::sync::atomic::Ordering::SeqCst), + 1, + "the service must be enabled exactly once when it started off" + ); + assert_eq!( + disable_calls.load(std::sync::atomic::Ordering::SeqCst), + 1, + "the service must be restored to disabled exactly once, since this \ + run was the one that turned it on" + ); +} + +#[test] +fn a11y_native_conflicts_with_selector_at_cli_level() { + // No mock server needed: clap must reject this combination before any + // connection is attempted. + let mut args = base_args(6000); + args.extend([ + "a11y".to_owned(), + "--native".to_owned(), + "--selector".to_owned(), + "main".to_owned(), + ]); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + assert!( + !output.status.success(), + "--native and --selector must be rejected together" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("native") && stderr.contains("selector"), + "clap's conflict error should name both flags: {stderr}" + ); +} + +/// "unit/e2e: enable failure surfaces as an explicit error or an annotated +/// fallback, never a silent one" AC — the "enable() succeeded but bootstrap() +/// still reports disabled" branch. +#[test] +fn a11y_native_errors_explicitly_when_enable_does_not_take_effect() { + let mut server = a11y_native_enable_does_not_take_server(); + let walker_calls = server.call_counter("getWalker"); + let port = server.port(); + let handle = std::thread::spawn(move || server.serve_one()); + + let mut args = base_args(port); + args.extend(["a11y".to_owned(), "--native".to_owned()]); + + let output = std::process::Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("failed to spawn ff-rdp"); + + handle.join().unwrap(); + assert!( + !output.status.success(), + "must fail explicitly rather than silently substituting the JS tree" + ); + // Per the JSON-only output convention, errors are emitted as a JSON + // envelope on stdout, not a human line on stderr. + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("bootstrap") && stdout.contains("disabled"), + "the error must explain that bootstrap still reports disabled: {stdout}" + ); + assert_eq!( + walker_calls.load(std::sync::atomic::Ordering::SeqCst), + 0, + "must not attempt to walk the tree at all once enable is known to \ + not have taken effect" + ); +} + // --------------------------------------------------------------------------- // a11y contrast: basic output // --------------------------------------------------------------------------- @@ -337,6 +735,11 @@ fn a11y_contrast_outputs_json_with_checks() { json["meta"]["summary"]["total"].is_number(), "summary should have total" ); + + // iter-143 Theme A: contrast checking is always DOM/computed-style + // based, so meta.source is always js-fallback. + assert_eq!(json["meta"]["source"], "js-fallback"); + assert_eq!(json["meta"]["source_reason"], "contrast-audit-js-only"); } // --------------------------------------------------------------------------- diff --git a/crates/ff-rdp-cli/tests/live/live_143_native_a11y_tree.rs b/crates/ff-rdp-cli/tests/live/live_143_native_a11y_tree.rs new file mode 100644 index 0000000..e1aa7f0 --- /dev/null +++ b/crates/ff-rdp-cli/tests/live/live_143_native_a11y_tree.rs @@ -0,0 +1,148 @@ +//! Live tests for iteration 143 — `meta.source` on `ff-rdp a11y` and the +//! opt-in `--native` flag (carry-over from +//! [[iteration-136-core-live-test-repairs]]). +//! +//! Firefox's platform accessibility service is off by default on a fresh +//! headless launch (iter-136), so a plain `ff-rdp a11y` against it exercises +//! the JS-derived fallback path and must report that honestly in +//! `meta.source`. `--native` opts in to the real platform tree for the +//! duration of one call and must restore the service to its previous +//! (disabled) state afterward — verified here by re-running a plain `a11y` +//! afterward and observing the fallback path again (DEC-027: the service +//! must not be left enabled behind the user's back). +//! +//! # Running +//! +//! FF_RDP_LIVE_TESTS=1 cargo test-live -p ff-rdp-cli \ +//! --test live live_143_native_a11y_tree -- --nocapture + +use std::process::{Command, Output}; + +use serde_json::Value; + +use crate::common::{LiveFirefox, base_args, ff_rdp_bin, live_tests_enabled}; + +fn parse_json(output: &Output) -> Value { + let s = String::from_utf8_lossy(&output.stdout); + serde_json::from_str(s.trim()).unwrap_or_else(|e| { + panic!( + "stdout is not valid JSON: {e}\nstdout={s}\nstderr={}", + String::from_utf8_lossy(&output.stderr) + ) + }) +} + +fn run_a11y(port: u16, extra: &[&str]) -> Value { + let mut args = base_args(port); + args.push("a11y".to_owned()); + args.extend(extra.iter().map(|s| (*s).to_owned())); + let out = Command::new(ff_rdp_bin()) + .args(&args) + .output() + .expect("ff-rdp a11y"); + assert!( + out.status.success(), + "ff-rdp a11y {extra:?} failed: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + parse_json(&out) +} + +/// live_a11y_source_meta: `ff-rdp a11y` output carries a `meta.source` of +/// `js-fallback` against a Firefox with the accessibility service off. +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1"] +fn live_a11y_source_meta() { + if !live_tests_enabled() { + eprintln!("live_a11y_source_meta: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + let Some(ff) = LiveFirefox::headless_on_random_port() else { + eprintln!("live_a11y_source_meta: Firefox not available — skipping"); + return; + }; + + let json = run_a11y(ff.port(), &[]); + assert_eq!( + json["meta"]["source"], "js-fallback", + "a plain `a11y` call against a fresh headless Firefox (accessibility \ + service off by default) must report meta.source = js-fallback: {json}" + ); + assert_eq!( + json["meta"]["source_reason"], "accessibility-service-disabled", + "the fallback reason must name why the native path was not used: {json}" + ); +} + +/// live_a11y_native_opt_in: with the opt-in flag, the root role is +/// `document`, and the tree contains platform roles the JS fallback does not +/// produce. +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1"] +fn live_a11y_native_opt_in() { + if !live_tests_enabled() { + eprintln!("live_a11y_native_opt_in: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + let Some(ff) = LiveFirefox::headless_on_random_port() else { + eprintln!("live_a11y_native_opt_in: Firefox not available — skipping"); + return; + }; + + let json = run_a11y(ff.port(), &["--native"]); + assert_eq!( + json["meta"]["source"], "native", + "--native must report meta.source = native: {json}" + ); + assert_eq!( + json["results"]["role"], "document", + "the native platform tree's root role must be \"document\" (not the \ + JS-derived fallback's DOM-approximated roles): {json}" + ); + assert!( + json["meta"].get("source_reason").is_none(), + "a successful native run must not carry a fallback reason: {json}" + ); +} + +/// live_a11y_service_restored: after an opt-in run that enabled the service, +/// `bootstrap().state.enabled` is back to its pre-run value. +/// +/// There is no CLI surface for a raw `bootstrap()` probe, so this asserts the +/// externally observable equivalent: a plain (non-`--native`) `a11y` call +/// immediately after the opt-in run must take the JS-fallback path again — +/// proof the service did not stay enabled behind the caller's back. +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1"] +fn live_a11y_service_restored() { + if !live_tests_enabled() { + eprintln!("live_a11y_service_restored: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + let Some(ff) = LiveFirefox::headless_on_random_port() else { + eprintln!("live_a11y_service_restored: Firefox not available — skipping"); + return; + }; + + // Pre-run: service is off by default on a fresh headless launch. + let before = run_a11y(ff.port(), &[]); + assert_eq!(before["meta"]["source"], "js-fallback"); + + // Opt-in run: enables the service, walks the native tree, restores. + let opted_in = run_a11y(ff.port(), &["--native"]); + assert_eq!(opted_in["meta"]["source"], "native"); + + // Post-run: must be back to js-fallback — the service must not have been + // left enabled after the opt-in call returned. + let after = run_a11y(ff.port(), &[]); + assert_eq!( + after["meta"]["source"], "js-fallback", + "the accessibility service must be restored to disabled after a --native \ + run that turned it on — got meta={:?}", + after["meta"] + ); + assert_eq!( + after["meta"]["source_reason"], "accessibility-service-disabled", + "post-restore state must match the pre-run disabled state: {after}" + ); +} diff --git a/crates/ff-rdp-cli/tests/live/live_a11y_critical.rs b/crates/ff-rdp-cli/tests/live/live_a11y_critical.rs index 9174c46..5a61775 100644 --- a/crates/ff-rdp-cli/tests/live/live_a11y_critical.rs +++ b/crates/ff-rdp-cli/tests/live/live_a11y_critical.rs @@ -70,6 +70,11 @@ fn a11y_critical_filters_to_violations() { ); assert_eq!(results[0]["violation"], "missing-alt"); assert_eq!(results[0]["role"], "img"); + // iter-143 Theme A: --critical is always JS-derived (no native-actor + // equivalent for a WCAG-critical severity), so meta.source is always + // reported as such. + assert_eq!(json["meta"]["source"], "js-fallback"); + assert_eq!(json["meta"]["source_reason"], "critical-audit-js-only"); // Clean page: has alt, no other violators. let good = "data:text/html,good\"hero\""; diff --git a/crates/ff-rdp-cli/tests/live/main.rs b/crates/ff-rdp-cli/tests/live/main.rs index 0f4d7a9..1d6b7e7 100644 --- a/crates/ff-rdp-cli/tests/live/main.rs +++ b/crates/ff-rdp-cli/tests/live/main.rs @@ -57,6 +57,7 @@ mod live_141_output_hygiene; mod live_142_daemon_stop_pid_honesty; mod live_142_disk_growth; mod live_142_eval_asi_await; +mod live_143_native_a11y_tree; mod live_61l; mod live_61q_resource_bus; mod live_61r_eval; diff --git a/crates/ff-rdp-core/src/actors/accessibility.rs b/crates/ff-rdp-core/src/actors/accessibility.rs index 57d964a..e7dfbf6 100644 --- a/crates/ff-rdp-core/src/actors/accessibility.rs +++ b/crates/ff-rdp-core/src/actors/accessibility.rs @@ -92,6 +92,56 @@ impl AccessibilityActor { .unwrap_or(false)) } + /// Enable the platform accessibility service via `enable()` on the root + /// actor's `parentAccessibilityActor` (obtained from + /// [`crate::actors::root::RootActor::get_root`]'s + /// `"parentAccessibilityActor"` field). + /// + /// This is a **browser-global, process-wide** change (iter-136, iter-143 + /// Theme B / DEC-027): its performance cost persists until the browser + /// shuts down, and it is not scoped to the tab or connection that made + /// the call. Callers MUST NOT call this unconditionally — check + /// [`Self::is_service_enabled`] first, only call `enable_service` when it + /// reports `false`, and only when the caller has explicit opt-in from the + /// user (e.g. a `--native` flag) to make a whole-browser mutation. Pair a + /// successful call with [`Self::disable_service`] once the caller is done + /// — but only when this call is what turned the service on, never when it + /// was already running for some other reason. + pub fn enable_service( + transport: &mut RdpTransport, + parent_accessibility_actor: &ActorId, + ) -> Result<(), ProtocolError> { + actor_request( + transport, + parent_accessibility_actor.as_ref(), + "enable", + None, + )?; + Ok(()) + } + + /// Disable the platform accessibility service via `disable()` on the root + /// actor's `parentAccessibilityActor`. The inverse of + /// [`Self::enable_service`]. + /// + /// On Windows an active screen reader can block `disable` from taking + /// effect (kb/rdp/actors/accessibility.md). Callers should treat a + /// failure here as best-effort — report it, but don't let it mask the + /// primary result of whatever the caller was doing with the service + /// enabled. + pub fn disable_service( + transport: &mut RdpTransport, + parent_accessibility_actor: &ActorId, + ) -> Result<(), ProtocolError> { + actor_request( + transport, + parent_accessibility_actor.as_ref(), + "disable", + None, + )?; + Ok(()) + } + /// Get the children of an accessible node. /// /// `children` is a **method on the accessible actor itself** @@ -386,9 +436,64 @@ pub fn filter_interactive(node: &AccessibleNode) -> Option { #[cfg(test)] mod tests { + use std::io::{BufReader, Write}; + use std::net::{TcpListener, TcpStream}; + use super::*; use serde_json::json; + /// Spins up a loopback TCP listener that replies with `response` to the + /// first request it receives, mirroring the pattern used across the + /// other actor test suites (e.g. `actors::root::tests`). + fn make_transport_with_response(response: serde_json::Value) -> RdpTransport { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let client = TcpStream::connect(addr).unwrap(); + let (accept, _) = listener.accept().unwrap(); + + std::thread::spawn(move || { + let mut srv_reader = BufReader::new(&accept); + let _ = crate::transport::recv_from(&mut srv_reader).unwrap(); + let frame = crate::transport::encode_frame(&serde_json::to_string(&response).unwrap()); + (&accept).write_all(frame.as_bytes()).unwrap(); + }); + + let writer = client.try_clone().unwrap(); + let reader = BufReader::new(client); + RdpTransport::from_parts(reader, writer) + } + + #[test] + fn enable_service_sends_enable_to_parent_actor() { + let response = json!({ "from": "server1.conn0.parentAccessibilityActor6" }); + let mut transport = make_transport_with_response(response); + let parent: ActorId = "server1.conn0.parentAccessibilityActor6".into(); + AccessibilityActor::enable_service(&mut transport, &parent) + .expect("enable_service should succeed on a plain {} reply"); + } + + #[test] + fn disable_service_sends_disable_to_parent_actor() { + let response = json!({ "from": "server1.conn0.parentAccessibilityActor6" }); + let mut transport = make_transport_with_response(response); + let parent: ActorId = "server1.conn0.parentAccessibilityActor6".into(); + AccessibilityActor::disable_service(&mut transport, &parent) + .expect("disable_service should succeed on a plain {} reply"); + } + + #[test] + fn enable_service_propagates_actor_error() { + let response = json!({ + "from": "server1.conn0.parentAccessibilityActor6", + "error": "unrecognizedPacketType", + "message": "enable" + }); + let mut transport = make_transport_with_response(response); + let parent: ActorId = "server1.conn0.parentAccessibilityActor6".into(); + let err = AccessibilityActor::enable_service(&mut transport, &parent).unwrap_err(); + assert!(err.is_unrecognized_packet_type()); + } + #[test] fn parse_accessible_node_full() { let v = json!({ diff --git a/kb/iterations/iteration-143-native-a11y-tree.md b/kb/iterations/iteration-143-native-a11y-tree.md index c6181bf..61148c8 100644 --- a/kb/iterations/iteration-143-native-a11y-tree.md +++ b/kb/iterations/iteration-143-native-a11y-tree.md @@ -11,8 +11,14 @@ dogfood_path: | ff-rdp a11y --port 6000 --native # → must return the platform tree (roles like "document"/"paragraph"), # not the DOM-derived one (roles like "generic") -first_call_sites: [] -status: planned +first_call_sites: + - primitive: "AccessibilityActor::enable_service/disable_service (ff-rdp-core) called from commands::a11y::run_native_opt_in for the --native opt-in path" + site: "crates/ff-rdp-cli/src/commands/a11y.rs" + - primitive: "connection_meta::merge_source — meta.source/source_reason on the a11y tree, --critical, and contrast output" + site: "crates/ff-rdp-cli/src/commands/a11y.rs" + - primitive: "connection_meta::merge_source on a11y contrast output" + site: "crates/ff-rdp-cli/src/commands/a11y_contrast.rs" +status: in-review --- # Iteration 143: decide and expose how `ff-rdp a11y` gets its tree @@ -90,20 +96,36 @@ the service is off will block for the full socket read timeout. Consider a short purpose-specific deadline on accessibility walker requests so a mistake costs milliseconds, not the default timeout. -## Acceptance Criteria [1/5] +## Acceptance Criteria [5/5] -- [ ] live_a11y_source_meta: `ff-rdp a11y` output carries a `meta.source` of +- [x] live_a11y_source_meta: `ff-rdp a11y` output carries a `meta.source` of `js-fallback` against a Firefox with the accessibility service off -- [ ] live_a11y_native_opt_in: with the opt-in flag, the root role is `document` and the + (`crates/ff-rdp-cli/tests/live/live_143_native_a11y_tree.rs`). Backed by the + mock-server counterpart `a11y_reports_js_fallback_source_when_service_disabled` + (`crates/ff-rdp-cli/tests/e2e/a11y.rs`), exercising `A11ySource::JsFallback` / + `connection_meta::merge_source` in `commands::a11y::run`. +- [x] live_a11y_native_opt_in: with the opt-in flag, the root role is `document` and the tree contains platform roles the JS fallback does not produce -- [ ] live_a11y_service_restored: after an opt-in run that enabled the service, + (`crates/ff-rdp-cli/tests/live/live_143_native_a11y_tree.rs`). Backed by the + mock-server counterparts `a11y_native_walks_platform_tree_when_service_already_enabled` + and `a11y_native_enables_walks_and_restores_service`, exercising + `commands::a11y::run_native_opt_in` and `AccessibilityActor::enable_service`. +- [x] live_a11y_service_restored: after an opt-in run that enabled the service, `bootstrap().state.enabled` is back to its pre-run value -- [ ] unit/e2e: enable failure surfaces as an explicit error or an annotated fallback, - never a silent one -- [x] [[decision-log]] records the default-vs-opt-in decision — done ahead of this - iteration's implementation work: DEC-027 (filed on main before this branch existed) - settles opt-in-never-default. No code landed yet for this iteration; only the - decision-log prerequisite is satisfied. + (`crates/ff-rdp-cli/tests/live/live_143_native_a11y_tree.rs`, asserted indirectly + via a plain `a11y` call reverting to `meta.source == "js-fallback"`). Backed by + `a11y_native_enables_walks_and_restores_service`'s `enable`/`disable` call-count + assertions (`crates/ff-rdp-cli/tests/e2e/a11y.rs`), exercising + `AccessibilityActor::disable_service`. +- [x] unit/e2e `a11y_native_errors_explicitly_when_enable_does_not_take_effect`: enable + failure surfaces as an explicit error or an annotated fallback, never a silent one + (`crates/ff-rdp-cli/tests/e2e/a11y.rs`), exercising the "bootstrap still reports + disabled after enable()" branch of `commands::a11y::run_native_opt_in`. +- [x] [[decision-log]] records the default-vs-opt-in decision via `AccessibilityActor::enable_service` + — done ahead of this iteration's implementation work: DEC-027 (filed on main before + this branch existed) settles opt-in-never-default. Implemented as designed: + `AccessibilityActor::enable_service` is called only from the opt-in `--native` path + (`commands::a11y::run_native_opt_in`), never automatically. ## Notes diff --git a/kb/iterations/iteration-144-session-hygiene-followup.md b/kb/iterations/iteration-144-session-hygiene-followup.md index 0a7de86..3f98a02 100644 --- a/kb/iterations/iteration-144-session-hygiene-followup.md +++ b/kb/iterations/iteration-144-session-hygiene-followup.md @@ -112,3 +112,19 @@ Same independence rule as iteration-142: these three sub-themes don't depend on Theme F still can't be reproduced in whatever environment implements this plan, split it into its own plan again rather than blocking C/D, and say so explicitly rather than silently dropping the AC. + +- **Precedent from [[iteration-143-native-a11y-tree]]** (merged ahead of this plan landing): two + patterns there are directly reusable here. + 1. *Restore-only-what-you-changed* (DEC-027): `AccessibilityActor::enable_service` is only + paired with a matching `disable_service` when the caller's own opt-in call is what turned the + state on, never when it was already in that state for another reason. Theme C's + `auto_consent` field-honesty redesign is the same shape of problem (a command reporting on + browser-global/session state it did not unilaterally create) — worth checking whether the + same "did I cause this, or was it already true" check applies before inventing a new + contract. + 2. *Bounded deadlines on RDP calls that can stall instead of error* (`A11Y_WALKER_TIMEOUT`, + iter-143 Theme C, working around the iter-136 walker stall): if Theme D's screenshot-stitch + investigation or Theme F's locale reproduction turns up an RDP call that blocks instead of + failing fast, narrowing the transport's read timeout around just that call (via + `RdpTransport::set_read_timeout`/`read_timeout`, restoring the previous value afterward) is + the established pattern rather than a bespoke one. diff --git a/kb/rdp/actors/accessibility.md b/kb/rdp/actors/accessibility.md index 640049f..f0db82f 100644 --- a/kb/rdp/actors/accessibility.md +++ b/kb/rdp/actors/accessibility.md @@ -71,5 +71,38 @@ Each node in the tree is its own actor. Check `bootstrap().state.enabled` on the content accessibility actor first (`AccessibilityActor::is_service_enabled`); enabling requires `enable()` on the root form's `parentAccessibilityActor`, which is a browser-global change - ff-rdp does not make on the user's behalf — `ff-rdp a11y` falls back to its - JS-derived tree instead. + ff-rdp does not make on the user's behalf by default — `ff-rdp a11y` falls + back to its JS-derived tree instead unless the caller opts in with + `--native` (iter-143, see below). + +## Opt-in native tree (`ff-rdp a11y --native`, iter-143 Theme B) + +`AccessibilityActor::enable_service`/`disable_service` (ff-rdp-core) wrap +`enable()`/`disable()` on `parentAccessibilityActor` (obtained from the root +form's `getRoot` response — `RootActor::get_root`). The CLI's +`run_native_opt_in` (`ff-rdp-cli/src/commands/a11y.rs`): + +1. Reads `parentAccessibilityActor` off `getRoot`. +2. Checks `bootstrap().state.enabled` on the content accessibility actor; if + already `true`, does nothing further (never touches state it did not + create — [[decision-log#DEC-027]]). +3. Otherwise calls `enable()`, re-checks `bootstrap()`, and errors explicitly + (never silently falls back) if it still reports disabled. +4. Walks the tree, then calls `disable()` — but only when step 2/3 is what + turned the service on. + +This is opt-in, never the default: `enable()` is browser-global and +process-wide, and its performance cost persists until the browser shuts down. +`ff-rdp a11y` (no flag) never calls it. + +## Bounded walker deadline (iter-143 Theme C) + +Both the auto-detect path (`run_native_or_js_fallback`) and `--native` +(`run_native_opt_in`/`walk_native_tree_bounded`) narrow the transport's read +timeout to `A11Y_WALKER_TIMEOUT` (3s) around `getWalker`/the root +accessor/`children` calls, restoring the previous timeout afterward. This +bounds the iter-136 stall (walker never replies while the service is off) to +a few seconds instead of the full `--timeout` (default 10s, but +user-configurable much higher) — relevant if a race disables the service +between the `bootstrap()` check and the walk, or a future call site skips the +check.