An export says what text its pages had no room for (#1465)

A page-based export leaves out the text its frames cannot fit, and said
nothing about it: an imported 58-page form lost whole sentences -- a
data-consent clause, a statutory rule, the last row of two tables -- with
`warnings` empty while preflight reported 19 overset stories.

The PDF and deck exports now report it, in the words preflight uses and with
the same count, naming the page and the frame. A reflowable export carries the
whole story, so it has nothing to report.

Carries the review fix: a spread export puts the whole spread on the sheet, so
choosing page 2 of a facing-pages document exports page 3 as well and page 3's
overset is as absent from the file as page 2's would be. The selection is
widened to the spreads it touches first. A page on another spread is still not
reported, so the filter keeps doing what it is for.

Rebased onto the post-split main: the PDF command is in export/pdf.rs, the
deck one in export/formats.rs, and preflight.rs is unchanged by the split. The
helper sits before export_pdf rather than between first_page_ref's doc comment
and its function, which is where the pre-split patch had put it.
This commit is contained in:
Hamza committed 2026-10-28 18:53:45 +05:00
1 parent 2d8207ed3c
commit 2f403698e0
4 files changed
+175 -2

No files matched your search

+2
View File
@@ -110,6 +110,8 @@ pub(super) fn export_pptx(s: &mut Session, p: &Value) -> Result<Value> {
if rasterized > 0 {
r.warnings.push(format!("{rasterized} object(s) went in as images"));
}
// A slide has no room for the overset text either (#1465).
r.warnings.extend(crate::cmd::preflight::overset_warnings(s, opts.pages.as_deref()));
match str_param(p, "path") {
Some(path) => {
#[cfg(not(target_arch = "wasm32"))]
+122 -1
View File
@@ -87,8 +87,27 @@ fn first_page_ref(pdf: &[u8]) -> Option<usize> {
s[i..].split_whitespace().next()?.parse().ok()
}
/// Every page sharing a spread with one of `pages` (0-based), for an export that puts whole
/// spreads on a sheet. The pages come back in document order.
fn with_their_spreads(s: &Session, pages: &[usize]) -> Vec<usize> {
let Some(st) = s.active() else { return pages.to_vec() };
let d = &st.doc;
let spreads: std::collections::BTreeSet<usize> = pages.iter().filter_map(|p| d.page_loc(*p)).map(|(si, _)| si).collect();
if spreads.is_empty() {
return pages.to_vec();
}
(0..d.page_count()).filter(|p| d.page_loc(*p).is_some_and(|(si, _)| spreads.contains(&si))).collect()
}
pub(super) fn export_pdf(s: &mut Session, p: &Value) -> Result<Value> {
let r = pdf_export(s, p)?;
let mut r = pdf_export(s, p)?;
// Text the pages have no room for is not in the file: say so, as preflight does (#1465).
// A spread export puts the whole spread on the sheet, so choosing page 2 exports page 3 as
// well and page 3's overset is as absent from the file as page 2's would be. The selection
// is widened to the spreads it touches before the warnings are collected (noted in review).
let opts = options(p, s.doc()?.doc.page_count())?;
let warn_pages = opts.pages.as_deref().map(|pages| if opts.spreads { with_their_spreads(s, pages) } else { pages.to_vec() });
r.warnings.extend(crate::cmd::preflight::overset_warnings(s, warn_pages.as_deref()));
match str_param(p, "path") {
Some(path) => {
write_file(path, &r.bytes)?;
@@ -461,3 +480,105 @@ mod page_list_tests {
assert!(s.execute("file.exportPdf", &json!({"pages": []})).is_err());
}
}
#[cfg(test)]
mod overset_tests {
use serde_json::json;
use crate::Session;
/// A page-based export leaves out the text its frames have no room for, and said nothing
/// about it: an imported 58-page form lost whole sentences — a data-consent clause, a
/// statutory rule, the last row of two tables — with `warnings` empty while preflight
/// reported 19 overset stories (#1465).
#[test]
fn an_export_warns_about_the_text_the_pages_had_no_room_for() {
let mut s = Session::new();
s.execute("file.new", &json!({"width": 300, "height": 200, "pages": 2, "facingPages": false, "margins": 0})).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!({"rect": [20, 20, 280, 60], "content": "text", "text": text})).unwrap();
let (id, sid) = (frame["id"].as_u64().unwrap(), frame["story"].clone());
// What preflight says about the same frame, for the warning to agree with.
let issues = s.execute("preflight.run", &json!({})).unwrap();
let overset = issues["issues"].as_array().unwrap().iter().find(|i| i["kind"] == "overset").expect("preflight reports it").clone();
assert_eq!(overset["message"], "Overset text: 76 characters");
for command in ["file.exportPdf", "file.exportPptx"] {
let r = s.execute(command, &json!({})).unwrap();
let warnings = r["warnings"].as_array().expect("warnings").clone();
let about: Vec<&str> = warnings.iter().filter_map(|w| w.as_str()).filter(|w| w.contains("not in the export")).collect();
assert_eq!(about.len(), 1, "{command}: {warnings:?}");
let w = about[0];
assert!(w.contains("page 1"), "{command}: the page it is on: {w}");
assert!(w.contains("76 character"), "{command}: the count preflight gives: {w}");
assert!(w.contains(&format!("frame {id}")), "{command}: the frame: {w}");
}
// The pages being exported are the ones reported on: page 2 has no overset frame.
let r = s.execute("file.exportPdf", &json!({"pages": [2]})).unwrap();
assert_eq!(r["warnings"], json!([]), "{r}");
// A reflowable export carries the whole story, so it has nothing to report.
let r = s.execute("file.exportHtml", &json!({})).unwrap();
assert_eq!(r["warnings"], json!([]), "{r}");
assert!(r["text"].as_str().unwrap().contains("must not vanish"), "the HTML has the text the PDF left out");
// `overset` is the index where the text stops fitting; `oversetChars` is how much is left,
// which is the number preflight reports. The two were read as the same thing.
let story = s.execute("story.get", &json!({"story": sid})).unwrap();
assert_eq!(story["overset"], json!(68), "{story}");
assert_eq!(story["oversetChars"], json!(76), "{story}");
// A frame the text fits: nothing is reported anywhere.
let mut s = Session::new();
s.execute("file.new", &json!({"width": 300, "height": 200, "pages": 1, "facingPages": false, "margins": 0})).unwrap();
let frame = s.execute("frame.create", &json!({"rect": [20, 20, 280, 180], "content": "text", "text": "Short."})).unwrap();
for command in ["file.exportPdf", "file.exportPptx"] {
assert_eq!(s.execute(command, &json!({})).unwrap()["warnings"], json!([]), "{command}");
}
let story = s.execute("story.get", &json!({"story": frame["story"].clone()})).unwrap();
assert!(story["overset"].is_null() && story["oversetChars"].is_null(), "{story}");
}
/// 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).
#[test]
fn a_spread_export_warns_about_the_pages_it_takes_with_it() {
let mut s = Session::new();
// Facing pages: 2 and 3 share a spread, 1 is the cover on its own.
s.execute("file.new", &json!({"width": 200, "height": 200, "pages": 4, "facingPages": true, "margins": 0})).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.";
// A frame on page 3, whose text does not fit it. `frame.create` takes a spread and
// spread coordinates, so page 3's own bounds say where to put it.
let (si, b) = {
let d = &s.active().expect("a document").doc;
let (si, pi) = d.page_loc(2).expect("page 3 is there");
(si, d.spreads[si].pages[pi].bounds())
};
let rect = json!([b.x0 + 20.0, b.y0 + 20.0, b.x0 + 180.0, b.y0 + 60.0]);
let frame = s.execute("frame.create", &json!({"spread": si, "rect": rect, "content": "text", "text": text})).unwrap();
let id = frame["id"].as_u64().unwrap();
let about = |r: &serde_json::Value| -> Vec<String> {
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()
};
// Page by page, choosing page 2 reports nothing: page 2 holds no overset frame.
let r = s.execute("file.exportPdf", &json!({"pages": [2]})).unwrap();
assert!(about(&r).is_empty(), "page 2 alone has nothing to report: {r}");
// As spreads, choosing page 2 exports page 3 with it, so page 3's overset is reported.
let r = s.execute("file.exportPdf", &json!({"pages": [2], "spreads": true})).unwrap();
let w = about(&r);
assert_eq!(w.len(), 1, "the page that came along is reported too: {r}");
assert!(w[0].contains("page 3") && w[0].contains(&format!("frame {id}")), "{}", w[0]);
// And a page on another spread entirely is still not reported.
let r = s.execute("file.exportPdf", &json!({"pages": [1], "spreads": true})).unwrap();
assert!(about(&r).is_empty(), "the cover's spread holds no overset frame: {r}");
}
}
+45
View File
@@ -113,6 +113,51 @@ fn overset_location(s: &Session, target: designcraft_doc::ItemId) -> Option<(des
None
}
/// Text that is in the document but not in a page-based export (PDF, PowerPoint): the overset
/// part of a story, and a story with no frame at all. The export used to write the file and
/// report nothing, so whole sentences left an imported document without a hint anywhere (#1465).
///
/// One line per story, naming the page and the frame as the preflight issue does. `pages` are the
/// 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.
pub fn overset_warnings(s: &Session, pages: Option<&[usize]>) -> Vec<String> {
let Some(st) = s.active() else { return vec![] };
let d = &st.doc;
let mut out = Vec::new();
for story in d.stories.values() {
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();
if n == 0 {
continue;
}
if story.frames.is_empty() {
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)
&& !pages.contains(&page)
{
continue;
}
let item = location.map(|(item, _)| item).or(last);
out.push(match (page, item) {
(Some(page), Some(item)) => {
format!("page {}: {n} character(s) of the story in frame {} do not fit it and are not in the export", page + 1, item.0)
}
(None, Some(item)) => format!("the story in frame {}: {n} character(s) do not fit and are not in the export", item.0),
_ => format!("{n} character(s) of a story do not fit their frame and are not in the export"),
});
}
out
}
pub fn check(s: &Session, min_ppi: f64) -> Vec<Issue> {
let Some(st) = s.active() else { return vec![] };
let d = &st.doc;
+6 -1
View File
@@ -191,12 +191,17 @@ pub fn specs() -> Vec<CommandSpec> {
})
}
),
cmd!(query "story.get", "Get Story", [], None, "{story? | frame?} → text, frames, paragraphs, vertical, overset", has_doc, |s, p| {
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| {
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))?;
let cs = s.cache.get(&st.doc, sid, None);
// `overset` is where the text stops fitting, not how much is left over: the two were
// read as the same number and disagreed with preflight's count (#1465).
v["overset"] = json!(cs.overset_at);
v["oversetChars"] = json!(cs
.overset_at
.map(|at| st.doc.stories.get(&sid).and_then(|story| story.text.get(at..)).unwrap_or("").chars().count()));
v["lines"] = json!(cs.line_count());
Ok(v)
}),