From cace97e7b2607d31e703bb04c17fa7975f40ec39 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 20:46:00 +0000 Subject: [PATCH] M4: one text style can't be given to two bookmark levels A line goes to the first level whose style it is in, so the same style for two levels, or one within the size tolerance of the other, left the later level empty without a word. The edit now refuses it, for the dialog and automation alike, naming the two levels (CodeRabbit on #4560). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GbuSKAS2k5248LGFEapMUR --- crates/engine/src/edit/run.rs | 10 ++ crates/engine/src/tests.rs | 150 ++++++++++++---------- crates/engine/src/text_style_bookmarks.rs | 2 +- 3 files changed, 95 insertions(+), 67 deletions(-) diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 8734c372b7..703d1b2cc0 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -296,6 +296,16 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E if levels.is_empty() { return Err(EditError::Invalid("choose a text style for at least one bookmark level".into())); } + // Each line goes to the first level whose style it is in: a style given again, or one + // within the size tolerance of an earlier level's, would leave its level empty. + for (i, a) in levels.iter().enumerate() { + if let Some(j) = levels.iter().skip(i.saturating_add(1)).position(|b| a.matches(b)) { + let (first, second) = (i.saturating_add(1), i.saturating_add(j).saturating_add(2)); + return Err(EditError::Invalid(format!( + "bookmark levels {first} and {second} have the same text style: choose a different one for each level" + ))); + } + } let entries = crate::text_style_bookmarks::headings(doc, levels); if entries.is_empty() { return Err(EditError::Invalid("no text is set in the chosen styles".into())); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 5dbcb7d81b..6439ba6363 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1927,6 +1927,24 @@ fn bookmarks_from_text_styles_nest_the_headings_and_undo() { assert!(s.get(id).unwrap().info.outline.is_empty()); } +/// CodeRabbit on #4560: a line goes to the first level whose style it is in, so the same style +/// for two levels (or one within the size tolerance of the other) would leave the later level +/// empty. Refused at the edit, for the dialog and automation alike; nothing changes. +#[test] +fn bookmarks_from_text_styles_refuse_one_style_for_two_levels() { + let mut s = Session::new(); + let id = s.open("styled.pdf", None, Arc::new(styled_headings_fixture()), None).unwrap(); + let styles = s.get(id).unwrap().text_styles(); + let (chapter, section) = (styles[0].style.clone(), styles[1].style.clone()); + let near = pdfcraft_engine_style(§ion, section.size + 0.1); + for levels in [vec![chapter.clone(), chapter.clone()], vec![chapter.clone(), section.clone(), near]] { + let n = levels.len(); + let err = s.apply(id, Edit::BookmarksFromTextStyles { levels }).unwrap_err(); + assert!(err.to_string().contains(&format!("levels {} and {n} have the same text style", n - 1)), "{err}"); + assert!(s.get(id).unwrap().info.outline.is_empty()); + } +} + /// CodeRabbit on #4560: bookmark titles from text styles are the document's text, so a document /// that forbids copying lists no styles and refuses the edit, as it refuses other extraction. #[test] @@ -8849,69 +8867,69 @@ fn pdf_with_annots(annots: &[&[u8]]) -> Vec { bytes } -/// ISO 32000-2 Table 224 (`/NeedAppearances`) and Table 192 (`/MK /CA`): a form that leaves its -/// appearances to the viewer shows its check boxes and radio buttons drawn from `/MK`, over the -/// filled squares the file carries: a checked box its check mark (ZapfDingbats `4`), an unchecked -/// one its empty border, a radio button its dot (`l`), on screen and printed as an image. Without -/// `/NeedAppearances` the file's own squares show. Saving writes the file as it was. -#[test] -fn need_appearances_buttons_are_displayed_from_their_captions() { - let pdf = |need: bool| { - let flag = if need { "/NeedAppearances true " } else { "" }; - format!( - "%PDF-1.7\n\ - 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >> endobj\n\ - 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ - 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 400 200] /Annots [6 0 R 7 0 R 8 0 R] >> endobj\n\ - 4 0 obj << /Fields [6 0 R 7 0 R 8 0 R] {flag}/DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> endobj\n\ - 5 0 obj << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> endobj\n\ - 6 0 obj << /Type /Annot /Subtype /Widget /FT /Btn /T (on) /V /Yes /AS /Yes /DA (/Helv 0 Tf 0 g) /MK << /BC [0 0 0] /CA (4) >> /Rect [20 100 60 140] /F 4 /P 3 0 R /AP << /N << /Yes 9 0 R /Off 9 0 R >> >> >> endobj\n\ - 7 0 obj << /Type /Annot /Subtype /Widget /FT /Btn /T (off) /V /Off /AS /Off /DA (/Helv 0 Tf 0 g) /MK << /BC [0 0 0] /CA (4) >> /Rect [100 100 140 140] /F 4 /P 3 0 R /AP << /N << /Yes 9 0 R /Off 9 0 R >> >> >> endobj\n\ - 8 0 obj << /Type /Annot /Subtype /Widget /FT /Btn /Ff 32768 /T (radio) /V /B /AS /B /DA (/Helv 0 Tf 0 g) /MK << /BC [0 0 0] /CA (l) >> /Rect [180 100 220 140] /F 4 /P 3 0 R /AP << /N << /B 9 0 R /Off 9 0 R >> >> >> endobj\n\ - 9 0 obj << /Type /XObject /Subtype /Form /BBox [0 0 40 40] /Length 18 >> stream\n0 g 4 4 32 32 re f\nendstream endobj\n\ - trailer << /Root 1 0 R >>\n%%EOF" - ) - .into_bytes() - }; - let render = |bytes: Arc>| { - let mut r = pdfcraft_render::PageRenderer::new(bytes, pdfcraft_render::RenderConfig::default()); - r.render(pdfcraft_render::RenderRequest { page: 0, scale: 1.0, ..Default::default() }) - }; - // Dark pixels of the page (drawn at scale 1, 400 x 200) in the user-space box x0..x1, y0..y1. - let ink = |page: &pdfcraft_render::RenderedPage, (x0, y0, x1, y1): (u32, u32, u32, u32)| { - let (px, _) = page.rgba.as_chunks::<4>(); - (200 - y1..200 - y0) - .flat_map(|y| (x0..x1).map(move |x| (x, y))) - .filter(|&(x, y)| px.get((y * page.width + x) as usize).is_some_and(|p| p[0] < 128 && p[1] < 128 && p[2] < 128)) - .count() - }; - // In a widget whose lower-left corner is (x, y): its whole inside, the top-left of its inside - // (where a check mark leaves the box empty), and the lower-left corner (outside a dot). - let inside = |x: u32, y: u32| (x + 6, y + 6, x + 34, y + 34); - let top_left = |x: u32, y: u32| (x + 6, y + 26, x + 14, y + 34); - let corner = |x: u32, y: u32| (x + 5, y + 5, x + 9, y + 9); - let mut s = Session::new(); - let id = s.open("form.pdf", None, Arc::new(pdf(true)), None).unwrap(); - let shown = render(s.get(id).unwrap().display.clone()); - let raster = pdfcraft_print::Raster { dpi: 72.0, grayscale: false }; - let pictures = crate::print_raster::rasterize(s.get(id).unwrap(), &[0], &raster, pdfcraft_print::Content::Document).unwrap(); - let printed = render(Arc::new(pictures)); - for (page, how) in [(&shown, "shown"), (&printed, "printed")] { - let (check, check_corner) = (ink(page, inside(20, 100)), ink(page, top_left(20, 100))); - assert!(check > 100 && check < 600, "{how}: a check mark, not the 28 x 28 square: {check}"); - assert_eq!(check_corner, 0, "{how}: the check mark leaves the top left empty"); - assert_eq!(ink(page, inside(100, 100)), 0, "{how}: the unchecked box is empty"); - assert!(ink(page, (100, 100, 140, 102)) > 30, "{how}: and keeps its border"); - let dot = ink(page, (188, 108, 212, 132)); - assert!(dot > 300 && ink(page, corner(180, 100)) == 0, "{how}: the radio button's dot: {dot}"); - } - let saved = s.save_bytes(id).unwrap(); - assert!(saved.windows(18).any(|w| w == b"0 g 4 4 32 32 re f"), "the file keeps its own appearances"); - - // Without /NeedAppearances the file's filled squares are shown. - let id = s.open("plain.pdf", None, Arc::new(pdf(false)), None).unwrap(); - let shown = render(s.get(id).unwrap().display.clone()); - for x in [20, 100, 180] { - assert!(ink(&shown, inside(x, 100)) > 700, "the file's square at {x}"); - } -} +/// ISO 32000-2 Table 224 (`/NeedAppearances`) and Table 192 (`/MK /CA`): a form that leaves its +/// appearances to the viewer shows its check boxes and radio buttons drawn from `/MK`, over the +/// filled squares the file carries: a checked box its check mark (ZapfDingbats `4`), an unchecked +/// one its empty border, a radio button its dot (`l`), on screen and printed as an image. Without +/// `/NeedAppearances` the file's own squares show. Saving writes the file as it was. +#[test] +fn need_appearances_buttons_are_displayed_from_their_captions() { + let pdf = |need: bool| { + let flag = if need { "/NeedAppearances true " } else { "" }; + format!( + "%PDF-1.7\n\ + 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 400 200] /Annots [6 0 R 7 0 R 8 0 R] >> endobj\n\ + 4 0 obj << /Fields [6 0 R 7 0 R 8 0 R] {flag}/DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> endobj\n\ + 5 0 obj << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> endobj\n\ + 6 0 obj << /Type /Annot /Subtype /Widget /FT /Btn /T (on) /V /Yes /AS /Yes /DA (/Helv 0 Tf 0 g) /MK << /BC [0 0 0] /CA (4) >> /Rect [20 100 60 140] /F 4 /P 3 0 R /AP << /N << /Yes 9 0 R /Off 9 0 R >> >> >> endobj\n\ + 7 0 obj << /Type /Annot /Subtype /Widget /FT /Btn /T (off) /V /Off /AS /Off /DA (/Helv 0 Tf 0 g) /MK << /BC [0 0 0] /CA (4) >> /Rect [100 100 140 140] /F 4 /P 3 0 R /AP << /N << /Yes 9 0 R /Off 9 0 R >> >> >> endobj\n\ + 8 0 obj << /Type /Annot /Subtype /Widget /FT /Btn /Ff 32768 /T (radio) /V /B /AS /B /DA (/Helv 0 Tf 0 g) /MK << /BC [0 0 0] /CA (l) >> /Rect [180 100 220 140] /F 4 /P 3 0 R /AP << /N << /B 9 0 R /Off 9 0 R >> >> >> endobj\n\ + 9 0 obj << /Type /XObject /Subtype /Form /BBox [0 0 40 40] /Length 18 >> stream\n0 g 4 4 32 32 re f\nendstream endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF" + ) + .into_bytes() + }; + let render = |bytes: Arc>| { + let mut r = pdfcraft_render::PageRenderer::new(bytes, pdfcraft_render::RenderConfig::default()); + r.render(pdfcraft_render::RenderRequest { page: 0, scale: 1.0, ..Default::default() }) + }; + // Dark pixels of the page (drawn at scale 1, 400 x 200) in the user-space box x0..x1, y0..y1. + let ink = |page: &pdfcraft_render::RenderedPage, (x0, y0, x1, y1): (u32, u32, u32, u32)| { + let (px, _) = page.rgba.as_chunks::<4>(); + (200 - y1..200 - y0) + .flat_map(|y| (x0..x1).map(move |x| (x, y))) + .filter(|&(x, y)| px.get((y * page.width + x) as usize).is_some_and(|p| p[0] < 128 && p[1] < 128 && p[2] < 128)) + .count() + }; + // In a widget whose lower-left corner is (x, y): its whole inside, the top-left of its inside + // (where a check mark leaves the box empty), and the lower-left corner (outside a dot). + let inside = |x: u32, y: u32| (x + 6, y + 6, x + 34, y + 34); + let top_left = |x: u32, y: u32| (x + 6, y + 26, x + 14, y + 34); + let corner = |x: u32, y: u32| (x + 5, y + 5, x + 9, y + 9); + let mut s = Session::new(); + let id = s.open("form.pdf", None, Arc::new(pdf(true)), None).unwrap(); + let shown = render(s.get(id).unwrap().display.clone()); + let raster = pdfcraft_print::Raster { dpi: 72.0, grayscale: false }; + let pictures = crate::print_raster::rasterize(s.get(id).unwrap(), &[0], &raster, pdfcraft_print::Content::Document).unwrap(); + let printed = render(Arc::new(pictures)); + for (page, how) in [(&shown, "shown"), (&printed, "printed")] { + let (check, check_corner) = (ink(page, inside(20, 100)), ink(page, top_left(20, 100))); + assert!(check > 100 && check < 600, "{how}: a check mark, not the 28 x 28 square: {check}"); + assert_eq!(check_corner, 0, "{how}: the check mark leaves the top left empty"); + assert_eq!(ink(page, inside(100, 100)), 0, "{how}: the unchecked box is empty"); + assert!(ink(page, (100, 100, 140, 102)) > 30, "{how}: and keeps its border"); + let dot = ink(page, (188, 108, 212, 132)); + assert!(dot > 300 && ink(page, corner(180, 100)) == 0, "{how}: the radio button's dot: {dot}"); + } + let saved = s.save_bytes(id).unwrap(); + assert!(saved.windows(18).any(|w| w == b"0 g 4 4 32 32 re f"), "the file keeps its own appearances"); + + // Without /NeedAppearances the file's filled squares are shown. + let id = s.open("plain.pdf", None, Arc::new(pdf(false)), None).unwrap(); + let shown = render(s.get(id).unwrap().display.clone()); + for x in [20, 100, 180] { + assert!(ink(&shown, inside(x, 100)) > 700, "the file's square at {x}"); + } +} diff --git a/crates/engine/src/text_style_bookmarks.rs b/crates/engine/src/text_style_bookmarks.rs index 07a0cacd78..5969188d67 100644 --- a/crates/engine/src/text_style_bookmarks.rs +++ b/crates/engine/src/text_style_bookmarks.rs @@ -24,7 +24,7 @@ pub struct TextStyle { impl TextStyle { /// Whether `other` is this style. Sizes within a quarter point match, as the same heading /// size is often written with rounding differences (12 and 11.96). - fn matches(&self, other: &TextStyle) -> bool { + pub(crate) fn matches(&self, other: &TextStyle) -> bool { self.font == other.font && self.bold == other.bold && self.italic == other.italic && (self.size - other.size).abs() <= SIZE_TOLERANCE } }