diff --git a/crates/engine/src/cmd/export/pdf.rs b/crates/engine/src/cmd/export/pdf.rs index 12d5577ec..b5f16c110 100644 --- a/crates/engine/src/cmd/export/pdf.rs +++ b/crates/engine/src/cmd/export/pdf.rs @@ -541,6 +541,76 @@ mod overset_tests { assert!(story["overset"].is_null() && story["oversetChars"].is_null(), "{story}"); } + /// `overset` indexes `text` in UTF-8 bytes, not characters: a client that slices a JavaScript + /// string at it cuts the wrong place once the text is not ASCII (noted in review). + #[test] + fn overset_is_a_byte_offset_into_the_text() { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 300, "height": 200, "pages": 1, "facingPages": false, "margins": 0})).unwrap(); + // Accented letters are two bytes each, so the byte offset runs ahead of the character index. + let text = "Réservé à l'évaluation détaillée des éléments.\nCette phrase dépasse le cadre et n'est pas dans le PDF exporté du tout."; + let frame = s.execute("frame.create", &json!({"rect": [20, 20, 280, 40], "content": "text", "text": text})).unwrap(); + let story = s.execute("story.get", &json!({"story": frame["story"].clone()})).unwrap(); + let at = story["overset"].as_u64().expect("it oversets") as usize; + assert!(text.is_char_boundary(at), "a byte offset that splits no character: {at}"); + assert!(at > text[..at].chars().count(), "ahead of the character index: {at} bytes, {} characters", text[..at].chars().count()); + // And `oversetChars` counts characters, which is why the two numbers differ. + assert_eq!(story["oversetChars"], json!(text[at..].chars().count()), "{story}"); + } + + /// A parent page's frame is composed once per page that shows it: its page number marker is + /// that page's, so the same frame fits on page 1 and oversets on page 10 -- and a parent page + /// no exported page uses puts nothing in the file to be missing. Composing it once with no + /// page reported neither: every selection got the same unpaged warning (noted in review). + #[test] + fn a_parent_frame_is_checked_on_the_pages_that_show_it() { + let about = |r: &serde_json::Value| -> Vec { + r["warnings"] + .as_array() + .map(|w| w.iter().filter_map(|w| w.as_str()).filter(|w| w.contains("not in the export")).map(str::to_string).collect()) + .unwrap_or_default() + }; + + // A parent page that only page 1 uses. + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 300, "height": 200, "pages": 2, "facingPages": false, "margins": 0})).unwrap(); + s.execute("layout.pages.applyParent", &json!({"pages": [1], "parent": null})).unwrap(); + let text = "First line is visible.\nSecond paragraph does not fit: this sentence carries the legal notice and must not vanish from the PDF without a warning."; + let frame = s + .execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": [20, 20, 280, 60], "content": "text", "text": text})) + .unwrap(); + let id = frame["id"].as_u64().unwrap(); + + let w = about(&s.execute("file.exportPdf", &json!({"pages": [1]})).unwrap()); + assert_eq!(w.len(), 1, "page 1 shows the parent: {w:?}"); + assert!(w[0].contains("page 1") && w[0].contains(&format!("frame {id}")) && w[0].contains("76 character"), "{}", w[0]); + + // Page 2 has no parent, so nothing of that frame is in its export to be missing. + let r = s.execute("file.exportPdf", &json!({"pages": [2]})).unwrap(); + assert!(about(&r).is_empty(), "page 2 does not use the parent: {r}"); + + // Page-dependent text: the marker is one digit up to page 9 and two from page 10, and the + // wider line no longer fits. 112 pt is the width where that is the only difference. + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 300, "height": 200, "pages": 12, "facingPages": false, "margins": 0})).unwrap(); + let text = format!("Continued on page {}", designcraft_doc::story::PAGE_NUMBER); + s.execute("frame.create", &json!({"spread": {"kind": "parent", "index": 0}, "rect": [20, 20, 132, 36], "content": "text", "text": text})) + .unwrap(); + + for page in [1, 9] { + let r = s.execute("file.exportPdf", &json!({"pages": [page]})).unwrap(); + assert!(about(&r).is_empty(), "page {page} fits one digit: {r}"); + } + let w = about(&s.execute("file.exportPdf", &json!({"pages": [10]})).unwrap()); + assert_eq!(w.len(), 1, "two digits do not fit: {w:?}"); + assert!(w[0].contains("page 10"), "{}", w[0]); + + // The whole document: one line naming every page it happens on, not one line per page. + let w = about(&s.execute("file.exportPdf", &json!({})).unwrap()); + assert_eq!(w.len(), 1, "one line for the story: {w:?}"); + assert!(w[0].starts_with("pages 10, 11, 12:"), "{}", w[0]); + } + /// A spread export puts the whole spread on the sheet, so choosing one page of it exports its /// partner too -- and that partner's overset is just as absent from the file. Reporting only /// the pages that were named left it unmentioned (noted in review of #1465). diff --git a/crates/engine/src/cmd/preflight.rs b/crates/engine/src/cmd/preflight.rs index b3a22d6ad..bf98f9ab9 100644 --- a/crates/engine/src/cmd/preflight.rs +++ b/crates/engine/src/cmd/preflight.rs @@ -121,16 +121,76 @@ fn overset_location(s: &Session, target: designcraft_doc::ItemId) -> Option<(des /// 0-based pages being exported; a story whose overset is on a page that is not being exported is /// not reported, and one with no page of its own always is. A reflowable export (EPUB, HTML) or a /// text export carries the whole story, so neither calls this. +/// Pages a single warning names before "and N more". +const MAX_WARNED_PAGES: usize = 10; + +/// How the pages one warning covers are named: "page 3", "pages 3, 4", "pages 3, 4 and 6 more". +fn page_list(pages: &[usize]) -> String { + let shown: Vec = pages.iter().take(MAX_WARNED_PAGES).map(|p| (p + 1).to_string()).collect(); + let rest = pages.len() - shown.len(); + let list = shown.join(", "); + match (pages.len(), rest) { + (1, _) => format!("page {list}"), + (_, 0) => format!("pages {list}"), + _ => format!("pages {list} and {rest} more"), + } +} + +/// The parent spread and the page of it a frame sits on, or `None` for a frame on a document page. +fn parent_page_of(d: &Document, target: designcraft_doc::ItemId) -> Option<(usize, usize)> { + let loc = d.find(target)?; + let SpreadRef::Parent(pi) = loc.spread else { return None }; + let bounds = d.parent_xf(&loc).transform_rect_bbox(d.item(target)?.bounds()); + Some((pi, d.spread(loc.spread)?.page_at_x(bounds.center().x).unwrap_or(0))) +} + +/// Document pages that draw parent page `pp` of parent spread `pi`, in document order. +fn pages_showing(d: &Document, (pi, pp): (usize, usize)) -> Vec { + (0..d.page_count()) + .filter(|p| d.page_loc(*p).is_some_and(|(si, pgi)| d.shown_parent_pages(SpreadRef::Doc(si), pgi).contains(&(pi, pp)))) + .collect() +} + +/// Characters of `story` that `cs` had no room for. +fn overset_chars(story: &designcraft_doc::Story, cs: &ComposedStory) -> usize { + story.text.get(cs.overset_at.unwrap_or(0)..).unwrap_or("").chars().count() +} + pub fn overset_warnings(s: &Session, pages: Option<&[usize]>) -> Vec { let Some(st) = s.active() else { return vec![] }; let d = &st.doc; let mut out = Vec::new(); for story in d.stories.values() { + let last = story.frames.last().copied(); + // A story on a parent page is composed once per page that shows it: its page number + // markers and variables are that page's, so the same frame can fit on one page and + // overset on another, and a parent page no exported page uses puts nothing in the file + // to be missing. Composing it once with no page reported neither (noted in review). + if let Some(parent) = last.and_then(|f| parent_page_of(d, f)) { + let mut by_count: std::collections::BTreeMap> = std::collections::BTreeMap::new(); + for page in pages_showing(d, parent) { + if pages.is_some_and(|sel| !sel.contains(&page)) { + continue; + } + let cs = s.cache.get(d, story.id, Some(page)); + let n = overset_chars(story, &cs); + if cs.is_overset() && n > 0 { + by_count.entry(n).or_default().push(page); + } + } + let item = last.map_or(0, |f| f.0); + out.extend( + by_count.into_iter().map(|(n, ps)| { + format!("{}: {n} character(s) of the story in frame {item} do not fit it and are not in the export", page_list(&ps)) + }), + ); + continue; + } let cs = s.cache.get(d, story.id, None); if !cs.is_overset() { continue; } - let n = story.text.get(cs.overset_at.unwrap_or(0)..).unwrap_or("").chars().count(); + let n = overset_chars(story, &cs); if n == 0 { continue; } @@ -138,7 +198,6 @@ pub fn overset_warnings(s: &Session, pages: Option<&[usize]>) -> Vec { out.push(format!("a story with no text frame: {n} character(s) of it are not in the export")); continue; } - let last = story.frames.last().copied(); let location = last.and_then(|f| overset_location(s, f)); let page = location.and_then(|(_, page)| page); if let (Some(page), Some(pages)) = (page, pages) diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 3e6dce9d5..01abf0e6e 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -191,7 +191,7 @@ pub fn specs() -> Vec { }) } ), - cmd!(query "story.get", "Get Story", [], None, "{story? | frame?} → text, frames, paragraphs, vertical, lines, overset (the index in `text` where it stops fitting its frames, null when it all fits) and oversetChars (how many characters that leaves, as preflight reports them)", has_doc, |s, p| { + cmd!(query "story.get", "Get Story", [], None, "{story? | frame?} → text, frames, paragraphs, vertical, lines, overset (the UTF-8 byte offset into `text` where it stops fitting its frames, not a character index, null when it all fits) and oversetChars (how many characters that leaves, as preflight reports them)", has_doc, |s, p| { let st = s.doc()?; let sid = story_of(s, p).ok_or_else(|| bad("story.get", "no story"))?; let mut v = st.doc.story_summary(sid).ok_or(designcraft_doc::DocError::NoStory(sid))?;