diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index 7758dc7c85..55eec00959 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -8,6 +8,15 @@ use super::{ }; use crate::{EditNotice, MarkKind}; +/// Whether the edit, or any edit inside a batch, deletes pages. +fn deletes_pages(edit: &Edit) -> bool { + match edit { + Edit::DeletePages { .. } => true, + Edit::Batch { edits, .. } => edits.iter().any(deletes_pages), + _ => false, + } +} + impl Session { /// Apply an edit. On success the previous state is undoable and the view data is refreshed. pub fn apply(&mut self, id: DocId, edit: Edit) -> Result<(), EditError> { @@ -163,6 +172,11 @@ impl Session { // Signed documents are only ever saved incrementally: an edit that needs a full rewrite // (applying redactions, changing security, sanitizing) would invalidate the signatures. // So does any edit to a file rebuilt from object headers: it has no chain to append to. + // Deleted pages remain in earlier incremental revisions (#3407), so rewrite + // unsigned files in full. Signed files must retain their signed bytes. + if !signed && deletes_pages(&edit) { + next.require_full_save(); + } let rewrites = |c: &pdfcraft_cos::Document| c.full_save_required() || c.encryption_changed(); if signed && ((rewrites(&next) && !rewrites(&editor.cos)) || (next.is_modified() && next.revisions().is_empty())) { return Err(EditError::SignedRewrite(edit.label().to_string())); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 333d0fd498..6fc0df2f27 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -934,9 +934,10 @@ fn revert_preserves_the_saved_pages_after_undo() { #[test] fn full_save_is_readable_and_smaller_after_deletes() { let (mut s, id) = session_with(5); + let original = s.save_bytes(id).unwrap(); s.apply(id, Edit::DeletePages { pages: vec![0, 1, 2] }).unwrap(); let full = s.save_full_bytes(id).unwrap(); - assert!(full.len() < s.save_bytes(id).unwrap().len()); + assert!(full.len() < original.len()); let mut s2 = Session::new(); let id2 = s2.open("full.pdf", None, full, None).unwrap(); assert_eq!(page_texts(&s2, id2), ["Page 4", "Page 5"]); @@ -9409,3 +9410,62 @@ fn repeated_backdrop_artwork_survives_background_undo_redo_and_both_save_modes() s.apply(rewritten, Edit::RemoveMarks { kind: MarkKind::Background }).unwrap(); black(&s, rewritten); } + +fn contains(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|w| w == needle) +} + +#[test] +fn saving_after_deleting_pages_rewrites_the_file_without_them() { + let (mut s, id) = session_with(6); + let original = s.save_bytes(id).unwrap(); + s.apply(id, Edit::DeletePages { pages: vec![1, 2, 3, 4] }).unwrap(); + let saved = s.save_bytes(id).unwrap(); + assert!(saved.len() < original.len(), "the file shrank: {} -> {}", original.len(), saved.len()); + assert!(!saved.starts_with(original.as_slice()), "a full rewrite, not an update appended to the old file"); + assert!(!contains(&saved, b"Page 3"), "a deleted page's text is not left in an earlier revision"); + let mut again = Session::new(); + let id2 = again.open("saved.pdf", None, saved, None).unwrap(); + assert_eq!(page_texts(&again, id2), ["Page 1", "Page 6"]); +} + +#[test] +fn saving_after_deleting_pages_from_a_signed_file_stays_incremental() { + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = sign::pkcs12::open(&p12, "test").unwrap(); + let mut s = Session::new().with_clock(|| 1_800_000_000); + let id = s.open("fixture.pdf", None, Arc::new(fixture(3)), None).unwrap(); + let opts = SignOptions { page: 0, rect: Some([20.0, 20.0, 160.0, 50.0]), ..SignOptions::default() }; + let signed = s.sign(id, &digital_id, opts).unwrap(); + s.mark_signed(id, signed.clone(), None).unwrap(); + s.apply(id, Edit::DeletePages { pages: vec![2] }).unwrap(); + let saved = s.save_bytes(id).unwrap(); + assert!(saved.starts_with(signed.as_ref()), "the signed bytes stay an incremental update"); + assert!(s.get(id).unwrap().is_signed()); +} + +#[test] +fn saving_after_a_batch_that_deletes_pages_rewrites_the_file() { + let (mut s, id) = session_with(6); + let original = s.save_bytes(id).unwrap(); + let batch = Edit::Batch { + label: "Delete and rotate".into(), + edits: vec![ + Edit::Batch { label: "Delete pages".into(), edits: vec![Edit::DeletePages { pages: vec![1, 2, 3, 4] }] }, + Edit::RotatePages { pages: vec![0], degrees: 90 }, + ], + }; + s.apply(id, batch).unwrap(); + let saved = s.save_bytes(id).unwrap(); + assert!(!saved.starts_with(original.as_slice()), "a full rewrite, not an update appended to the old file"); + assert!(!contains(&saved, b"Page 3"), "a deleted page's text is not left in an earlier revision"); +} + +#[test] +fn saving_without_deleting_pages_stays_incremental() { + let (mut s, id) = session_with(3); + let original = s.save_bytes(id).unwrap(); + s.apply(id, Edit::RotatePages { pages: vec![0], degrees: 90 }).unwrap(); + let saved = s.save_bytes(id).unwrap(); + assert!(saved.starts_with(original.as_slice()), "an edit that deletes nothing is still appended"); +} diff --git a/crates/optimize/tests/optimize.rs b/crates/optimize/tests/optimize.rs index 2e3ea2d42f..683cbd0684 100644 --- a/crates/optimize/tests/optimize.rs +++ b/crates/optimize/tests/optimize.rs @@ -1775,7 +1775,8 @@ fn monochrome_images_keep_what_they_should() { optimize(&mut doc, &Settings::default()).unwrap(); assert_eq!([alone, deep].map(|r| raw(&doc, r)), before, "a Group 4 halftone alone, and sixteen-bit grey"); assert_eq!(filters(&doc, wrapped), 1, "a wrapped Group 4 halftone is stored again alone by default"); - assert_eq!(stream(&doc, scan).dict.name(b"Filter"), Some(&b"CCITTFaxDecode"[..]), "the scan: Group 4 is smaller"); + assert!(matches!(stream(&doc, scan).filters().as_slice(), [(Filter::CcittFax | Filter::Flate, _)]), "the scan: Group 4 or Flate"); + assert_eq!(doc.decoded_image(&stream(&doc, scan)).unwrap(), ragged, "the scan preserves every sample"); assert_eq!((stream(&doc, fax).dict.name(b"Filter"), filters(&doc, fax)), (Some(&b"CCITTFaxDecode"[..]), 1), "the busy fax image"); let (mut doc, [_, wrapped, scan, _, _]) = build(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 0b6f658dd6..5f52e8fbc9 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1287,7 +1287,7 @@ status = "shipped" commands = [] tools = [] evidence = ["crates/ui-egui/tests/it/view.rs::pages_sidebar_select_all_keeps_current_page_and_keyboard_scope", "crates/ui-egui/tests/it/editing.rs::dragging_pages_panel_thumbnails_reorders_pages", "crates/ui-egui/tests/it/viewer.rs::pages_panel_thumbnails_have_a_context_menu", "crates/ui-egui/tests/it/editing.rs::split_from_pages_panel_context_menu", "crates/ui-egui/tests/it/editing.rs::deleting_pages_from_the_pages_panel_menu", "crates/ui-egui/tests/it/editing.rs::the_delete_key_deletes_pages_picked_in_the_pages_panel_but_never_every_page"] -notes = "Ctrl/Cmd+A selects every page while the Pages sidebar owns the keyboard, without navigation; Escape clears that selection. Viewer and text-field shortcuts keep their own scope. Drag the selection (or the page grabbed) to reorder, with a drop bar and edge scrolling; right-click offers Split document…, Extract pages, Cut, Copy, Paste after and Delete pages (also the Delete key over the panel). Not yet: Rotate, Insert and Page labels in the menu, dragging pages between documents, or dropping files onto the panel." +notes = "Ctrl/Cmd+A selects every page while the Pages sidebar owns the keyboard, without navigation; Escape clears that selection. Viewer and text-field shortcuts keep their own scope. Drag the selection (or the page grabbed) to reorder, with a drop bar and edge scrolling; right-click offers Split document…, Extract pages, Cut, Copy, Paste after and Delete pages (also the Delete key over the panel). Deleting pages makes the next Save a full rewrite (unsigned files), so the deleted pages leave the file; signed files stay incremental. Not yet: Rotate, Insert and Page labels in the menu, dragging pages between documents, or dropping files onto the panel." [[feature]] id = "view.bookmarks-panel"