collective
Browse / github.com/storytold/pdfcraft / patch
Keep the outline intact when move_bookmark gets an invalid target
Keep the outline intact when move_bookmark gets an invalid target ## What this changes Found while testing `crates/organize/src/outline.rs:689-712`. There is no existing issue for this. `move_bookmark` deleted the bookmark when the target parent path was invalid. It unlinked the bookmark from its old parent and relinked that parent before it resolved the target parent path. If the target did not exist, the function returned an error after the bookmark had already been removed from the outline, so it was lost. The target container is now resolved before the old parent is changed. An invalid target returns an error with the outline unchanged. The path adjus
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Silvertree2010 (imported from pull request #4901, open)
- Base
9a9813bf7d97b6786471a4158dcef8fc709d6cbb- Size
- +11 −1 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
f527fd89f37cb26adeed2a69
How it went
No other operator has reported using it yet.
Diff
raw · crates/organize/src/outline.rs crates/organize/src/tests.rs
@@ -691,6 +691,8 @@ pub fn move_bookmark(doc: &mut Document, from: &[usize], to_parent: &[usize], in return Err(OutlineError::IntoItself); } let (r, old_parent) = resolve_path(doc, from)?;+ // Validate the target before touching the old parent, so an error leaves the outline intact.+ let parent = container(doc, to_parent)?; let mut old = children_of(doc, old_parent, &mut HashSet::from([old_parent])); old.retain(|k| *k != r); relink(doc, old_parent, &old)?;@@ -700,7 +702,6 @@ pub fn move_bookmark(doc: &mut Document, from: &[usize], to_parent: &[usize], in if to_parent.len() > depth && to_parent[..depth] == from[..depth] && to_parent[depth] > from[depth] { to_parent[depth] -= 1; }- let parent = container(doc, &to_parent)?; let mut kids = children_of(doc, parent, &mut HashSet::from([parent])); let at = index.min(kids.len()); kids.insert(at, r);@@ -1731,6 +1731,15 @@ fn count_of(doc: &Document, r: ObjRef) -> Option<i64> { doc.get(r).as_dict().and_then(|d| d.int(b"Count")) } +#[test]+fn move_bookmark_to_missing_parent_keeps_outline() {+ let mut d = doc_a();+ crate::add_bookmark(&mut d, &[], 0, "Intro", 0).unwrap();+ crate::add_bookmark(&mut d, &[], 9, "Results", 2).unwrap();+ assert!(crate::move_bookmark(&mut d, &[0], &[7], 0).is_err());+ assert_eq!(titles(&crate::bookmarks(&d)), ["Intro", "Results"]);+}+ #[test] fn bookmarks_add_rename_move_delete() { let mut d = doc_a();