diff --git a/crates/organize/src/outline.rs b/crates/organize/src/outline.rs index 4bcfe93ae..cc1bbb088 100644 --- a/crates/organize/src/outline.rs +++ b/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)?; diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 8fc0fa50d..1ba8861cb 100644 --- a/crates/organize/src/tests.rs +++ b/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]"]); diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 4972ad689..827e67856 100644 --- a/crates/render/src/inspect.rs +++ b/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, + /// 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. diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index 44f132ac9..a7820c2d0 100644 --- a/crates/ui-egui/src/panels.rs +++ b/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::(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::(id)).unwrap_or(item.open); let x_text = indent + 20.0; if let Some((rpath, text)) = cx.rename.as_mut() && rpath.as_slice() == path diff --git a/crates/ui-egui/tests/it/bookmarks.rs b/crates/ui-egui/tests/it/bookmarks.rs index 1cf78958f..be8c5151b 100644 --- a/crates/ui-egui/tests/it/bookmarks.rs +++ b/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"); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 276ade35a..2ae6221c7 100644 --- a/parity/acrobat-features.toml +++ b/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"