iter-140: element targeting — refs, ambiguous selectors, frame diagnostics

crates/ff-rdp-cli/src/cli/args.rs

@@ -701,6 +701,15 @@ top-level attempt and the scan (e.g. --frame sourcepoint). If the selector
 matches nowhere, the error names how many frames were tried and their URLs
 instead of a bare timeout.
 
+When a selector matches more than one element, the default (flag-less)
+behaviour clicks DOM-order index 0 — which may be hidden. Use --visible to
+click the first non-hidden match instead, or --index N to pick a specific
+match (0-based). Mutually exclusive with each other. On success, the output
+gains {\"match_count\": N, \"chosen_index\": N}; on failure (no visible match /
+index out of range), the error names the match count. If the plain selector
+times out because it resolved to a hidden element, the error itself suggests
+--visible/--index with the observed match count.
+
 Output: {\"results\": {\"clicked\": true, \"tag\": \"...\", \"text\": \"...\", \"frame_url\": null}, \"total\": 1, \"meta\": {\"frame_url\": null, ...}}
 `frame_url` is always present (never omitted) — null when the click landed on
 the top-level document, the frame's URL string when it landed inside a frame.
@@ -725,6 +734,12 @@ The value is set via the native HTMLInputElement/HTMLTextAreaElement/HTMLSelectE
 prototype setter so React/Vue/Svelte value trackers are invalidated, and `input`
 and `change` events are dispatched after the assignment.
 
+When a selector matches more than one element (e.g. two `input[name=keywords]`
+on the same page, one hidden), the default (flag-less) behaviour types into
+DOM-order index 0. Use --visible to target the first non-hidden match instead,
+or --index N for a specific match (0-based). Mutually exclusive with each
+other. On success, the output gains {\"match_count\": N, \"chosen_index\": N}.
+
 Output: {\"results\": {\"typed\": true, \"tag\": \"INPUT\", \"value\": \"...\"}, \"total\": 1, \"meta\": {...}}")]
     Type(TypeArgs),
     /// Wait for a condition to become true (polls every 100ms).
@@ -1031,6 +1046,12 @@ Output (--all): full resolved-style object per match (dumps every property)"
     #[command(
         long_about = "Inspect CSS styles for an element matching a CSS selector.
 
+When a selector matches more than one element, use --visible to inspect the
+first non-hidden match instead of the default DOM-order index 0, or --index N
+for a specific match (0-based). Mutually exclusive with each other; resolved
+before styles are read, so the reported selector/rules are for the chosen
+element only.
+
 Output (computed):  {\"results\": [{\"selector\": \"...\", \"computed\": {\"color\": \"...\", ...}}], \"total\": N, \"meta\": {...}}
 Output (--applied): {\"results\": [{\"selector\": \"...\", \"rules\": [{\"selector\": \"...\", \"properties\": [...]}]}], \"total\": N, \"meta\": {...}}
 Output (--layout):  {\"results\": [{\"selector\": \"...\", \"box\": {\"margin\": {...}, \"border\": {...}, \"padding\": {...}, \"content\": {...}}}], \"total\": N, \"meta\": {...}}"
@@ -1605,6 +1626,15 @@ pub struct ClickArgs {
     /// top-level attempt and the frame scan (e.g. --frame sourcepoint)
     #[arg(long, value_name = "URL_SUBSTRING")]
     pub frame: Option<String>,
+    /// When the selector matches more than one element, click the first
+    /// *visible* one instead of blindly taking DOM-order index 0. Mutually
+    /// exclusive with --index.
+    #[arg(long, conflicts_with = "index")]
+    pub visible: bool,
+    /// When the selector matches more than one element, click the Nth match
+    /// (0-based), regardless of visibility. Mutually exclusive with --visible.
+    #[arg(long, value_name = "N", conflicts_with = "visible")]
+    pub index: Option<usize>,
 }
 
 #[derive(clap::Args)]
@@ -1639,6 +1669,15 @@ pub struct TypeArgs {
     /// After typing, wait for network and DOM to idle
     #[arg(long)]
     pub settle: bool,
+    /// When the selector matches more than one element, type into the first
+    /// *visible* one instead of blindly taking DOM-order index 0. Mutually
+    /// exclusive with --index.
+    #[arg(long, conflicts_with = "index")]
+    pub visible: bool,
+    /// When the selector matches more than one element, type into the Nth
+    /// match (0-based), regardless of visibility. Mutually exclusive with --visible.
+    #[arg(long, value_name = "N", conflicts_with = "visible")]
+    pub index: Option<usize>,
 }
 
 #[derive(clap::Args)]
@@ -1967,6 +2006,15 @@ pub struct StylesArgs {
     /// Comma-separated list of CSS property names to include (computed mode only)
     #[arg(long, value_delimiter = ',', conflicts_with_all = ["applied", "layout"])]
     pub properties: Option<Vec<String>>,
+    /// When the selector matches more than one element, inspect the first
+    /// *visible* one instead of blindly taking DOM-order index 0. Mutually
+    /// exclusive with --index.
+    #[arg(long, conflicts_with = "index")]
+    pub visible: bool,
+    /// When the selector matches more than one element, inspect the Nth match
+    /// (0-based), regardless of visibility. Mutually exclusive with --visible.
+    #[arg(long, value_name = "N", conflicts_with = "visible")]
+    pub index: Option<usize>,
 }
 
 #[derive(clap::Args)]

crates/ff-rdp-cli/src/commands/click.rs

@@ -20,8 +20,9 @@ use crate::output_pipeline::OutputPipeline;
 
 use super::connect_tab::{ConnectedTab, connect_and_get_target};
 use super::js_helpers::{
-    DispatchMode, JSON_SENTINEL, WaitForPredicate, autowait_element, build_click_js,
-    escape_selector, resolve_result, settle_page, wait_for_predicates,
+    DispatchMode, JSON_SENTINEL, MatchPolicy, WaitForPredicate, autowait_element, build_click_js,
+    escape_selector, resolve_disambiguated_target, resolve_result, settle_page,
+    wait_for_predicates,
 };
 use super::network_events::build_network_entries;
 
@@ -43,6 +44,11 @@ pub struct ClickOptions<'a> {
     /// substring, skipping both the top-level attempt and the frame scan.
     /// `None` runs the default top-level-first, scan-on-not-found behaviour.
     pub frame: Option<&'a str>,
+    /// iter-140 Theme C: `--visible` / `--index N` — disambiguate a selector
+    /// that matches more than one element before doing anything else. `None`
+    /// (the default, flag-less path) is completely unchanged: DOM-order index
+    /// 0, same timing, same JS.
+    pub match_policy: Option<MatchPolicy>,
 }
 
 impl Default for ClickOptions<'_> {
@@ -55,6 +61,7 @@ impl Default for ClickOptions<'_> {
             wait_for_timeout_ms: None,
             settle: false,
             frame: None,
+            match_policy: None,
         }
     }
 }
@@ -97,6 +104,28 @@ pub fn run_core(
     let wait_timeout_ms = opts.wait_timeout_ms.unwrap_or(cli.timeout);
     let console_actor = ctx.target.console_actor.clone();
 
+    // iter-140 Theme C: `--visible`/`--index` resolve an ambiguous selector to
+    // a single, genuinely-unique element selector up front, so every step
+    // below (auto-wait, the click itself) acts on exactly the element the
+    // flag named instead of blindly taking DOM-order index 0. Flag-less calls
+    // skip this entirely — see `ClickOptions::match_policy`'s doc comment.
+    let resolved_selector;
+    let mut disambiguation: Option<(usize, usize)> = None; // (match_count, chosen_index)
+    let selector: &str = if let Some(policy) = opts.match_policy {
+        let target = resolve_disambiguated_target(
+            &mut ctx,
+            &console_actor,
+            selector,
+            policy,
+            wait_timeout_ms,
+        )?;
+        disambiguation = Some((target.match_count, target.chosen_index));
+        resolved_selector = target.selector;
+        &resolved_selector
+    } else {
+        selector
+    };
+
     // A1: Auto-wait for element readiness (unless --no-wait).
     //
     // iter-129: `autowait_element` only ever polled the top-level document.
@@ -195,6 +224,14 @@ pub fn run_core(
     if let Some(sm) = settle_method {
         result["settle_method"] = json!(sm.as_meta_str());
     }
+    // iter-140 Theme B/C: when --visible/--index disambiguated an ambiguous
+    // selector, report how many elements matched and which was chosen — the
+    // same transparency the plan asks for on the failure path, surfaced here
+    // on success so `--visible`/`--index` calls aren't silent about it.
+    if let Some((match_count, chosen_index)) = disambiguation {
+        result["match_count"] = json!(match_count);
+        result["chosen_index"] = json!(chosen_index);
+    }
     Ok(result)
 }
 
@@ -213,14 +250,16 @@ pub fn run(
     let settle_method = result
         .as_object_mut()
         .and_then(|o| o.remove("settle_method"));
-    // iter-129: `frame_url` moves to `meta` the same way — always present
-    // (null or a URL string), never omitted.
-    let frame_url = result.as_object_mut().and_then(|o| o.remove("frame_url"));
+    // iter-140 Theme E: `--help` documents `frame_url` as present in BOTH
+    // `results` AND `meta` (never omitted from either) — the code used to
+    // `.remove()` it from `results` here, so `--jq '.results.frame_url'`
+    // threw on every call. Copy instead of removing so it stays in both.
+    let frame_url = result.get("frame_url").cloned().unwrap_or(Value::Null);
     let mut meta = json!({"selector": selector});
     if let Some(sm) = settle_method {
         meta["settle_method"] = sm;
     }
-    meta["frame_url"] = frame_url.unwrap_or(Value::Null);
+    meta["frame_url"] = frame_url;
     crate::connection_meta::merge_into_if_verbose(
         &mut meta,
         &cli.host,
@@ -394,6 +433,14 @@ fn build_click_js_for_mode(escaped_selector: &str, mode: DispatchMode) -> String
 /// does not throw. Returns a descriptive `AppError::User` — never the bare
 /// upstream timeout — when `frame_filter` matches no frame, or when every
 /// candidate frame's eval still throws the not-found error.
+// iter-140 Theme D: on a many-frame page (theguardian.com: 97 frames, most
+// of them consent-string-laden ad iframe URLs) joining every URL raw
+// produced a 65 KB error message. Cap both the number of URLs listed and
+// each URL's length (reusing iter-128's `middle_ellipsis`, already wired
+// into the same shape of problem for `perf`/`network` — see iter-139).
+const MAX_LISTED_FRAME_URLS: usize = 10;
+const FRAME_URL_MAX_LEN: usize = 80;
+
 fn click_in_scanned_frame(
     ctx: &mut ConnectedTab,
     selector: &str,
@@ -410,21 +457,37 @@ fn click_in_scanned_frame(
         })
         .collect();
 
-    let all_urls = || -> String {
-        targets
+    let bounded_urls = |targets: &[&TargetEvent]| -> String {
+        let total = targets.len();
+        let listed: Vec<String> = targets
             .iter()
-            .map(|t| t.url.as_deref().unwrap_or("<no-url>"))
-            .collect::<Vec<_>>()
-            .join(", ")
+            .take(MAX_LISTED_FRAME_URLS)
+            .map(|t| {
+                crate::output::middle_ellipsis(
+                    t.url.as_deref().unwrap_or("<no-url>"),
+                    FRAME_URL_MAX_LEN,
+                )
+            })
+            .collect();
+        if total > MAX_LISTED_FRAME_URLS {
+            format!(
+                "{} (+{} more)",
+                listed.join(", "),
+                total - MAX_LISTED_FRAME_URLS
+            )
+        } else {
+            listed.join(", ")
+        }
     };
 
     if let Some(filter) = frame_filter
         && candidates.is_empty()
     {
+        let all: Vec<&TargetEvent> = targets.iter().collect();
         return Err(AppError::User(format!(
             "click --frame '{filter}' matched no frame ({} frame(s) available: {})",
             targets.len(),
-            all_urls()
+            bounded_urls(&all)
         )));
     }
 
@@ -451,11 +514,17 @@ fn click_in_scanned_frame(
 
     // Nothing matched anywhere — the informative, frame-aware diagnostic
     // that replaces the old bare "element not found" / 10s timeout.
-    let n = targets.len();
+    //
+    // iter-140 Theme D: this must count `candidates` — the frames actually
+    // tried — not `targets.len()`. With `--frame guim` on a 97-frame page,
+    // `--frame` narrows the scan to a handful of candidates; reporting
+    // "matched in 0 of 97 frames" claimed every frame was tried when only the
+    // filtered subset was.
+    let tried = candidates.len();
+    let total = targets.len();
     Err(AppError::User(format!(
-        "click: selector {selector:?} matched in 0 of {n} frames (top + {} subframes: {})",
-        n.saturating_sub(1),
-        all_urls()
+        "click: selector {selector:?} matched in 0 of {tried} frame(s) tried (of {total} total): {}",
+        bounded_urls(&candidates)
     )))
 }
 

crates/ff-rdp-cli/src/commands/dom.rs

@@ -9,7 +9,9 @@ use crate::output_controls::{OutputControls, SortDir};
 use crate::output_pipeline::OutputPipeline;
 
 use super::connect_tab::connect_and_get_target;
-use super::js_helpers::{JSON_SENTINEL, escape_selector, eval_or_bail, resolve_result};
+use super::js_helpers::{
+    JSON_SENTINEL, UNIQUE_SELECTOR_JS_FN, escape_selector, eval_or_bail, resolve_result,
+};
 
 #[derive(Debug, Clone, Copy)]
 pub enum OutputMode {
@@ -37,7 +39,15 @@ pub enum OutputMode {
 /// The ref ID is injected by the Rust caller as a counter (`__REF_START__`).
 /// Actionable attributes only: id, name, type, href, aria-*, data-state, role,
 /// placeholder, value (for inputs).
+///
+/// `__UNIQUE_SELECTOR_FN__` is replaced with [`js_helpers::UNIQUE_SELECTOR_JS_FN`]
+/// (iter-140 Theme A) — each node's `__resolver` is that function's output for
+/// the matched element, a genuine CSS selector, not a `querySelectorAll(sel)[i]`
+/// JS expression. The old expression form round-tripped into
+/// `document.querySelector('...')` call sites (`click`/`type`/`styles`/etc.)
+/// as a double-nested, invalid selector string — see the plan's Theme A bug #1.
 const ARIA_TREE_JS_TEMPLATE: &str = r"(function() {
+  __UNIQUE_SELECTOR_FN__
   var ACTIONABLE_ATTRS = ['id','name','type','href','placeholder','value',
     'aria-label','aria-expanded','aria-hidden','aria-haspopup','aria-selected',
     'aria-checked','aria-disabled','aria-controls','aria-describedby',
@@ -106,8 +116,10 @@ const ARIA_TREE_JS_TEMPLATE: &str = r"(function() {
       node.hasShadowRoot = true;
       node.shadowMode = sr.mode || 'open';
     }
-    // Resolver expression: re-selects this element by its querySelectorAll index.
-    node.__resolver = 'document.querySelectorAll(\'__SELECTOR__\')[' + i + ']';
+    // Resolver: a genuinely-unique CSS selector for this element (iter-140
+    // Theme A), safe to feed straight back into `document.querySelector` /
+    // `DomWalkerActor::query_selector` from any later `--ref e<N>` call.
+    node.__resolver = __ffrdpUniqueSelector(el);
     results.push(node);
   }
   if (results.length === 1) return '__FF_RDP_JSON__' + JSON.stringify(results[0]);
@@ -442,6 +454,7 @@ fn build_js_with_ref_start(selector: &str, mode: OutputMode, ref_start: u64) ->
             ARIA_TREE_JS_TEMPLATE
                 .replace("__SELECTOR__", &escaped)
                 .replace("__REF_START__", &ref_start.to_string())
+                .replace("__UNIQUE_SELECTOR_FN__", UNIQUE_SELECTOR_JS_FN)
         }
         OutputMode::OuterHtml => format!(
             r"(function() {{

crates/ff-rdp-cli/src/commands/index.rs

@@ -372,11 +372,19 @@ fn crawl_page(
         .filter(|s| !s.is_empty());
 
     // Extract forms via our JS template.
-    let forms_json = eval_js_string(cli, form_extraction_js_template()).unwrap_or_default();
+    let forms_json = eval_js_string(
+        cli,
+        &form_extraction_js_template(crate::commands::js_helpers::UNIQUE_SELECTOR_JS_FN),
+    )
+    .unwrap_or_default();
     let forms = parse_forms_from_json(&forms_json);
 
     // Extract landmarks via our JS template.
-    let landmarks_json = eval_js_string(cli, landmark_extraction_js_template()).unwrap_or_default();
+    let landmarks_json = eval_js_string(
+        cli,
+        &landmark_extraction_js_template(crate::commands::js_helpers::UNIQUE_SELECTOR_JS_FN),
+    )
+    .unwrap_or_default();
     let landmarks = parse_landmarks_from_json(&landmarks_json);
 
     // Extract outgoing links.