M3: a closed top-level bookmark opens closed (ISO 32000-2 Table 151 /Count) (#4389)

The Bookmarks panel opened every top-level bookmark whatever its /Count
said, so a bookmark with a negative /Count showed its children. Every
level now starts as its /Count says (positive open; negative or missing
closed). The reader's toggles and a search still override it.

Moving a bookmark into a different parent (Indent) now opens that
parent, as adding one already did; the forced top-level open used to
hide that it stayed closed.


Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/organize/src/outline.rs

@@ -553,10 +553,7 @@ pub fn add_bookmark_at(
     let at = index.min(kids.len());
     kids.insert(at, item);
     relink(doc, parent, &kids)?;
-    // Show the new bookmark: open its parent (the root is always open).
-    if !parent_path.is_empty() && doc.get(parent).as_dict().and_then(|d| d.int(b"Count")).is_none_or(|c| c <= 0) {
-        put(doc, parent, b"Count", Some(Object::Int(1)))?;
-    }
+    show_children(doc, parent, parent_path)?;
     recount(doc)?;
     let mut path = parent_path.to_vec();
     path.push(at);
@@ -699,11 +696,23 @@ pub fn move_bookmark(doc: &mut Document, from: &[usize], to_parent: &[usize], in
     let at = index.min(kids.len());
     kids.insert(at, r);
     relink(doc, parent, &kids)?;
+    if parent != old_parent {
+        show_children(doc, parent, &to_parent)?;
+    }
     recount(doc)?;
     to_parent.push(at);
     Ok(to_parent)
 }
 
+/// Open `parent` so a bookmark just added or moved into it shows (the root is always open).
+/// Only the sign of `/Count` matters here: [`recount`] sets its value.
+fn show_children(doc: &mut Document, parent: ObjRef, parent_path: &[usize]) -> Result<()> {
+    if !parent_path.is_empty() && doc.get(parent).as_dict().and_then(|d| d.int(b"Count")).is_none_or(|c| c <= 0) {
+        put(doc, parent, b"Count", Some(Object::Int(1)))?;
+    }
+    Ok(())
+}
+
 /// Expand or collapse a bookmark that has children.
 pub fn set_bookmark_open(doc: &mut Document, path: &[usize], open: bool) -> Result<()> {
     let (r, _) = resolve_path(doc, path)?;

crates/organize/src/tests.rs

@@ -1748,6 +1748,18 @@ fn bookmarks_add_rename_move_delete() {
     assert_eq!(crate::move_bookmark(&mut d, &[0], &[2], 9).unwrap(), vec![1, 1]);
     assert_eq!(titles(&crate::bookmarks(&d)), ["Intro", "Results[Table,Figure 1]"]);
     assert_eq!(crate::move_bookmark(&mut d, &[1], &[1, 0], 0), Err(crate::OutlineError::IntoItself));
+    // Into a bookmark with no children yet (Indent): it opens, so the moved one shows.
+    assert_eq!(crate::move_bookmark(&mut d, &[1, 1], &[0], 0).unwrap(), vec![0, 0]);
+    let intro = crate::bookmarks(&d)[0].obj;
+    assert_eq!(count_of(&d, intro), Some(1));
+    // Reordering inside a closed parent leaves it closed.
+    crate::add_bookmark(&mut d, &[0], 1, "Second", 0).unwrap();
+    crate::set_bookmark_open(&mut d, &[0], false).unwrap();
+    crate::move_bookmark(&mut d, &[0, 1], &[0], 0).unwrap();
+    assert_eq!(count_of(&d, intro), Some(-2));
+    crate::delete_bookmark(&mut d, &[0, 0]).unwrap();
+    crate::move_bookmark(&mut d, &[0, 0], &[1], 9).unwrap();
+    assert_eq!(titles(&crate::bookmarks(&d)), ["Intro", "Results[Table,Figure 1]"]);
     crate::delete_bookmark(&mut d, &[1, 0]).unwrap();
     assert_eq!(titles(&crate::bookmarks(&d)), ["Intro", "Results[Figure 1]"]);
 

crates/render/src/inspect.rs

@@ -405,6 +405,9 @@ pub struct OutlineItem {
     /// Where on `page` the bookmark goes ([`DestView::Top`] for a bookmark without a page).
     pub view: DestView,
     pub children: Vec<OutlineItem>,
+    /// Whether the bookmark starts open: only a positive `/Count` says so (ISO 32000-2 §12.3.3,
+    /// Table 151). A negative one is closed, and so is a missing one, which a bookmark with
+    /// children should not have.
     pub open: bool,
     /// The title's colour (`/C`, DeviceRGB, ISO 32000-2 §12.3.3 Table 151), or `None` for the
     /// default black or a value that isn't three numbers.

crates/ui-egui/src/panels.rs

@@ -1139,7 +1139,10 @@ fn outline_item(ui: &mut egui::Ui, t: &Tokens, info: &DocInfo, item: &OutlineIte
     if let Some(levels) = cx.expand {
         ui.data_mut(|d| d.insert_temp(id, depth < levels));
     }
-    let mut open = cx.searching || ui.data(|d| d.get_temp::<bool>(id)).unwrap_or(item.open || depth == 0);
+    // A bookmark opens as the file says at every level: a positive /Count is open, a negative
+    // one (or none) closed (ISO 32000-2 §12.3.3, Table 151). The reader's own toggles and a
+    // search override it.
+    let mut open = cx.searching || ui.data(|d| d.get_temp::<bool>(id)).unwrap_or(item.open);
     let x_text = indent + 20.0;
     if let Some((rpath, text)) = cx.rename.as_mut()
         && rpath.as_slice() == path

crates/ui-egui/tests/it/bookmarks.rs

@@ -77,6 +77,7 @@ fn new_rename_reorder_indent_delete_and_undo() {
 
     menu(&mut h, "Results", "Indent");
     assert_eq!(outline(&h), [("Intro".into(), Some(0), 0), ("Results".into(), Some(2), 1)]);
+    h.get_by_label("Results"); // its new parent opened, so it still shows
     menu(&mut h, "Results", "Outdent");
     menu(&mut h, "Results", "Move up");
     assert_eq!(outline(&h)[0].0, "Results");
@@ -310,3 +311,60 @@ fn style_and_colour_from_the_bookmark_menu() {
     h.run_steps(3);
     assert!(look(&h).0, "undoable");
 }
+
+/// Top-level bookmarks open as the file says, like every other level: a positive /Count shows
+/// the children, a negative one hides them until the reader opens it (ISO 32000-2 §12.3.3,
+/// Table 151). They used to open regardless.
+#[test]
+fn a_closed_top_level_bookmark_opens_closed() {
+    const OUTLINED: &[u8] = b"%PDF-1.7
+1 0 obj << /Type /Catalog /Pages 2 0 R /Outlines 10 0 R >> endobj
+2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >> endobj
+3 0 obj << /Type /Page /Parent 2 0 R >> endobj
+10 0 obj << /Type /Outlines /First 11 0 R /Last 13 0 R /Count 5 >> endobj
+11 0 obj << /Title (Closed parent) /Parent 10 0 R /Next 12 0 R /First 21 0 R /Last 22 0 R /Count -2 /Dest [3 0 R /Fit] >> endobj
+12 0 obj << /Title (Open parent) /Parent 10 0 R /Prev 11 0 R /Next 13 0 R /First 23 0 R /Last 24 0 R /Count 2 /Dest [3 0 R /Fit] >> endobj
+13 0 obj << /Title (Uncounted parent) /Parent 10 0 R /Prev 12 0 R /First 25 0 R /Last 25 0 R /Dest [3 0 R /Fit] >> endobj
+21 0 obj << /Title (Closed child A) /Parent 11 0 R /Next 22 0 R /Dest [3 0 R /Fit] >> endobj
+22 0 obj << /Title (Closed child B) /Parent 11 0 R /Prev 21 0 R /Dest [3 0 R /Fit] >> endobj
+23 0 obj << /Title (Open child A) /Parent 12 0 R /Next 24 0 R /Dest [3 0 R /Fit] >> endobj
+24 0 obj << /Title (Open child B) /Parent 12 0 R /Prev 23 0 R /Dest [3 0 R /Fit] >> endobj
+25 0 obj << /Title (Uncounted child) /Parent 13 0 R /Dest [3 0 R /Fit] >> endobj
+trailer << /Root 1 0 R >>
+%%EOF";
+    let mut h = harness();
+    h.state_mut().open_bytes("outlined.pdf", None, OUTLINED.to_vec()).unwrap();
+    h.run_steps(4);
+    let shown = |h: &Harness<'static, PdfCraftApp>, t: &str| h.query_by_label(t).is_some();
+    assert!(shown(&h, "Closed parent") && shown(&h, "Open parent") && shown(&h, "Uncounted parent"));
+    assert!(!shown(&h, "Closed child A") && !shown(&h, "Closed child B"), "a negative /Count opens closed");
+    assert!(shown(&h, "Open child A") && shown(&h, "Open child B"), "a positive /Count opens open");
+    assert!(!shown(&h, "Uncounted child"), "only a positive /Count opens a bookmark");
+
+    // The disclosure triangle at the row's left still opens and closes it.
+    let toggle = |h: &mut Harness<'static, PdfCraftApp>, title: &str| {
+        let row = h.get_by_label(title).rect();
+        let at = egui::pos2(row.left() + 8.0, row.top() + 14.0);
+        h.hover_at(at);
+        h.run_steps(1);
+        h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE });
+        h.event(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
+        h.run_steps(3);
+    };
+    toggle(&mut h, "Closed parent");
+    assert!(shown(&h, "Closed child A") && shown(&h, "Closed child B"), "opened by the reader");
+    toggle(&mut h, "Closed parent");
+    assert!(!shown(&h, "Closed child A"), "closed again");
+    toggle(&mut h, "Open parent");
+    assert!(!shown(&h, "Open child A"), "an open bookmark closes");
+
+    // A search still shows the matches inside a closed bookmark, and clearing it restores it.
+    h.get_by_label("Search").click();
+    h.run_steps(1);
+    h.get_by_label("Search").type_text("child B");
+    h.run_steps(3);
+    assert!(shown(&h, "Closed child B") && shown(&h, "Open child B"));
+    h.get_by_label("Clear").click();
+    h.run_steps(3);
+    assert!(!shown(&h, "Closed child B") && !shown(&h, "Open child B"), "search leaves the open state alone");
+}

parity/acrobat-features.toml

@@ -3363,8 +3363,8 @@ milestone = "M4"
 status = "shipped"
 commands = ["file.properties"]
 tools = []
-evidence = ["crates/ui-egui/tests/it/bookmarks.rs::expanding_and_collapsing_all_bookmarks"]
-notes = "Bookmarks panel options: expand all, expand top-level, collapse all"
+evidence = ["crates/ui-egui/tests/it/bookmarks.rs::expanding_and_collapsing_all_bookmarks", "crates/ui-egui/tests/it/bookmarks.rs::a_closed_top_level_bookmark_opens_closed", "crates/organize/src/tests.rs::bookmarks_add_rename_move_delete"]
+notes = "Bookmarks panel options: expand all, expand top-level, collapse all. Each bookmark, top level included, opens as its /Count says (positive open; negative or missing closed, ISO 32000-2 Table 151); the triangle toggles it for the session and a search shows matches inside closed ones. Adding or moving a bookmark into another opens that one. Missing: the open/closed state the reader sets is not written back to /Count on save"
 
 [[feature]]
 id = "organize.bookmarks-from-structure"