From 3aa66745e8a4b8aae011a0f7fb688c4555880698 Mon Sep 17 00:00:00 2001
From: "Dylan M. Taylor" <dylan@dylanmtaylor.com>
Date: Sun, 8 Nov 2026 22:51:51 -0500
Subject: [PATCH] review: count only real actions, walk each field once, keep
 name selections

---
 crates/render/src/inspect/annotations.rs | 45 ++++++++++++++++++++----
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs
index c86c263fed..30f4c99098 100644
--- a/crates/render/src/inspect/annotations.rs
+++ b/crates/render/src/inspect/annotations.rs
@@ -359,18 +359,44 @@ impl<'a> Inspector<'a> {
     /// walks the same tree `field` does, bounded the same way: a field left out of the listing for
     /// want of text allowance can still have its action on a widget several levels down.
     fn has_action(&self, o: &Object, depth: u32) -> bool {
+        self.has_action_seen(o, depth, &mut HashSet::new())
+    }
+
+    /// The walk behind [`Self::has_action`]. Each indirect descendant is read once: a field tree
+    /// whose `/Kids` repeat the same objects would otherwise be walked exponentially.
+    fn has_action_seen(&self, o: &Object, depth: u32, seen: &mut HashSet<ObjectId>) -> bool {
         if depth > 32 {
             return false;
         }
+        if let Object::Reference(id) = o
+            && !seen.insert(*id)
+        {
+            return false;
+        }
         let Some(d) = self.dict(o) else { return false };
-        if d.get(b"A").is_ok() || d.get(b"AA").is_ok() {
+        if self.carries_action(&d) {
             return true;
         }
         let kids = match d.get(b"Kids").map(|k| self.resolve(k)) {
             Ok(Object::Array(k)) => k,
             _ => return false,
         };
-        kids.iter().any(|k| self.has_action(k, depth + 1))
+        kids.iter().any(|k| self.has_action_seen(k, depth + 1, seen))
+    }
+
+    /// Whether `/A` or `/AA` here holds an action. An editor can leave an empty dictionary (or
+    /// array) behind, and that is no action: a document with none must not report JavaScript. An
+    /// `/A` is an action when it carries `/S`; an `/AA` is one when an event maps to such an action.
+    fn carries_action(&self, d: &Dictionary) -> bool {
+        let action = |o: &Object| matches!(self.resolve(o), Object::Dictionary(a) if a.get(b"S").is_ok());
+        [&b"A"[..], &b"AA"[..]].iter().any(|k| match d.get(*k) {
+            Ok(o) => match self.resolve(o) {
+                Object::Dictionary(x) => x.get(b"S").is_ok() || x.iter().any(|(_, v)| action(v)),
+                Object::Array(a) => a.iter().any(action),
+                _ => false,
+            },
+            Err(_) => false,
+        })
     }
 
     pub(super) fn field(
@@ -445,11 +471,18 @@ impl<'a> Inspector<'a> {
                 let mut shown = Vec::new();
                 let mut dropped = false;
                 for x in a {
-                    if let Object::String(bytes, _) = self.resolve(x) {
-                        match self.field_spend(bytes, text_string) {
+                    match self.resolve(x) {
+                        Object::String(bytes, _) => match self.field_spend(bytes, text_string) {
                             Some(text) => shown.push(text),
                             None => dropped = true,
-                        }
+                        },
+                        // A name entry (`/V [/Opt1 (b)]`) is a selection too, as the forms reader
+                        // takes it; leaving it out reported a partial selection as complete.
+                        Object::Name(n) => match self.field_spend(n, |n| String::from_utf8_lossy(n).into_owned()) {
+                            Some(text) => shown.push(text),
+                            None => dropped = true,
+                        },
+                        _ => {}
                     }
                 }
                 // A selected value the allowance could not hold shows as an omission rather than
@@ -469,7 +502,7 @@ impl<'a> Inspector<'a> {
         // The widget is either this dict (merged field/widget) or its first kid.
         let widget = if d.get(b"Rect").is_ok() { Some(d) } else { kids.first().and_then(|k| self.dict(k)) };
         let page = widget.and_then(|w| w.get(b"P").ok()).and_then(|p| self.page_of(p)).or_else(|| self.find_widget_page(o, kids));
-        let has_actions = d.get(b"AA").is_ok() || widget.is_some_and(|w| w.get(b"AA").is_ok() || w.get(b"A").is_ok());
+        let has_actions = self.carries_action(d) || widget.is_some_and(|w| self.carries_action(w));
         let rect = widget.and_then(|w| w.get(b"Rect").ok()).map(|r| rect4(self.resolve(r)));
         out.push(Field { name: full, kind, value, page, tooltip: self.field_text(d, b"TU"), has_actions, rect });
     }