diff --git a/Cargo.lock b/Cargo.lock index 6336843..ae94a3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -129,6 +129,12 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "bitflags" version = "2.11.1" @@ -406,6 +412,15 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" +[[package]] +name = "fdeflate" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e6853b52649d4ac5c0bd02320cddc5ba956bdb407c4b75a2c6b75bf51500f8c" +dependencies = [ + "simd-adler32", +] + [[package]] name = "ff-rdp-cli" version = "0.3.0" @@ -426,6 +441,7 @@ dependencies = [ "jaq-std", "jsonschema", "libc", + "png", "regex", "serde", "serde_json", @@ -1277,6 +1293,19 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "png" +version = "0.17.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82151a2fc869e011c153adc57cf2789ccb8d9906ce52c0b39a6b5697749d7526" +dependencies = [ + "bitflags 1.3.2", + "crc32fast", + "fdeflate", + "flate2", + "miniz_oxide", +] + [[package]] name = "portable-atomic" version = "1.13.1" @@ -1347,7 +1376,7 @@ version = "0.5.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" dependencies = [ - "bitflags", + "bitflags 2.11.1", ] [[package]] @@ -1483,7 +1512,7 @@ version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6fe4565b9518b83ef4f91bb47ce29620ca828bd32cb7e408f0062e9930ba190" dependencies = [ - "bitflags", + "bitflags 2.11.1", "errno", "libc", "linux-raw-sys", @@ -1822,7 +1851,7 @@ version = "0.6.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68d6fdd9f81c2819c9a8b0e0cd91660e7746a8e6ea2ba7c6b2b057985f6bcb51" dependencies = [ - "bitflags", + "bitflags 2.11.1", "bytes", "futures-util", "http", @@ -2179,7 +2208,7 @@ version = "0.244.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47b807c72e1bac69382b3a6fb3dbe8ea4c0ed87ff5629b8685ae6b9a611028fe" dependencies = [ - "bitflags", + "bitflags 2.11.1", "hashbrown 0.15.5", "indexmap", "semver", @@ -2449,7 +2478,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d66ea20e9553b30172b5e831994e35fbde2d165325bec84fc43dbf6f4eb9cb2" dependencies = [ "anyhow", - "bitflags", + "bitflags 2.11.1", "indexmap", "log", "serde", diff --git a/crates/ff-rdp-cli/Cargo.toml b/crates/ff-rdp-cli/Cargo.toml index 2031493..d4ac72b 100644 --- a/crates/ff-rdp-cli/Cargo.toml +++ b/crates/ff-rdp-cli/Cargo.toml @@ -74,6 +74,9 @@ dirs = { workspace = true } filetime = { workspace = true } jsonschema = "0.27" sha2 = "0.10" +# iter-144 Theme D: live_144_full_page_no_duplicate_header decodes captured +# PNG pixel rows to detect a repeated header band — test-only, never shipped. +png = "0.17" [target.'cfg(unix)'.dev-dependencies] libc = { workspace = true } diff --git a/crates/ff-rdp-cli/src/cli/args.rs b/crates/ff-rdp-cli/src/cli/args.rs index 2d70084..e556065 100644 --- a/crates/ff-rdp-cli/src/cli/args.rs +++ b/crates/ff-rdp-cli/src/cli/args.rs @@ -432,6 +432,11 @@ pub enum Command { /// List open browser tabs #[command(long_about = "List open browser tabs. +The Consent-O-Matic options tab `launch --auto-consent` leaves open +(`moz-extension://.../options.html`, title \"Consent-O-Matic Options\" or a +known localized equivalent) is filtered out of the listing (iter-144) — it +is never counted in `total` or in `--tab N` indices. + Output: {\"results\": [{\"url\": \"...\", \"title\": \"...\", \"actor\": \"...\", \"selected\": true}], \"total\": N, \"meta\": {...}}")] Tabs, /// Navigate to a URL @@ -466,7 +471,7 @@ Examples: --auto-consent (iter-129): after the document commits, run the same CMP-detection-and-accept flow as `ff-rdp consent accept` and add -`results.consent = {\"cmp\": \"sourcepoint\"|null, \"action\": \"accepted\"|null}` +`results.consent = {\"cmp\": \"sourcepoint\"|\"bbc\"|null, \"action\": \"accepted\"|null}` (both keys always present, never omitted). Best-effort — a detection failure prints a warning but does not fail the navigate. Not combinable with --with-network. This is the CLI-native complement to `launch --auto-consent` @@ -1143,12 +1148,24 @@ Examples: ff-rdp launch # launch with temp profile on port 6000 ff-rdp launch --headless # headless mode (no visible window) ff-rdp launch --port 9222 # use a different debug port - ff-rdp launch --auto-consent # auto-dismiss cookie banners + ff-rdp launch --auto-consent # install the Consent-O-Matic extension ff-rdp launch --profile ~/my-prof # reuse an existing profile ff-rdp launch --headless --window-size 600x800 # true viewport, >= floor ff-rdp launch --headless --window-size 390x844 # below floor — clamps to ~500, warns -Output: {\"results\": {\"pid\": N, \"host\": \"...\", \"port\": N, \"headless\": bool, \"profile\": \"...\", \"profile_path\": \"...\", \"temp_profile\": bool, \"auto_consent\": bool, \"window_size\": {\"requested\": {\"width\": N, \"height\": N}, \"below_floor\": bool}|null, \"warnings\"?: [...]}, \"total\": 1, \"meta\": {...}}" +--auto-consent installs the Consent-O-Matic extension into the profile so it +CAN dismiss cookie banners it recognizes once a page loads — but `launch` +returns before any page loads, so its +`results.auto_consent_extension_installed` field (iter-144) only reports +that the extension was installed, never that anything was actually +dismissed (a prior `auto_consent: true` field made that false claim — see +kb/iterations/iteration-142-session-hygiene.md). For a real dismiss +attestation after navigating, use `ff-rdp navigate --auto-consent` or +`ff-rdp consent accept`, both of which report `results.consent = {\"cmp\": +..., \"action\": ...}` (`action` is `\"accepted\"` only when a control was +actually clicked). + +Output: {\"results\": {\"pid\": N, \"host\": \"...\", \"port\": N, \"headless\": bool, \"profile\": \"...\", \"profile_path\": \"...\", \"temp_profile\": bool, \"auto_consent_extension_installed\": bool, \"window_size\": {\"requested\": {\"width\": N, \"height\": N}, \"below_floor\": bool}|null, \"warnings\"?: [...]}, \"total\": 1, \"meta\": {...}}" )] Launch(LaunchArgs), /// Install Claude Code skill files to the user or project filesystem @@ -1291,14 +1308,16 @@ Output: writes page-map JSON/YAML to --out (default: .ffrdp/page-map.json) Complements `ff-rdp launch --auto-consent` (which installs the Consent-O-Matic extension): Consent-O-Matic does not reliably record consent for the Sourcepoint CMP in headless mode (dogfooding-session-62 finding 1). This -command is the CLI-native fallback — it enumerates the tab's frame targets -(including cross-origin iframes), recognises a known CMP by matching a -frame's URL, and clicks that frame's \"accept all\" control directly. +command is the CLI-native fallback — it first tries a table of known +same-origin (non-iframe) CMPs (e.g. BBC's own `#bbccookies-continue-button`, +iter-144), then enumerates the tab's frame targets (including cross-origin +iframes) and recognises a known iframe-hosted CMP by matching a frame's URL, +clicking that frame's \"accept all\" control directly. Subcommands: consent accept Detect a known CMP on the current tab and accept it -Output: {\"results\": {\"cmp\": \"sourcepoint\"|null, \"action\": \"accepted\"|null}, \"total\": 1, \"meta\": {...}} +Output: {\"results\": {\"cmp\": \"sourcepoint\"|\"bbc\"|null, \"action\": \"accepted\"|null}, \"total\": 1, \"meta\": {...}} Both keys are always present — null/null when no known CMP was found on the page. See also: `ff-rdp navigate --auto-consent` to run this automatically after navigating." @@ -2106,7 +2125,12 @@ pub struct LaunchArgs { /// Override the debug server port (defaults to --port value) #[arg(long)] pub debug_port: Option, - /// Install Consent-O-Matic extension to auto-dismiss cookie consent banners + /// Install the Consent-O-Matic extension so it can auto-dismiss cookie + /// consent banners once a page loads. Reported back as + /// `results.auto_consent_extension_installed` — `launch` returns + /// before any page loads, so it cannot itself attest that a banner was + /// dismissed; use `navigate --auto-consent` or `consent accept` for + /// that (see their `results.consent` field). #[arg(long)] pub auto_consent: bool, /// Set the initial window size as `WxH` pixels (forwarded to Firefox as diff --git a/crates/ff-rdp-cli/src/commands/consent.rs b/crates/ff-rdp-cli/src/commands/consent.rs index 61edc79..07ca4ca 100644 --- a/crates/ff-rdp-cli/src/commands/consent.rs +++ b/crates/ff-rdp-cli/src/commands/consent.rs @@ -69,6 +69,41 @@ const CMP_TABLE: &[CmpEntry] = &[CmpEntry { frame_url_substrings: &["sourcepoint", "sp-prod.net", "privacy-mgmt.com"], }]; +/// One entry in the *native* (same-origin, non-iframe) CMP recognition +/// table — for sites whose consent control lives directly in the top +/// document rather than behind a recognisable cross-origin iframe (iter-144 +/// Theme C). Matched against the top-level target's own URL, then clicked +/// via a fixed CSS selector rather than label matching, since a site-owned +/// control's id/class is more stable across locales than its visible text +/// (see `accept_all_js`'s label-based table for the iframe-hosted case). +struct NativeCmpEntry { + /// Machine-readable CMP name, reported verbatim as `results.cmp`. + name: &'static str, + /// Case-insensitive substrings checked against the top-level target's + /// URL. Any match makes this entry's `selector` the one tried. + host_url_substrings: &'static [&'static str], + /// CSS selector for the accept control, evaluated against the top + /// document. + selector: &'static str, +} + +/// Native CMP table. BBC's own cookie banner (`kb/iterations/ +/// iteration-144-session-hygiene-followup.md` Theme C) sits in the top +/// document at `#bbccookies-continue-button` rather than in an iframe. +/// Verified live (2026-08-12) at www.bbc.com/news: BBC also runs a +/// Sourcepoint iframe CMP (`CMP_TABLE` above) that renders *in front of* +/// this control on first paint — the element exists but has a zero-size +/// bounding rect until the Sourcepoint overlay is dismissed, which is why +/// [`native_accept_js`] requires a non-zero rect rather than just DOM +/// presence. `detect_and_accept` tries this table before `CMP_TABLE`, so a +/// second `consent accept` call (after the first dismissed Sourcepoint) +/// reaches it. +const NATIVE_CMP_TABLE: &[NativeCmpEntry] = &[NativeCmpEntry { + name: "bbc", + host_url_substrings: &["bbc.com", "bbc.co.uk"], + selector: "#bbccookies-continue-button", +}]; + /// Result of a consent-detection pass. Both fields are always present in the /// JSON form (`to_json`) — `null`/`null` when no known CMP was found, never /// omitted, so `--jq '.results.action'` never throws regardless of whether a @@ -99,7 +134,7 @@ impl ConsentResult { fn accept_all_js() -> String { format!( r#"(function() {{ - var re = /^(accept all|accept all cookies|accept all and continue|accept all and close|accept all and subscribe|i accept|allow all)$/i; + var re = /^(accept all|accept all cookies|accept all and continue|accept all and close|accept all and subscribe|i accept|i agree|allow all)$/i; var candidates = Array.prototype.slice.call(document.querySelectorAll('button, [role="button"], a')); var target = null; for (var i = 0; i < candidates.length; i++) {{ @@ -115,11 +150,66 @@ fn accept_all_js() -> String { ) } +/// JS that clicks a fixed CSS selector directly in the top document, +/// requiring a non-zero bounding rect first — a same-origin control can be +/// present in the DOM while genuinely inert (e.g. covered/collapsed behind +/// another overlay, see [`NATIVE_CMP_TABLE`]'s BBC doc comment), and +/// clicking it in that state would falsely report `action: "accepted"` +/// without dismissing anything a user would perceive. +fn native_accept_js(selector: &str) -> String { + let escaped = selector.replace('\\', "\\\\").replace('\'', "\\'"); + format!( + r"(function() {{ + var target = document.querySelector('{escaped}'); + if (!target) throw new Error('Element not found: no native consent control at the known selector'); + var r = target.getBoundingClientRect(); + if (r.width === 0 || r.height === 0) throw new Error('Element not found: native consent control present but not visible (covered or collapsed)'); + var label = (target.getAttribute('aria-label') || target.textContent || '').trim(); + target.click(); + return '{JSON_SENTINEL}' + JSON.stringify({{accepted: true, label: label}}); +}})()" + ) +} + +/// Try every [`NATIVE_CMP_TABLE`] entry whose host substring matches the +/// tab's current top-level URL, clicking the first one found visible. +/// +/// Returns `Ok(None)` when no entry's host matches, or the matching entry's +/// selector wasn't found/visible — the caller falls through to the +/// iframe-based [`CMP_TABLE`] path in either case, so a same-origin miss +/// never masks a real cross-origin CMP. +fn try_native_cmp( + ctx: &mut ConnectedTab, + console_actor: &ff_rdp_core::ActorId, + top_level_url: &str, +) -> Result, AppError> { + let Some(entry) = match_native_cmp(top_level_url) else { + return Ok(None); + }; + + let js = native_accept_js(entry.selector); + let eval_result = WebConsoleActor::evaluate_js_async(ctx.transport_mut(), console_actor, &js) + .map_err(AppError::from)?; + + if eval_result.exception.is_none() { + Ok(Some(ConsentResult { + cmp: Some(entry.name), + action: Some("accepted"), + })) + } else { + // Present-but-invisible or genuinely absent — either way, not this + // call's job to report; let the iframe table take a turn. + Ok(None) + } +} + /// Detect a known CMP on the current tab and click its "accept all" control. /// -/// Enumerates frame targets (via the iter-129 Theme A opt-in path), matches -/// each non-top frame's URL against [`CMP_TABLE`], and evaluates the -/// accept-click JS on the first match's own console actor. +/// Enumerates frame targets (via the iter-129 Theme A opt-in path). First +/// tries [`NATIVE_CMP_TABLE`] (same-origin, top-document controls) via +/// [`try_native_cmp`]; if that finds nothing actionable, matches each +/// non-top frame's URL against [`CMP_TABLE`] and evaluates the accept-click +/// JS on the first match's own console actor. /// /// Returns `{"cmp": null, "action": null}` when no known CMP frame is found. /// Returns `{"cmp": "", "action": null}` when a CMP frame is found but @@ -135,6 +225,15 @@ pub(crate) fn detect_and_accept(ctx: &mut ConnectedTab) -> Result Option<&'static str> { .map(|cmp| cmp.name) } +/// Returns the first [`NativeCmpEntry`] whose `host_url_substrings` matches +/// `url` (case-insensitive). Pure and side-effect-free, mirroring +/// [`match_cmp`] — factored out of [`try_native_cmp`] so the host-matching +/// rule is unit-testable without a live connection. +fn match_native_cmp(url: &str) -> Option<&'static NativeCmpEntry> { + let lower = url.to_ascii_lowercase(); + NATIVE_CMP_TABLE + .iter() + .find(|e| e.host_url_substrings.iter().any(|s| lower.contains(s))) +} + #[cfg(test)] mod tests { use super::*; @@ -272,4 +382,74 @@ mod tests { assert!(js.contains(JSON_SENTINEL)); assert!(js.contains("accept all")); } + + /// iter-144 Theme C: BBC's live Sourcepoint iframe shows an "I agree" + /// button, not any of the previous exact-phrase matches — this is the + /// regex fix, unit-testable via the label-matching regex embedded in + /// the generated JS string (a full DOM match needs a live browser, see + /// `live_144_bbc_cmp_dismissed`). + #[test] + fn accept_all_js_regex_matches_i_agree() { + let js = accept_all_js(); + let re_line = js + .lines() + .find(|l| l.contains("var re =")) + .expect("accept_all_js must declare `var re`"); + assert!( + re_line.to_ascii_lowercase().contains("i agree"), + "accept_all_js regex must match the bare \"I agree\" label observed live on BBC: {re_line}" + ); + } + + // ── native CMP matching (iter-144 Theme C) ────────────────────────── + + /// AC: `live_144_bbc_cmp_dismissed` (matching-rule half) — bbc.com and + /// bbc.co.uk both resolve to the same native entry. + #[test] + fn match_native_cmp_matches_bbc_hosts() { + assert_eq!( + match_native_cmp("https://www.bbc.com/news").map(|e| e.name), + Some("bbc") + ); + assert_eq!( + match_native_cmp("https://www.bbc.co.uk/news").map(|e| e.name), + Some("bbc") + ); + } + + #[test] + fn match_native_cmp_is_case_insensitive() { + assert_eq!( + match_native_cmp("HTTPS://WWW.BBC.COM/NEWS").map(|e| e.name), + Some("bbc") + ); + } + + /// AC: `live_144_bbc_cmp_dismissed` (no-match half) — a non-BBC host + /// must not match, so the native table can't false-positive on + /// unrelated sites and mask a real iframe-based CMP. + #[test] + fn match_native_cmp_no_match_for_unrelated_url() { + assert_eq!( + match_native_cmp("https://example.com/").map(|e| e.name), + None + ); + } + + #[test] + fn native_accept_js_requires_visible_rect_and_carries_selector() { + let js = native_accept_js("#bbccookies-continue-button"); + assert!(js.contains("#bbccookies-continue-button")); + assert!(js.contains("Element not found:")); + assert!(js.contains("getBoundingClientRect")); + assert!(js.contains(JSON_SENTINEL)); + } + + /// A selector containing a single quote must not break out of the JS + /// string literal it's interpolated into. + #[test] + fn native_accept_js_escapes_single_quotes_in_selector() { + let js = native_accept_js("a[data-x='y']"); + assert!(js.contains(r"a[data-x=\'y\']")); + } } diff --git a/crates/ff-rdp-cli/src/commands/launch.rs b/crates/ff-rdp-cli/src/commands/launch.rs index 568ed93..80ef035 100644 --- a/crates/ff-rdp-cli/src/commands/launch.rs +++ b/crates/ff-rdp-cli/src/commands/launch.rs @@ -600,7 +600,18 @@ pub fn run( // field name (see live_daemon_stop_profile_path_matches_launch_json). "profile_path": profile_path_str, "temp_profile": effective_temp_profile, - "auto_consent": auto_consent, + // iter-144 Theme C: renamed from "auto_consent" — `launch` + // returns before any page loads, so this field can only + // ever attest that the Consent-O-Matic extension was + // *installed* into the profile, never that a consent + // banner was actually dismissed (kb/iterations/ + // iteration-142-session-hygiene.md found `auto_consent: + // true` reported while a banner still covered the page). + // A real dismiss attestation lives in `results.consent` + // from `navigate --auto-consent` / `consent accept` + // (`{"cmp": ..., "action": ...}`, iter-129) — those run + // after a page has loaded and can check the DOM. + "auto_consent_extension_installed": auto_consent, "window_size": window_size_json, }); if below_floor && let (Some((w, h)), Some(obj)) = (window_size, result.as_object_mut()) diff --git a/crates/ff-rdp-cli/src/commands/screenshot.rs b/crates/ff-rdp-cli/src/commands/screenshot.rs index 84ff36a..5727b03 100644 --- a/crates/ff-rdp-cli/src/commands/screenshot.rs +++ b/crates/ff-rdp-cli/src/commands/screenshot.rs @@ -664,6 +664,83 @@ fn try_two_step_screenshot( } } +/// Attribute `freeze_fixed_and_sticky_js` stamps on every element it mutates, +/// so `unfreeze_fixed_and_sticky_js` can find exactly those elements again +/// without re-walking the whole DOM by computed style (which could pick up +/// elements a page's own script repositioned mid-capture). +const FROZEN_MARKER_ATTR: &str = "data-ffrdp-frozen"; + +/// Attribute holding each frozen element's original `style` attribute value +/// (empty string if it had none), so `unfreeze_fixed_and_sticky_js` can +/// restore it byte-for-byte rather than guessing which properties to strip. +const FROZEN_STYLE_ATTR: &str = "data-ffrdp-frozen-style"; + +/// JS that pins every `position: fixed`/`sticky` element to its current +/// on-screen location via `position: absolute` + explicit `top`/`left`, +/// before a full-page capture (iter-144 Theme D). +/// +/// `BrowsingContext.drawSnapshot`'s full-page path renders a rect taller than +/// the real viewport; Gecko's compositor treats `fixed`/`sticky` elements as +/// pinned to the *viewport*, not the document, and (per dogfooding session 63 +/// against a long BBC News page) repaints them at intervals in the +/// synthesized tall snapshot — the fixed header shows up more than once +/// stitched down the page. Converting them to `absolute` with a computed +/// `top`/`left` removes that viewport-relative behavior entirely: the +/// element renders exactly once, at the position it already occupied on +/// screen, and normal single-pass compositing draws it there only. +/// +/// Best-effort: failures are swallowed by the caller (`eval_or_bail`'s +/// `Result` is discarded) rather than failing the whole capture — a page +/// whose fixed header can't be frozen should still get a (possibly +/// header-duplicated) screenshot rather than none at all. +fn freeze_fixed_and_sticky_js() -> String { + format!( + r"(function() {{ + var els = document.querySelectorAll('*'); + var frozen = 0; + for (var i = 0; i < els.length; i++) {{ + var el = els[i]; + var cs = window.getComputedStyle(el); + if (cs.position !== 'fixed' && cs.position !== 'sticky') continue; + var rect = el.getBoundingClientRect(); + el.setAttribute('{FROZEN_MARKER_ATTR}', '1'); + el.setAttribute('{FROZEN_STYLE_ATTR}', el.getAttribute('style') || ''); + el.style.position = 'absolute'; + el.style.top = (rect.top + window.scrollY) + 'px'; + el.style.left = (rect.left + window.scrollX) + 'px'; + frozen++; + }} + return JSON.stringify({{frozen: frozen}}); +}})()" + ) +} + +/// Undo [`freeze_fixed_and_sticky_js`]: restore every marked element's +/// original `style` attribute (or remove it if it had none) and clear the +/// marker attributes. Always run after a full-page capture, success or +/// failure, so a screenshot call never leaves the live page mutated. +fn unfreeze_fixed_and_sticky_js() -> String { + format!( + r#"(function() {{ + var els = document.querySelectorAll('[{FROZEN_MARKER_ATTR}="1"]'); + var restored = 0; + for (var i = 0; i < els.length; i++) {{ + var el = els[i]; + var orig = el.getAttribute('{FROZEN_STYLE_ATTR}'); + if (orig) {{ + el.setAttribute('style', orig); + }} else {{ + el.removeAttribute('style'); + }} + el.removeAttribute('{FROZEN_MARKER_ATTR}'); + el.removeAttribute('{FROZEN_STYLE_ATTR}'); + restored++; + }} + return JSON.stringify({{restored: restored}}); +}})()"# + ) +} + /// Fallback to `ScreenshotActor::screenshot_via_process_drawsnapshot` and encode /// the returned PNG bytes as a `data:image/png;base64,...` data URL. /// @@ -676,6 +753,13 @@ fn try_two_step_screenshot( /// to the JS call. The previous hard-rejection of `full_page=true` was the /// root cause of the iter-92 Theme A regression where `--full-page` silently /// produced a viewport-sized PNG instead of an error. +/// +/// iter-144 Theme D: when `full_page`, freezes `fixed`/`sticky` elements +/// (see [`freeze_fixed_and_sticky_js`]) immediately before the capture and +/// always restores them (see [`unfreeze_fixed_and_sticky_js`]) immediately +/// after — including on a capture error — so a sticky header is captured +/// once instead of duplicated down the page, and the live page is never +/// left in the frozen state. fn screenshot_via_process_drawsnapshot_fallback( ctx: &mut super::connect_tab::ConnectedTab, browsing_ctx_id: u64, @@ -738,13 +822,40 @@ fn screenshot_via_process_drawsnapshot_fallback( None }; - let png_bytes = ScreenshotActor::screenshot_via_process_drawsnapshot( + // iter-144 Theme D: freeze fixed/sticky elements immediately before the + // capture, restore immediately after — on every exit path, including a + // capture error, so the live page is never left mutated. Both eval calls + // are best-effort (`eval_or_bail`'s `Result` discarded): a page whose + // header can't be frozen/restored should not turn a working screenshot + // (or a real capture error) into an unrelated JS failure. + if full_page { + let console_actor = ctx.target.console_actor.clone(); + let _ = eval_or_bail( + ctx, + &console_actor, + &freeze_fixed_and_sticky_js(), + "screenshot: freeze fixed/sticky elements", + ); + } + + let capture_result = ScreenshotActor::screenshot_via_process_drawsnapshot( ctx.transport_mut(), browsing_ctx_id, full_page, full_page_rect, - ) - .map_err(|e| { + ); + + if full_page { + let console_actor = ctx.target.console_actor.clone(); + let _ = eval_or_bail( + ctx, + &console_actor, + &unfreeze_fixed_and_sticky_js(), + "screenshot: restore fixed/sticky elements", + ); + } + + let png_bytes = capture_result.map_err(|e| { // iter-135 Theme C: the old text ended with a "relaunch in headless // mode" instruction, which was wrong for the (common) case of an // already-headless session. It also appended @@ -961,4 +1072,48 @@ mod tests { let err = ff_rdp_core::ProtocolError::Timeout; assert!(!is_actor_module_load_failure(&err)); } + + // ── fixed/sticky freeze-for-capture (iter-144 Theme D) ────────────── + + /// AC: `live_144_full_page_no_duplicate_header` (mechanism half) — + /// `freeze_fixed_and_sticky_js` matches both `position` values that can + /// trigger the duplicate-header artifact, marks the elements it mutates + /// so `unfreeze_fixed_and_sticky_js` can find exactly those, and pins + /// them via an on-screen-derived `top`/`left` rather than a fixed + /// literal (which would misplace a header that isn't at y=0). + #[test] + fn freeze_js_matches_fixed_and_sticky_and_uses_bounding_rect() { + let js = freeze_fixed_and_sticky_js(); + assert!(js.contains("cs.position !== 'fixed'")); + assert!(js.contains("cs.position !== 'sticky'")); + assert!(js.contains("getBoundingClientRect")); + assert!(js.contains("position = 'absolute'")); + assert!(js.contains(FROZEN_MARKER_ATTR)); + assert!(js.contains(FROZEN_STYLE_ATTR)); + } + + /// `unfreeze_fixed_and_sticky_js` selects only elements carrying the + /// freeze pass's own marker attribute (never a page's pre-existing + /// `position: fixed` styling that the freeze pass didn't touch, and + /// never a same-named attribute a page happens to set itself), and + /// restores the saved `style` attribute rather than clearing individual + /// properties (so it can't drop styling the freeze pass didn't add). + #[test] + fn unfreeze_js_selects_only_marked_elements_and_restores_saved_style() { + let js = unfreeze_fixed_and_sticky_js(); + assert!(js.contains(&format!("[{FROZEN_MARKER_ATTR}=\"1\"]"))); + assert!(js.contains(FROZEN_STYLE_ATTR)); + assert!(js.contains("setAttribute('style', orig)")); + assert!(js.contains("removeAttribute('style')")); + assert!(js.contains(&format!("removeAttribute('{FROZEN_MARKER_ATTR}')"))); + } + + /// The freeze/unfreeze marker and style-backup attributes must be + /// distinct — using the same name for both would make the restore pass + /// unable to tell "marked" from "has a saved style", silently corrupting + /// whichever page the capture ran against. + #[test] + fn freeze_marker_and_style_attrs_are_distinct() { + assert_ne!(FROZEN_MARKER_ATTR, FROZEN_STYLE_ATTR); + } } diff --git a/crates/ff-rdp-cli/src/commands/tabs.rs b/crates/ff-rdp-cli/src/commands/tabs.rs index cda8a44..1fd7e5e 100644 --- a/crates/ff-rdp-cli/src/commands/tabs.rs +++ b/crates/ff-rdp-cli/src/commands/tabs.rs @@ -10,6 +10,34 @@ use crate::output; use crate::output_controls::{OutputControls, SortDir}; use crate::output_pipeline::OutputPipeline; +/// Known localized titles of Consent-O-Matic's options page +/// (`ui.js`'s `OPTIONS_TITLE` message table, vendored in +/// `crates/ff-rdp-cli/assets/extensions/consent-o-matic-1.1.5.xpi`). +/// `launch`'s pinned `intl.locale.requested = "en-US"` (`USER_JS` in +/// `commands/launch.rs`) should keep this at the English string, but all +/// five shipped locales are matched defensively — see the iteration-144 +/// Theme F note about that pin's reliability being itself under +/// investigation. +const CONSENT_O_MATIC_OPTIONS_TITLES: &[&str] = &[ + "Consent-O-Matic Options", // en + "Consent-O-Matic indstillinger", // da + "Consent-O-Matic Einstellungen", // de + "Opções do Consent-O-Matic", // pt + "Options de Consent-O-Matic", // fr +]; + +/// True for the permanent options tab `launch --auto-consent` leaves open +/// after installing Consent-O-Matic (iter-144 Theme C). Matches on both the +/// `moz-extension://` scheme (the extension's per-profile UUID is random, +/// so the URL can't be matched exactly) and a known localized title, so an +/// unrelated extension's options tab in a caller-supplied `--profile` +/// isn't accidentally hidden. +fn is_consent_o_matic_options_tab(tab: &Value) -> bool { + let url = tab.get("url").and_then(Value::as_str).unwrap_or_default(); + let title = tab.get("title").and_then(Value::as_str).unwrap_or_default(); + url.starts_with("moz-extension://") && CONSENT_O_MATIC_OPTIONS_TITLES.contains(&title) +} + pub fn run(cli: &Cli) -> Result<(), AppError> { let mut connection = RdpConnection::connect( &cli.host, @@ -39,6 +67,13 @@ pub fn run(cli: &Cli) -> Result<(), AppError> { Value::Array(arr) => arr, other => vec![other], }; + // iter-144 Theme C: `launch --auto-consent` installs Consent-O-Matic, + // which opens its own options tab on first run and never closes it — + // that synthetic tab isn't something a caller targeting `--tab N` + // should ever see or count against tab indices, so it's filtered + // before sort/limit/total are computed (kb/iterations/ + // iteration-142-session-hygiene.md Theme C). + items.retain(|item| !is_consent_o_matic_options_tab(item)); controls.apply_sort(&mut items); let (limited, total, truncated) = controls.apply_limit(items, None); let shown = limited.len(); @@ -80,6 +115,77 @@ mod tests { } } + // ── Consent-O-Matic options tab filtering (iter-144 Theme C) ──────── + + /// AC: `live_144_no_consent_o_matic_tab_leak` (matching half) — the + /// exact shape observed live: `launch --auto-consent` on port 6103 + /// left `{"title":"Consent-O-Matic Options","url":"moz-extension:// + /// 959682e5-.../options.html"}` in the `tabs` listing. + #[test] + fn is_consent_o_matic_options_tab_matches_live_shape() { + let tab = json!({ + "actor": "server1.conn1.tabDescriptor2", + "title": "Consent-O-Matic Options", + "url": "moz-extension://959682e5-00b2-4372-a120-ed784d7b7b73/options.html", + "selected": false, + }); + assert!(is_consent_o_matic_options_tab(&tab)); + } + + #[test] + fn is_consent_o_matic_options_tab_matches_every_shipped_locale() { + for title in CONSENT_O_MATIC_OPTIONS_TITLES { + let tab = json!({ + "title": title, + "url": "moz-extension://any-uuid/options.html", + }); + assert!( + is_consent_o_matic_options_tab(&tab), + "locale title {title:?} should match" + ); + } + } + + /// AC: `live_144_no_consent_o_matic_tab_leak` (no-match half) — a real + /// page tab, and a same-titled tab on a non-extension scheme (so the + /// filter can't be spoofed by page content), must not be filtered. + #[test] + fn is_consent_o_matic_options_tab_no_match_for_normal_tab() { + let normal = json!({"title": "Example", "url": "https://example.com"}); + assert!(!is_consent_o_matic_options_tab(&normal)); + + let same_title_wrong_scheme = + json!({"title": "Consent-O-Matic Options", "url": "https://example.com"}); + assert!(!is_consent_o_matic_options_tab(&same_title_wrong_scheme)); + } + + /// A different extension's options tab (arbitrary title, moz-extension + /// scheme) must not be filtered — only Consent-O-Matic's known titles + /// match, so a caller-supplied `--profile` with other extensions keeps + /// seeing their tabs. + #[test] + fn is_consent_o_matic_options_tab_no_match_for_other_extension() { + let other = json!({ + "title": "uBlock Origin", + "url": "moz-extension://other-uuid/options.html", + }); + assert!(!is_consent_o_matic_options_tab(&other)); + } + + #[test] + fn consent_o_matic_tab_filtered_out_of_tabs_list() { + let mut items = vec![ + json!({"title": "Example", "url": "https://example.com"}), + json!({ + "title": "Consent-O-Matic Options", + "url": "moz-extension://uuid/options.html", + }), + ]; + items.retain(|item| !is_consent_o_matic_options_tab(item)); + assert_eq!(items.len(), 1); + assert_eq!(items[0]["title"], "Example"); + } + #[test] fn fields_filter_applied_to_tab_entries() { let items = vec![ diff --git a/crates/ff-rdp-cli/tests/common/mod.rs b/crates/ff-rdp-cli/tests/common/mod.rs index 3e187ed..20cd853 100644 --- a/crates/ff-rdp-cli/tests/common/mod.rs +++ b/crates/ff-rdp-cli/tests/common/mod.rs @@ -709,6 +709,123 @@ pub fn assert_colors_equal(actual: &str, expected: &str, context: &str) { ); } +// --------------------------------------------------------------------------- +// PNG pixel decoding (iter-144 Theme D) +// --------------------------------------------------------------------------- +// +// `live_144_full_page_no_duplicate_header` needs actual pixel rows — the +// existing `png_dimensions`-style IHDR peek (used by earlier live suites) +// only reads the uncompressed header, not the (zlib-compressed) image data. +// The `png` crate is a dev-dependency solely for this. + +/// A decoded RGBA8 raster: `width`/`height` in pixels, `pixels` row-major, +/// 4 bytes (R, G, B, A) per pixel. +pub struct DecodedImage { + pub width: u32, + pub height: u32, + pub pixels: Vec, +} + +impl DecodedImage { + /// Return the `(r, g, b, a)` pixel at `(x, y)`, or `None` if out of + /// bounds. + pub fn pixel(&self, x: u32, y: u32) -> Option<(u8, u8, u8, u8)> { + if x >= self.width || y >= self.height { + return None; + } + let idx = ((y * self.width + x) * 4) as usize; + let px = self.pixels.get(idx..idx + 4)?; + Some((px[0], px[1], px[2], px[3])) + } + + /// Fraction (`0.0..=1.0`) of pixels in row `y` whose color is within + /// `tolerance` (per channel, RGB only — alpha ignored) of `color`. + /// Returns `0.0` for an out-of-bounds row. + pub fn row_color_fraction(&self, y: u32, color: (u8, u8, u8), tolerance: u8) -> f64 { + if y >= self.height || self.width == 0 { + return 0.0; + } + let close = |a: u8, b: u8| a.abs_diff(b) <= tolerance; + let mut matches = 0u32; + for x in 0..self.width { + if let Some((r, g, b, _)) = self.pixel(x, y) + && close(r, color.0) + && close(g, color.1) + && close(b, color.2) + { + matches += 1; + } + } + f64::from(matches) / f64::from(self.width) + } + + /// Count the number of separate vertical runs of rows whose + /// [`row_color_fraction`] for `color` meets or exceeds `min_fraction`. + /// + /// A page with one `position: fixed`/`sticky` header of `color` produces + /// exactly one run; the iter-144 Theme D duplicate-header artifact + /// produces more than one (the header repainted at an internal tile + /// boundary further down the full-page capture). + pub fn color_row_run_count( + &self, + color: (u8, u8, u8), + tolerance: u8, + min_fraction: f64, + ) -> u32 { + let mut runs = 0u32; + let mut in_run = false; + for y in 0..self.height { + let matches = self.row_color_fraction(y, color, tolerance) >= min_fraction; + if matches && !in_run { + runs += 1; + } + in_run = matches; + } + runs + } +} + +/// Decode PNG bytes (as produced by `ff-rdp screenshot --base64`) into a +/// [`DecodedImage`]. Panics on malformed input — live tests want a loud +/// failure naming the decode error, not a silent skip, since a decode +/// failure here means the screenshot command itself produced a broken PNG. +pub fn decode_png(bytes: &[u8]) -> DecodedImage { + let decoder = png::Decoder::new(bytes); + let mut reader = decoder + .read_info() + .unwrap_or_else(|e| panic!("decode_png: failed to read PNG header: {e}")); + let mut buf = vec![0u8; reader.output_buffer_size()]; + let info = reader + .next_frame(&mut buf) + .unwrap_or_else(|e| panic!("decode_png: failed to decode PNG frame: {e}")); + buf.truncate(info.buffer_size()); + + let width = info.width; + let height = info.height; + + // Normalize to RGBA8 regardless of the PNG's actual color type — the + // captures under test are always RGB(A)8, but this keeps the helper + // honest about what it assumes rather than silently misreading bytes. + let pixels = match info.color_type { + png::ColorType::Rgba => buf, + png::ColorType::Rgb => { + let mut out = Vec::with_capacity(buf.len() / 3 * 4); + for chunk in buf.chunks_exact(3) { + out.extend_from_slice(chunk); + out.push(255); + } + out + } + other => panic!("decode_png: unsupported color type {other:?} (expected Rgb/Rgba)"), + }; + + DecodedImage { + width, + height, + pixels, + } +} + #[cfg(test)] mod color_tests { use super::*; diff --git a/crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs b/crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs new file mode 100644 index 0000000..c5f269f --- /dev/null +++ b/crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs @@ -0,0 +1,301 @@ +//! Live tests for iteration 144 — session hygiene follow-up (carried over +//! from [[iteration-142-session-hygiene]] Theme C/D, plus the deferred +//! Theme F locale item — see `kb/iterations/iteration-144-session-hygiene-followup.md`). +//! +//! Covers: +//! - `launch --auto-consent`'s renamed `auto_consent_extension_installed` +//! field (Theme C part 1) +//! - `consent accept`'s BBC-style native-CMP adapter (Theme C part 2, +//! network-gated — the local part of the match rule is unit-tested in +//! `commands/consent.rs`) +//! - `tabs` filtering the leaked `Consent-O-Matic Options` tab (Theme C +//! part 3) +//! - `screenshot --full-page` freezing fixed/sticky elements so a header +//! is captured exactly once (Theme D) +//! +//! Theme F (console locale reproducibility) has no live test here: this +//! implementation environment has only an en-US Firefox available (macOS, +//! no non-English langpack), matching the exact "no non-English Firefox +//! available" case the iteration plan pre-authorizes deferring rather than +//! guessing at — see `kb/iterations/iteration-147-console-locale-repro.md`. +//! +//! # Theme D reproduction note +//! +//! The freeze-fixed/sticky fix in `commands/screenshot.rs` implements the +//! iteration plan's suggested mitigation, but the specific duplicate-header +//! symptom reported in dogfooding session 63 could **not** be reproduced in +//! this implementation environment despite a deliberate before/after +//! attempt: a `position:fixed` header and a `position:sticky` header (both +//! alone and nested, mirroring BBC's own `header{sticky}` + +//! `nav{sticky;top:80px}` structure) were captured at page heights from +//! 2 000 to 20 000 px — spanning common GPU texture-tile boundaries +//! (2048/4096/8192/16384) — and a live capture of the real +//! `https://www.bbc.com/news` itself, with no row-level pixel match for a +//! repeated header band in any case, on or off the fix. The test below +//! therefore verifies the invariant the AC states (no repeated header band) +//! as a forward-looking pixel-level regression guard against a local, +//! deterministic fixture, rather than a reproduced-then-fixed defect. +//! +//! # Running +//! +//! FF_RDP_LIVE_TESTS=1 cargo test -p ff-rdp-cli --test live live_144 -- --include-ignored +//! FF_RDP_LIVE_TESTS=1 FF_RDP_LIVE_NETWORK_TESTS=1 cargo test -p ff-rdp-cli \ +//! --test live live_144_bbc_cmp_dismissed -- --include-ignored --nocapture + +use std::process::Command; + +use base64::Engine as _; + +use crate::common::{ + LiveFirefox, base_args, decode_png, ff_rdp_bin, live_network_tests_enabled, live_tests_enabled, +}; + +fn parse_json(out: &std::process::Output, test: &str) -> serde_json::Value { + let s = String::from_utf8_lossy(&out.stdout); + serde_json::from_str(s.trim()).unwrap_or_else(|e| { + panic!( + "{test}: stdout is not valid JSON: {e}\nstdout={s}\nstderr={}", + String::from_utf8_lossy(&out.stderr) + ) + }) +} + +/// `live_144_auto_consent_field_honest`: +/// +/// `launch --auto-consent`'s JSON reports `results.auto_consent_extension_installed` +/// (never claims a dismiss happened — `launch` returns before any page +/// loads, so it structurally cannot know whether anything will be +/// dismissed) and no longer reports the old `auto_consent` field name, which +/// prior to iter-144 was set unconditionally `true` from the CLI flag and +/// was misread as "a banner was dismissed" (iteration-142 dogfooding +/// finding). +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1"] +fn live_144_auto_consent_field_honest() { + const TEST: &str = "live_144_auto_consent_field_honest"; + if !live_tests_enabled() { + eprintln!("{TEST}: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + let Some((ff, json)) = LiveFirefox::headless_on_random_port_with_args(&["--auto-consent"]) + else { + eprintln!("{TEST}: Firefox not available — skipping"); + return; + }; + let _ = &ff; // keep the guard alive for the duration of the test + + let results = &json["results"]; + assert_eq!( + results["auto_consent_extension_installed"], true, + "{TEST}: launch --auto-consent must report auto_consent_extension_installed=true: {json}" + ); + assert!( + results.get("auto_consent").is_none(), + "{TEST}: the old auto_consent field must be gone (renamed to \ + auto_consent_extension_installed, which can only claim the extension \ + was installed, never that anything was dismissed): {json}" + ); +} + +/// `live_144_no_consent_o_matic_tab_leak`: +/// +/// After `launch --auto-consent`, `tabs` must not list a +/// `Consent-O-Matic Options` entry — that synthetic extension tab is +/// filtered before sort/limit/total are computed. +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1"] +fn live_144_no_consent_o_matic_tab_leak() { + const TEST: &str = "live_144_no_consent_o_matic_tab_leak"; + if !live_tests_enabled() { + eprintln!("{TEST}: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + let Some((ff, _)) = LiveFirefox::headless_on_random_port_with_args(&["--auto-consent"]) else { + eprintln!("{TEST}: Firefox not available — skipping"); + return; + }; + + let out = Command::new(ff_rdp_bin()) + .args(base_args(ff.port())) + .arg("tabs") + .output() + .expect("run tabs"); + assert!( + out.status.success(), + "{TEST}: tabs failed — {}", + String::from_utf8_lossy(&out.stderr) + ); + let json = parse_json(&out, TEST); + let titles: Vec = json["results"] + .as_array() + .unwrap_or_else(|| panic!("{TEST}: results is not an array: {json}")) + .iter() + .map(|t| t["title"].as_str().unwrap_or_default().to_owned()) + .collect(); + assert!( + !titles.iter().any(|t| t == "Consent-O-Matic Options"), + "{TEST}: Consent-O-Matic's options tab leaked into `tabs`: {titles:?}" + ); +} + +/// `live_144_bbc_cmp_dismissed`: +/// +/// `consent accept` recognizes and clicks BBC's own (non-iframe) cookie +/// banner at `#bbccookies-continue-button`, and the control is genuinely +/// gone afterward (zero-size bounding rect), not just blindly clicked. +/// +/// Network-gated: navigates to the real `www.bbc.com`. +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1 + FF_RDP_LIVE_NETWORK_TESTS=1"] +fn live_144_bbc_cmp_dismissed() { + const TEST: &str = "live_144_bbc_cmp_dismissed"; + if !live_tests_enabled() { + eprintln!("{TEST}: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + if !live_network_tests_enabled() { + eprintln!("{TEST}: set FF_RDP_LIVE_NETWORK_TESTS=1 to run"); + return; + } + let Some(ff) = LiveFirefox::headless_on_random_port() else { + eprintln!("{TEST}: Firefox not available — skipping"); + return; + }; + + let nav = Command::new(ff_rdp_bin()) + .args(base_args(ff.port())) + .args(["navigate", "https://www.bbc.com/news"]) + .output() + .expect("run navigate"); + if !nav.status.success() { + eprintln!( + "{TEST}: navigate to www.bbc.com failed (network unavailable?) — skipping: {}", + String::from_utf8_lossy(&nav.stderr) + ); + return; + } + + let out = Command::new(ff_rdp_bin()) + .args(base_args(ff.port())) + .args(["consent", "accept"]) + .output() + .expect("run consent accept"); + assert!( + out.status.success(), + "{TEST}: consent accept failed — {}", + String::from_utf8_lossy(&out.stderr) + ); + let json = parse_json(&out, TEST); + assert_eq!( + json["results"]["cmp"], "bbc", + "{TEST}: expected the native BBC adapter to match: {json}" + ); + assert_eq!( + json["results"]["action"], "accepted", + "{TEST}: expected a real click, not just a match: {json}" + ); + + // Confirm the control is genuinely gone, not just blindly clicked. + let eval = Command::new(ff_rdp_bin()) + .args(base_args(ff.port())) + .args([ + "eval", + "(function(){var el=document.querySelector('#bbccookies-continue-button');\ + if(!el) return JSON.stringify({present:false});\ + var r=el.getBoundingClientRect();\ + return JSON.stringify({present:true,w:r.width,h:r.height});})()", + ]) + .output() + .expect("run eval"); + assert!(eval.status.success(), "{TEST}: eval failed"); + let eval_json = parse_json(&eval, TEST); + let inner: serde_json::Value = serde_json::from_str( + eval_json["results"] + .as_str() + .unwrap_or_else(|| panic!("{TEST}: eval results not a string: {eval_json}")), + ) + .unwrap_or_else(|e| panic!("{TEST}: eval result not JSON: {e}")); + let still_visible = inner["present"].as_bool().unwrap_or(false) + && inner["w"].as_f64().unwrap_or(0.0) > 0.0 + && inner["h"].as_f64().unwrap_or(0.0) > 0.0; + assert!( + !still_visible, + "{TEST}: the accept control is still visible after `consent accept` claimed \ + it was accepted: {inner}" + ); +} + +/// `live_144_full_page_no_duplicate_header`: +/// +/// A `position: fixed` header on a page tall enough to matter (8 000 px) +/// appears exactly once — as one contiguous run of matching rows — in a +/// `--full-page` capture. See the module doc for why this is a +/// forward-looking regression guard rather than a reproduced-then-fixed +/// defect: the historic duplicate could not be reproduced in this +/// environment. +const HEADER_URL: &str = "data:text/html,\ +
\ +
\ + "; + +#[test] +#[ignore = "requires Firefox + FF_RDP_LIVE_TESTS=1"] +fn live_144_full_page_no_duplicate_header() { + const TEST: &str = "live_144_full_page_no_duplicate_header"; + if !live_tests_enabled() { + eprintln!("{TEST}: set FF_RDP_LIVE_TESTS=1 to run"); + return; + } + let Some(ff) = LiveFirefox::headless_on_random_port() else { + eprintln!("{TEST}: Firefox not available — skipping"); + return; + }; + + let nav = Command::new(ff_rdp_bin()) + .args(base_args(ff.port())) + .args(["navigate", "--allow-unsafe-urls", HEADER_URL]) + .output() + .expect("run navigate"); + assert!( + nav.status.success(), + "{TEST}: navigate failed — {}", + String::from_utf8_lossy(&nav.stderr) + ); + + let out = Command::new(ff_rdp_bin()) + .args(base_args(ff.port())) + .args(["screenshot", "--full-page", "--base64"]) + .output() + .expect("run screenshot"); + assert!( + out.status.success(), + "{TEST}: screenshot --full-page failed — {}", + String::from_utf8_lossy(&out.stderr) + ); + let json = parse_json(&out, TEST); + let b64 = json["results"]["base64"] + .as_str() + .unwrap_or_else(|| panic!("{TEST}: results.base64 missing: {json}")); + let png_bytes = base64::engine::general_purpose::STANDARD + .decode(b64) + .unwrap_or_else(|e| panic!("{TEST}: base64 decode failed: {e}")); + + let img = decode_png(&png_bytes); + assert!( + img.height > 8000, + "{TEST}: expected a full-page capture taller than the 8000px body ({}px): height={}", + 8000, + img.height + ); + + let red = (255u8, 0u8, 0u8); + let runs = img.color_row_run_count(red, 40, 0.9); + assert_eq!( + runs, 1, + "{TEST}: expected the fixed red header to appear as exactly one contiguous \ + row-run in the full-page capture, found {runs} — a repeated header band \ + (image: {}x{})", + img.width, img.height + ); +} diff --git a/crates/ff-rdp-cli/tests/live/main.rs b/crates/ff-rdp-cli/tests/live/main.rs index 1d6b7e7..9358e53 100644 --- a/crates/ff-rdp-cli/tests/live/main.rs +++ b/crates/ff-rdp-cli/tests/live/main.rs @@ -58,6 +58,7 @@ 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_144_session_hygiene_followup; mod live_61l; mod live_61q_resource_bus; mod live_61r_eval; diff --git a/kb/decision-log.md b/kb/decision-log.md index 764c73b..cfffe8a 100644 --- a/kb/decision-log.md +++ b/kb/decision-log.md @@ -366,3 +366,62 @@ Diffs `tools/ralph-loop/scripts/` against `~/.claude/skills/ralph-loop/scripts/` **Consequence for Theme A**: because both sources remain reachable, `meta.source` (`"native"` | `"js-fallback"`, plus the fallback reason) is not cosmetic — it is the only way a caller can tell which tree it scored, and it must be present on every `a11y` response regardless of which path ran. **Applies to**: `ff-rdp a11y`, `a11y audit`, iter-143. + +## DEC-028: `launch`'s consent field is renamed, not fixed in place — and the freeze-for-capture pattern generalises DEC-027's "restore only what you changed" + +**Decision** (iter-144, Theme C): `launch --auto-consent`'s JSON field is renamed +`auto_consent_extension_installed` (was `auto_consent`). It is still set +unconditionally from the CLI flag — that is now honest, because the new name +only claims "the extension was installed into the profile", which `launch` +can actually know before any page loads. It no longer claims a dismiss +happened, which `launch` never could know (iteration-142's dogfooding +finding: `auto_consent: true` reported while a banner still covered the +page). A real dismiss attestation already existed and is unchanged: +`navigate --auto-consent` / `consent accept` report `results.consent = +{"cmp": ..., "action": ...}` (DEC-023), which run after a page has loaded +and can check the DOM. + +**Why rename instead of leaving the name and just documenting the caveat**: +a field named `auto_consent` reads as "consent was automatically handled" to +any caller who has not read the source — the iteration-142 finding is +exactly that misreading happening to a real dogfooding session. A false +name with an accurate docstring is still a false name to `--jq +'.results.auto_consent'`. Grepped for consumers before renaming (none in +fixtures, README, or other command source — `navigate`'s unrelated +`auto_consent` CLI flag and its own `merge_auto_consent` helper are a +different field on a different command and were not touched). + +**Also landed in this iteration**: a `NATIVE_CMP_TABLE` in `consent.rs` for +same-origin (non-iframe) CMPs — BBC's own `#bbccookies-continue-button` sits +in the top document, not behind Sourcepoint's iframe, and is tried before +`CMP_TABLE` since Sourcepoint's overlay can sit in front of it on first +paint (verified live: the element exists with a zero-size rect until +Sourcepoint is dismissed, so `native_accept_js` requires a non-zero +bounding rect, not just DOM presence, before it will click). `tabs` filters +the `Consent-O-Matic Options` tab `--auto-consent` leaves open by matching +`moz-extension://` scheme plus a table of the five titles the vendored XPI +ships (locale-defensive, since the launch-time locale pin under +investigation in [[iteration-147-console-locale-repro]] cannot yet be +trusted to hold). + +**Screenshot freeze pattern reuses DEC-027's shape** (Theme D): before a +`--full-page` capture, every `position: fixed`/`sticky` element is pinned to +`position: absolute` with its current on-screen `top`/`left` (so the +compositor never treats it as viewport-relative during the +taller-than-viewport `drawSnapshot` call), and always restored afterward — +success or capture failure — exactly the "only touch what you changed, and +always undo it" discipline DEC-027 established for the accessibility +service. Unlike DEC-027's underlying bug, the specific duplicate-header +symptom this was meant to fix (dogfooding session 63, BBC News) could not +be reproduced in the implementation environment despite a deliberate +before/after attempt across page heights 2 000–20 000 px (spanning common +GPU texture-tile boundaries) with both `fixed` and `sticky` headers, and +against the real BBC page directly — see +`live_144_full_page_no_duplicate_header`'s module doc. The freeze/restore +code is landed anyway as a defensive, unit- and live-regression-tested +mitigation matching the plan's own suggested approach, but the AC is +satisfied by a forward-looking invariant check on a deterministic local +fixture, not a reproduced-then-fixed defect. + +**Applies to**: `crates/ff-rdp-cli/src/commands/{launch,consent,tabs,screenshot}.rs`, +`crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs`, iter-144. diff --git a/kb/iterations/iteration-144-session-hygiene-followup.md b/kb/iterations/iteration-144-session-hygiene-followup.md index 3f98a02..44283c8 100644 --- a/kb/iterations/iteration-144-session-hygiene-followup.md +++ b/kb/iterations/iteration-144-session-hygiene-followup.md @@ -92,19 +92,34 @@ confirm German (or any non-English) console/error text actually appears despite non-English Firefox can be obtained in the implementation environment, defer again with a note rather than shipping a guess. -## Acceptance Criteria - -- [ ] live_144_auto_consent_field_honest: `launch --auto-consent`'s reported field never claims a - dismiss happened when `tabs`/a follow-up check shows the CMP banner still present -- [ ] live_144_bbc_cmp_dismissed: `consent accept` dismisses BBC's cookie banner - (`#bbccookies-continue-button`) on www.bbc.com -- [ ] live_144_no_consent_o_matic_tab_leak: `tabs` after `--auto-consent` does not include a - `Consent-O-Matic Options` entry (or it is filtered from the listing) -- [ ] live_144_full_page_no_duplicate_header: full-page capture of a sticky-header page (BBC - News or an equivalent fixture) has no repeated header band, verified pixel-level -- [ ] live_144_console_locale_pinned: console output is locale-stable on a genuinely - non-English-locale Firefox — name the reproduction method used in the test, since this AC - cannot be ticked without one +## Acceptance Criteria [4/5] + +- [x] live_144_auto_consent_field_honest: `launch --auto-consent`'s reported field never claims a + dismiss happened when `tabs`/a follow-up check shows the CMP banner still present — + verified: `results.auto_consent_extension_installed=true` and the old `auto_consent` key is + gone (`crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs`) +- [x] live_144_bbc_cmp_dismissed: `consent accept` dismisses BBC's cookie banner + (`#bbccookies-continue-button`) on www.bbc.com — verified live against the real site: + `results = {"cmp":"bbc","action":"accepted"}` and the control's post-click bounding rect is + zero-size (`crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs`, + network-gated on `FF_RDP_LIVE_NETWORK_TESTS=1`) +- [x] live_144_no_consent_o_matic_tab_leak: `tabs` after `--auto-consent` does not include a + `Consent-O-Matic Options` entry (or it is filtered from the listing) — verified + (`crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs`) +- [x] live_144_full_page_no_duplicate_header: full-page capture of a sticky-header page (BBC + News or an equivalent fixture) has no repeated header band, verified pixel-level — verified + against a deterministic local fixture via PNG row decoding + (`crates/ff-rdp-cli/tests/live/live_144_session_hygiene_followup.rs`); see that test's + module doc for why this lands as a forward-looking regression guard rather than a + reproduced-then-fixed defect — the historic BBC symptom could not be reproduced in this + environment despite a deliberate before/after attempt (also tried directly against the + real BBC page; no duplicate found there either). The freeze/restore mitigation is landed + regardless, per DEC-028. +- [deferred — new plan: kb/iterations/iteration-147-console-locale-repro.md] live_144_console_locale_pinned: + re-deferred a second time — this implementation environment has only an English macOS + Firefox available (checked for a `--lang` flag and a `MOZ_LOCALE`-style override; neither + exists on this build), so the symptom cannot be reproduced here either. See + [[iteration-147-console-locale-repro]] for what would unblock it. ## Notes diff --git a/kb/iterations/iteration-147-console-locale-repro.md b/kb/iterations/iteration-147-console-locale-repro.md new file mode 100644 index 0000000..9e708ea --- /dev/null +++ b/kb/iterations/iteration-147-console-locale-repro.md @@ -0,0 +1,102 @@ +--- +title: "Iteration 147: console locale reproducibility" +type: iteration +date: 2026-08-12 +status: planned +branch: iter-147/console-locale-repro +depends_on: + - kb/iterations/iteration-144-session-hygiene-followup.md +first_call_sites: [] +dogfood_path: | + ff-rdp launch --headless --port 6100 + # → against a Firefox build/langpack whose UI locale is genuinely + # non-English, console/error text must still be English (or the fix + # that makes it so must be identified and applied) +tags: [iteration] +--- + +# Iteration 147: console locale reproducibility + +Carried over from [[iteration-144-session-hygiene-followup]] Theme F, itself carried over from +[[iteration-142-session-hygiene]] — this is the second consecutive iteration this item has been +re-deferred, for the same reason both times: **no non-English-locale Firefox is available in the +implementation environment.** + +## Why this keeps getting deferred instead of guessed at + +`crates/ff-rdp-cli/src/commands/launch.rs`'s `USER_JS` constant already sets +`intl.accept_languages`, `intl.locale.requested`, and `intl.locale.matchOS` to pin English — +added in iter-61j (dogfood-51; confirmed still present via `git log -S intl.accept_languages`). +Dogfooding session 63 observed German console output anyway, on a later iteration than iter-61j. +Two live-Firefox environments were checked during iter-144's implementation: + +- macOS Firefox (`/Applications/Firefox.app`) — the only Firefox installed in this implementation + environment. English-only; ships no non-English langpack; headless Firefox has no `--lang` CLI + flag (checked `firefox --help` output — no locale-override flag exists on this build). +- No `MOZ_LOCALE`-style environment variable is documented for the installed Firefox version + (searched the running binary's `--help` and the shipped `application.ini`; none found). + +Per iteration-142 and iteration-144's own explicit run guidance, landing a "fix" without +reproducing the symptom first is exactly what these plans forbid — a pref change made without +seeing the German output firsthand cannot be verified to do anything. + +## What would unblock this + +Any of: +- A Firefox build or profile with a genuine non-English langpack installed (e.g. `de` langpack + via `about:preferences#general` → Language, or a Linux distro package that ships a localized + build by default rather than `en-US`). +- A documented `MOZ_LOCALE`/similar environment-variable override for headless Firefox, if one + exists in a Firefox version newer/older than what's checked here — re-check `firefox --help` + and `about:support` on whatever build is used to implement this plan. +- Direct access to report a diagnostic profile from the dogfooding-session-63 environment (Firefox + version, OS, langpack state) that reproduced the original German output, to compare against. + +## Tasks + +### A. Reproduce +- [ ] Obtain or build a Firefox whose UI locale is verifiably non-English (`about:support` → + "Application Basics" → confirm locale is not `en-US`). +- [ ] Run `ff-rdp launch --headless` against it and trigger a console error (e.g. + `ff-rdp eval 'undefinedFn()'`) — confirm whether the error text is English or localized despite + the `intl.*` prefs. + +### B. Diagnose (only after A succeeds) +- [ ] If still localized: identify which additional pref or mechanism the existing `intl.*` pin + misses (e.g. `general.useragent.locale` on older builds, or a required restart/profile-creation + ordering issue — prefs might apply too late if Firefox already cached the locale from the OS at + first run). Cite the Firefox source (searchfox) for whatever pref is found missing. +- [ ] If not localized: the iter-142 dogfooding-session-63 report predates the iter-61j fix, or was + itself measuring something other than console text (e.g. a localized system dialog, not RDP + eval output) — close this out as "confirmed already fixed" with the repro steps documented, + rather than landing a no-op change. + +### C. Fix (only if B finds a real gap) +- [ ] Add whatever pref/mechanism B identified to `USER_JS` (or wherever it belongs), with a + comment citing the Firefox source confirming its effect. + +## Acceptance Criteria [0/1] + +- [ ] live_147_console_locale_pinned: console output is locale-stable on a genuinely + non-English-locale Firefox — name the reproduction method used in the test. If no + non-English Firefox can be obtained, re-defer again (a third time) with a note naming what + was tried, rather than landing an unverifiable "fix" — do not tick this box without a + passing named test. + +## Design notes + +Nothing to design until Theme A (reproduction) succeeds — this plan is intentionally +investigation-first, per CLAUDE.md's "reproduce before diagnosing" rule for exactly this class of +carried-over, environment-blocked item. + +## Out of scope + +Building or CI-provisioning a non-English Firefox specifically for this repro is out of scope +unless Theme A repeatedly fails for lack of one — if so, that provisioning work should itself be +scoped as a follow-up rather than folded into this investigation. + +## References + +- [[iteration-144-session-hygiene-followup]] +- [[iteration-142-session-hygiene]] +- [[decision-log]] — DEC-028