diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 88856305b..52008170c 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -3101,4 +3101,23 @@ mod tests { std::fs::set_permissions(&target, perms).unwrap(); let _ = std::fs::remove_dir_all(&dir); } + + /// A recovered document, or a new one, rotated and then undone still has never been saved: + /// `doc_close` refuses it unless told to discard it (#2346 review). + #[test] + fn closing_a_recovered_or_new_document_after_undo_is_refused() { + let mut a = Automation::new(); + let recovered = a.session.open("Report.pdf", None, Arc::new(numbered_pages(2)), None).unwrap(); + a.session.mark_recovered(recovered, Some("Report.pdf".into())); + let new = a.session.open_new("Combined.pdf", Arc::new(numbered_pages(2))).unwrap(); + for id in [recovered, new] { + json(&mut a, "page_rotate", json!({"doc": id.0, "pages": [1], "degrees": 90})); + json(&mut a, "edit_undo", json!({"doc": id.0})); + let refused = a.call("doc_close", &json!({"doc": id.0})); + assert!(matches!(&refused, Err(e) if e.to_string().contains("unsaved changes")), "{id:?}: {refused:?}"); + assert!(a.session.get(id).is_some(), "still open"); + json(&mut a, "doc_close", json!({"doc": id.0, "discard_changes": true})); + assert!(a.session.get(id).is_none()); + } + } } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f86af3de0..0a1b01828 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -305,6 +305,10 @@ struct Editor { saved: Arc>, /// Passwords that open [`Self::saved`]. saved_keys: Keys, + /// [`Self::saved`] was never written where the document belongs: a new document (combine, + /// extract, create) or one restored from recovery. It stays unsaved until a save, whatever + /// undo brings back, or closing it would lose the whole document without asking. + unsaved_file: bool, undo: Vec, redo: Vec, keys: Keys, @@ -3542,6 +3546,7 @@ impl Session { cos, saved, saved_keys: keys.clone(), + unsaved_file: false, undo: Vec::new(), redo: Vec::new(), keys, @@ -3839,7 +3844,7 @@ impl Session { /// file is clean; undo past a later save is not, because the restored snapshot's own base /// is an earlier revision. fn sync_dirty(doc: &mut Document) { - doc.dirty = doc.editor.as_ref().is_some_and(|e| e.cos.is_modified() || !Arc::ptr_eq(e.cos.bytes(), &e.saved)); + doc.dirty = doc.editor.as_ref().is_some_and(|e| e.unsaved_file || e.cos.is_modified() || !Arc::ptr_eq(e.cos.bytes(), &e.saved)); } /// Use the current state's password for viewing (protection edits change it). @@ -4124,6 +4129,7 @@ impl Session { pdfcraft_cos::Document::open_with_password(bytes.clone(), keys.reopen.as_deref()).map_err(|e| EditError::Reopen(e.to_string()))?; editor.saved = bytes; editor.saved_keys = keys; + editor.unsaved_file = false; } if let Some(p) = path { doc.name = std::path::Path::new(&p).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| p.clone()); @@ -4187,7 +4193,7 @@ impl Session { editor.undo.clear(); editor.redo.clear(); Self::adopt_keys(doc); - doc.dirty = false; + Self::sync_dirty(doc); doc.generation += 1; Self::refresh(doc).inspect_err(|_| Self::show_released(doc)) } @@ -4667,6 +4673,9 @@ impl Session { let id = self.open(name, None, bytes, None)?; if let Some(d) = self.docs.iter_mut().find(|d| d.id == id) { d.dirty = true; + if let Some(e) = d.editor.as_mut() { + e.unsaved_file = true; + } // Unsaved work with no edit yet still needs its first recovery snapshot. d.generation += 1; } @@ -4719,6 +4728,9 @@ impl Session { if let Some(d) = self.docs.iter_mut().find(|d| d.id == id) { d.path = path; d.dirty = true; + if let Some(e) = d.editor.as_mut() { + e.unsaved_file = true; + } d.generation += 1; d.snapshot_generation = d.generation; // its bytes are already in the recovery store } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 61c99bb70..204e75c82 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5911,3 +5911,31 @@ fn text_blocks_stay_cached_after_an_edit_and_follow_undo_and_redo() { assert_eq!(redone[0].editor_text, "Edited"); assert!(Arc::ptr_eq(&redone, &blocks(&session))); } + +/// A new document, or one restored from recovery, has never been written where it belongs: undo +/// back to the bytes it was opened from must not make it clean, or closing it would discard the +/// whole document without asking. Saving makes it clean; Revert of a saved one too. +#[test] +fn undo_never_makes_an_unsaved_document_clean() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let new = s.open_new("Combined.pdf", Arc::new(fixture(2))).unwrap(); + s.apply(new, Edit::RotatePages { pages: vec![0], degrees: 90 }).unwrap(); + s.undo(new).unwrap(); + assert!(s.get(new).unwrap().dirty, "a new document is unsaved until it is saved"); + assert_eq!(s.autosave_snapshots().len(), 1, "and still needs recovery"); + s.revert(new).unwrap(); + assert!(s.get(new).unwrap().dirty, "Revert goes back to the created bytes, which aren't saved anywhere"); + + let recovered = s.open("Report.pdf", None, Arc::new(fixture(2)), None).unwrap(); + s.mark_recovered(recovered, Some("Report.pdf".into())); + s.apply(recovered, Edit::RotatePages { pages: vec![0], degrees: 90 }).unwrap(); + s.undo(recovered).unwrap(); + assert!(s.get(recovered).unwrap().dirty, "the recovered work was never saved"); + + let saved = s.save_bytes(new).unwrap(); + s.mark_saved(new, saved, Some("Combined.pdf".into())).unwrap(); + assert!(!s.get(new).unwrap().dirty); + s.apply(new, Edit::RotatePages { pages: vec![0], degrees: 90 }).unwrap(); + s.undo(new).unwrap(); + assert!(!s.get(new).unwrap().dirty, "once saved, undo back to the saved state is clean"); +}