fix(engine): a new or recovered document stays unsaved through undo (#2346)

* fix(engine): a new or recovered document stays unsaved through undo

#1435 made undo back to the last saved bytes clean. For a document
created by Combine, Extract or Create, or restored from recovery, those
bytes were never written where the document belongs, so edit then Undo
made it clean: Close no longer asked to save and autosave stopped keeping
it. Editor::unsaved_file, set by open_new and mark_recovered and cleared
by mark_saved, keeps such a document unsaved until it is saved; Revert
follows the same rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz

* fix(engine): test that doc_close refuses a recovered or new document after undo

Review follow-up on #2346: a recovered document and a new one, rotated
and undone, are refused by doc_close without discard_changes, and close
with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
mtlcl410
2026-10-18 03:47:43 -04:00
committed by GitHub
co-authored by Claude Opus 5.5 Brandon Thomas
parent 36d239d2b1
commit 94f1a0d157
3 changed files with 61 additions and 2 deletions
+19
View File
@@ -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());
}
}
}
+14 -2
View File
@@ -305,6 +305,10 @@ struct Editor {
saved: Arc<Vec<u8>>,
/// 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<Snapshot>,
redo: Vec<Snapshot>,
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
}
+28
View File
@@ -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");
}