diff --git a/ROADMAP.md b/ROADMAP.md index a1fbd1da05..f8dab21269 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -140,7 +140,7 @@ Ranked, to beta (detail and remaining milestones in [docs/roadmap.md](docs/roadm ## Progress log - 2026-11-04: OpenDocument files are shown in one of two styles (gap 19): Microsoft Word style, the default, or LibreOffice style (File › Options › Advanced › OpenDocument files; `file.options` `{"odf": {"preset": "word"}}` or `"libreoffice"`; `WORDCRAFT_ODF=word` or `WORDCRAFT_ODF=libreoffice`). Custom stays for one's own value per case; `wordcraft`, the earlier id of Word's style, is still taken. -- 2026-11-04: ODT element audit (gap 19, [docs/odt-element-audit.md](docs/odt-element-audit.md)): every element of the ODF 1.3 schema checked against 365 files and a document of our own for each one no file held; all open and save, and a save without edits keeps them (password, number, time and grid form controls included). Over the 220 files from other applications, 177 round-trip pixel-identical, 201 with the same text and 218 with the same page count (the latest measurement, after the groups and form-control work). +- 2026-11-04: ODT element audit (gap 19, [docs/odt-element-audit.md](docs/odt-element-audit.md)): every element of the ODF 1.3 schema checked against 365 files and a document of our own for each one no file held that a text document can hold (the `presentation:`, `anim:` and `db:` elements only presentations and databases hold stay unseen); all open and save, and a save without edits keeps them (password, number, time and grid form controls included). Over the 220 files from other applications, 177 round-trip pixel-identical, 201 with the same text and 218 with the same page count (the latest measurement, after the groups and form-control work). - 2026-11-04: ODT checked against 220 files from other applications (Microsoft Office, OpenOffice.org, StarOffice, Calligra, Lotus Symphony; local only): every file LibreOffice opens opens again after a WordCraft save (a manifest entry was missing), 166 round-trip pixel-identical and 194 with the same text at the time (an earlier measurement; the element-audit entry above has the current figures); hidden sections stay hidden and unedited lists are written as read (gap 19). - 2026-11-03: ODT paragraphs holding bookmarks, notes, formulas and objects, and the page-anchored frames before a paragraph, are written as read while unchanged (gap 19); styles of different families sharing a name are kept apart. Over the corpus 123 of 133 files round-trip pixel-identical (118), 132 with the same text, and the attributes and values a save loses fall from 539 to under 315. - 2026-11-03: ODT paragraphs showing text boxes and shapes holding text are written as read while that text is unchanged (gap 19). Over the corpus 111 of 133 files now round-trip pixel-identical (103), the round-trip difference falls from 44k to 39k pixels, and the element kinds a save loses fall from 16 to 8. diff --git a/crates/doc/src/section.rs b/crates/doc/src/section.rs index 0e6660dfb9..7541c35d02 100644 --- a/crates/doc/src/section.rs +++ b/crates/doc/src/section.rs @@ -733,8 +733,9 @@ pub struct SectionProps { /// Distance from the page edge to the header / footer. pub header: f32, pub footer: f32, - /// The space kept between a header too tall for the top margin and the body, and between - /// the body and such a footer, points (ODF's header and footer spacing; Word keeps none). + /// The space kept between a header and the body, and between the body and a footer, points + /// (ODF's header and footer spacing; Word keeps none). The margin already holds the band and + /// its spacing, so it only moves the body once content and spacing reach past the margin. #[serde(skip_serializing_if = "is_zero")] pub header_gap: f32, #[serde(skip_serializing_if = "is_zero")] diff --git a/crates/engine/src/cmd/citations/generated.rs b/crates/engine/src/cmd/citations/generated.rs index 0a95bea7db..44a3dd8e08 100644 --- a/crates/engine/src/cmd/citations/generated.rs +++ b/crates/engine/src/cmd/citations/generated.rs @@ -71,7 +71,7 @@ const MAX_WRAPPED: usize = 10_000; /// A generated list (index, bibliography, table of figures or authorities) held as a complex /// field that starts at body block `i` and runs over its entries: its field code and the block /// it ends in. `None` for anything else (a TOC has its own update). -fn wrapped_list(s: &Session, i: usize) -> Option<(String, usize)> { +fn wrapped_list(s: &Session, i: usize) -> Option<(String, usize, usize)> { let p = s.doc.body.get(i)?.as_para()?; let (k, off, instr) = p.objects_with_offsets().enumerate().find_map(|(k, (off, o))| match o { InlineObject::FieldStart { instr, .. } => { @@ -90,13 +90,13 @@ fn wrapped_list(s: &Session, i: usize) -> Option<(String, usize)> { let mut depth = 0usize; for (j, b) in s.doc.body.iter().enumerate().skip(i).take(MAX_WRAPPED) { let Some(q) = b.as_para() else { continue }; - for o in q.objects.iter().skip(if j == i { k } else { 0 }) { + for (at, o) in q.objects_with_offsets().skip(if j == i { k } else { 0 }) { match o { InlineObject::FieldStart { .. } => depth += 1, InlineObject::FieldEnd => { depth = depth.saturating_sub(1); if depth == 0 { - return Some((instr, j)); + return Some((instr, j, at + wordcraft_doc::para::OBJ.len_utf8())); } } _ => {} @@ -195,18 +195,39 @@ pub fn update_generated(s: &mut Session) -> Result<(), CmdError> { while i < s.doc.body.len() { // A list held as a field around its entries (as Word and ODT files hold them): made the // field's own paragraph, as WordCraft inserts one, its old entries rebuilt below. - if let Some((instr, last)) = wrapped_list(s, i) { - let mut p = Paragraph::default(); + // What follows its closing field end, and its first paragraph's properties and section + // break, are the document's and stay. + if let Some((instr, last, end)) = wrapped_list(s, i) { + // The entries between its first and last paragraphs go. + for _ in i.saturating_add(1)..last { + s.doc.remove_block(StoryRef::Body, &Path::top(i + 1))?; + } + // Its first paragraph keeps only the field; its last (or, ending in its first, the + // same) loses what is up to and through the field end, and goes when nothing is left + // of it and it ends no section. + let first_end = if last > i { + let q = s.doc.para_mut(StoryRef::Body, &Path::top(i + 1))?; + q.delete(0, end)?; + if q.is_empty() && q.section.is_none() { + s.doc.remove_block(StoryRef::Body, &Path::top(i + 1))?; + } + s.doc.para_mut(StoryRef::Body, &Path::top(i))?.len() + } else { + end + }; + let p = s.doc.para_mut(StoryRef::Body, &Path::top(i))?; + p.delete(0, first_end)?; p.insert_object( 0, InlineObject::Field { instr, result: String::new(), locked: false, dirty: false, code: false, form: String::new() }, &Default::default(), )?; - for _ in i..last { - s.doc.remove_block(StoryRef::Body, &Path::top(i + 1))?; - } - if let Some(b) = s.doc.body.get_mut(i) { - *b = wordcraft_doc::para_block(p); + // What followed the field end in its own paragraph comes after the list, in a + // paragraph of its own that takes the section break. + let field = wordcraft_doc::para::OBJ.len_utf8(); + if p.len() > field { + let rest = p.split_off(field)?; + s.doc.insert_block(StoryRef::Body, &Path::top(i + 1), Block::Para(rest))?; } } let kind = s.doc.body.get(i).and_then(|b| b.as_para()).and_then(list_kind); @@ -410,6 +431,59 @@ mod tests { p.insert_object(n, InlineObject::FieldEnd, &props).unwrap(); } let s = Session::new(d); - assert_eq!(wrapped_list(&s, 0), Some(("BIBLIOGRAPHY".to_string(), 1))); + assert_eq!(wrapped_list(&s, 0).map(|(instr, last, _)| (instr, last)), Some(("BIBLIOGRAPHY".to_string(), 1))); + } + + /// A wrapped list as a session holds it: the field starting paragraph `first`'s text, ending + /// in paragraph `last` at byte `at`. + fn wrapped(text: &str, first: usize, last: usize, at: usize) -> Session { + let mut d = Document::from_text(text); + let props = CharProps::default(); + if let Ok(p) = d.para_mut(StoryRef::Body, &Path::top(last)) { + p.insert_object(at, InlineObject::FieldEnd, &props).unwrap(); + } + if let Ok(p) = d.para_mut(StoryRef::Body, &Path::top(first)) { + p.insert_object(0, InlineObject::FieldStart { instr: "BIBLIOGRAPHY".into(), locked: false, dirty: false }, &props).unwrap(); + } + Session::new(d) + } + + fn texts(s: &Session) -> Vec { + s.doc.body.iter().filter_map(|b| b.as_para()).map(|p| p.plain_text()).collect() + } + + /// What follows a wrapped list's closing field end, in its last paragraph or in its only one, + /// stays, after the list; the last paragraph's section break stays too. + #[test] + fn what_follows_a_wrapped_lists_end_stays() { + // Ending in a later paragraph, with text after the end and a section break. + let mut s = wrapped("Old one\nOld two\nOld three. After\nBelow", 0, 2, "Old three.".len()); + if let Ok(p) = s.doc.para_mut(StoryRef::Body, &Path::top(2)) { + p.section = Some(Box::default()); + } + if let Ok(p) = s.doc.para_mut(StoryRef::Body, &Path::top(0)) { + p.props.indent_left = Some(9.0); + } + update_generated(&mut s).unwrap(); + let t = texts(&s); + assert!(t.iter().all(|t| !t.contains("Old")), "{t:?}"); + assert!(t.iter().any(|t| t == " After"), "{t:?}"); + assert_eq!(t.last().map(String::as_str), Some("Below")); + let after = s.doc.body.iter().filter_map(|b| b.as_para()).find(|p| p.plain_text() == " After"); + assert!(after.is_some_and(|p| p.section.is_some()), "the section break went"); + let first = s.doc.body.first().and_then(|b| b.as_para()); + assert!(first.is_some_and(|p| p.props.indent_left == Some(9.0) && list_kind(p).is_some()), "{first:?}"); + // An empty last paragraph without a section break goes. + let mut s = wrapped("Old one\nOld two\nBelow", 0, 1, "Old two".len()); + update_generated(&mut s).unwrap(); + // (The field's paragraph shows nothing: there are no sources to list.) + assert_eq!(texts(&s), ["", "Below"]); + // Ending in its own paragraph: what follows comes after the list. + let mut s = wrapped("Old entry. Then this\nBelow", 0, 0, "Old entry.".len()); + update_generated(&mut s).unwrap(); + let t = texts(&s); + assert!(t.first().is_some_and(|t| !t.contains("Old")), "{t:?}"); + assert_eq!(t.get(1).map(String::as_str), Some(" Then this"), "{t:?}"); + assert_eq!(t.last().map(String::as_str), Some("Below")); } } diff --git a/crates/formats/src/odt/body/notes.rs b/crates/formats/src/odt/body/notes.rs index 6fc80f564a..591a1c75d5 100644 --- a/crates/formats/src/odt/body/notes.rs +++ b/crates/formats/src/odt/body/notes.rs @@ -236,6 +236,8 @@ impl Body<'_> { let Some(n) = self.note.take() else { return }; self.tables.truncate(n.tables); self.cells.truncate(n.cells); + self.formulas.truncate(n.cells); + self.cell_tops.truncate(n.cells); self.lists_saved.truncate(n.lists_saved); self.containers.truncate(n.containers + 1); let blocks = if self.containers.len() > n.containers { self.containers.pop().unwrap_or_default() } else { Vec::new() }; @@ -346,3 +348,34 @@ impl Body<'_> { self.xml_space_end = false; } } + +#[cfg(test)] +mod tests { + use crate::odt::{Body, Pages, Styles, events}; + + /// The cell stacks after reading `xml`: (cells, formulas, cell tops). + fn stacks_after(xml: &str) -> (usize, usize, usize) { + let (styles, pages) = (Styles::default(), Pages::default()); + let mut none = |_: &str| None; + let mut b = Body::new(&styles, &pages, &mut none); + b.in_text = true; + events(xml.as_bytes(), |ev| b.feed(ev)); + (b.cells.len(), b.formulas.len(), b.cell_tops.len()) + } + + /// A formula cell a note or a deletion leaves open (malformed) goes with it: nothing of it is + /// left for the cell round the note, or for the next cell read. + #[test] + fn a_cell_a_note_or_deletion_leaves_open_takes_its_formula_with_it() { + // The cell, its row and its table end with tags that are none of theirs. + let open_cell = r#"2"#; + let in_note = format!( + "a1{open_cell}" + ); + assert_eq!(stacks_after(&in_note), (0, 0, 0)); + let in_deletion = format!( + "A{open_cell}b" + ); + assert_eq!(stacks_after(&in_deletion), (0, 0, 0)); + } +} diff --git a/crates/formats/src/odt/body/tracked.rs b/crates/formats/src/odt/body/tracked.rs index 93cec7b7ed..9a448d88ef 100644 --- a/crates/formats/src/odt/body/tracked.rs +++ b/crates/formats/src/odt/body/tracked.rs @@ -106,6 +106,8 @@ impl Body<'_> { } self.tables.truncate(d.tables); self.cells.truncate(d.cells); + self.formulas.truncate(d.cells); + self.cell_tops.truncate(d.cells); self.lists_saved.truncate(d.lists_saved); self.fmt.truncate(d.fmt); self.lists = d.lists; diff --git a/crates/formats/src/odt/import.rs b/crates/formats/src/odt/import.rs index f4db12a34b..b67751b27e 100644 --- a/crates/formats/src/odt/import.rs +++ b/crates/formats/src/odt/import.rs @@ -583,6 +583,33 @@ fn kept_markup(blocks: &[FBlock], depth: usize, out: &mut Vec<(String, std::sync } } +/// Drop the markup kept as read (of paragraphs and tables, as [`kept_markup`] finds it) that +/// links to one of the package files in `missing`. +fn unkeep_linking(blocks: &mut [FBlock], depth: usize, missing: &std::collections::BTreeSet) { + if missing.is_empty() { + return; + } + for b in blocks { + match b { + FBlock::Para(p) => { + if p.kept.as_deref().is_some_and(|x| super::kept::links_to(x, missing)) { + p.kept = None; + } + } + FBlock::Table(t) => { + if t.kept.as_deref().is_some_and(|x| super::kept::links_to(x, missing)) { + t.kept = None; + } + if t.kept.is_none() && depth < crate::model::MAX_DEPTH { + for c in t.rows.iter_mut().flatten() { + unkeep_linking(&mut c.blocks, depth + 1, missing); + } + } + } + } + } +} + /// What [`parse_tracked`] reads besides the flow. struct Parsed { flow: Flow, @@ -795,7 +822,13 @@ fn parse_tracked(bytes: &[u8], paper: (f32, f32), profile: &OdfProfile) -> Resul for b in parts.blocks_mut() { kept_markup(b, 0, &mut linking); } - let kept_files = super::kept::files(&linking, &entries, &mut fetch); + let (kept_files, missing) = super::kept::files(&linking, &entries, &mut fetch); + // Markup linking to a file that couldn't be read is written from the model: as read, it + // would point at a file the saved package hasn't got. + unkeep_linking(&mut blocks, 0, &missing); + for b in parts.blocks_mut() { + unkeep_linking(b, 0, &missing); + } // Their manifest entries, and their folders' (an object's says what it is). for (path, _) in &kept_files { let dir = path.rsplit_once('/').map(|(d, _)| format!("{d}/")); diff --git a/crates/formats/src/odt/kept.rs b/crates/formats/src/odt/kept.rs index eaeda9a3b8..d5d0f57841 100644 --- a/crates/formats/src/odt/kept.rs +++ b/crates/formats/src/odt/kept.rs @@ -324,24 +324,51 @@ const MAX_FILES: usize = 1000; /// The package files `defs` refer to (`xlink:href` to a path inside the package, as a style's /// fill picture; to a folder, as an embedded object in a header: the files in it), read with /// `fetch` (`entries`: the package's paths): a definition written as read needs them. -pub(super) fn files(defs: &[(String, Arc)], entries: &[String], fetch: &mut dyn FnMut(&str) -> Option>) -> Vec<(String, Vec)> { +/// Also the paths linked to that are in the package but couldn't be read (over the picture +/// budget, unreadable): markup linking to one can't be written as read. +pub(super) fn files( + defs: &[(String, Arc)], + entries: &[String], + fetch: &mut dyn FnMut(&str) -> Option>, +) -> (Vec<(String, Vec)>, BTreeSet) { let mut paths = BTreeSet::new(); for (_, xml) in defs { hrefs(xml, &mut paths); } let mut out = Vec::new(); + let mut missing = BTreeSet::new(); for p in paths { if entries.contains(&p) { - out.extend(fetch(&p).map(|b| (p, b))); + match fetch(&p) { + Some(b) => out.push((p, b)), + None => { + missing.insert(p); + } + } } else { // A folder (an object): the files in it. let dir = format!("{}/", p.trim_end_matches('/')); for e in entries.iter().filter(|e| e.starts_with(&dir) && !e.ends_with('/')).take(MAX_FILES) { - out.extend(fetch(e).map(|b| (e.clone(), b))); + match fetch(e) { + Some(b) => out.push((e.clone(), b)), + None => { + missing.insert(p.clone()); + } + } } } } - out + (out, missing) +} + +/// Whether markup `xml` links to one of the package files in `missing` (see [`files`]). +pub(super) fn links_to(xml: &str, missing: &BTreeSet) -> bool { + if missing.is_empty() { + return false; + } + let mut paths = BTreeSet::new(); + hrefs(xml, &mut paths); + paths.iter().any(|p| missing.contains(p)) } /// The package files `xml` links to (`xlink:href` to a path inside the package), added to diff --git a/crates/formats/src/odt/package.rs b/crates/formats/src/odt/package.rs index 0d0ea97359..e4fa5b8545 100644 --- a/crates/formats/src/odt/package.rs +++ b/crates/formats/src/odt/package.rs @@ -1057,7 +1057,7 @@ fn export_as(doc: &Document, mime: &str) -> Result, String> { let scripts = kept.and_then(|k| k.defs.get("top:office:scripts")).map(|d| d.to_string()).unwrap_or_default(); let soft = soft_page_breaks(kept, &body); let content = format!( - "\n{scripts}{}\n{}{tracked}{var_decls}{user_decls}{decls}{other_decls}{body}\n", + "\n{scripts}{}\n{}{tracked}{var_decls}{decls}{user_decls}{other_decls}{body}\n", w.automatic_styles(), // The file's forms as read, with the document's controls. super::kept::merge_forms(kept.and_then(|k| k.defs.get("forms")).map(|d| &**d), super::forms::forms_xml(&w.forms, &hidden_controls(doc))) @@ -1208,7 +1208,9 @@ fn page_borders(b: Option, margins: [f32; 4]) -> let mut attrs = String::new(); for (i, (side, edge)) in [("top", b.top), ("bottom", b.bottom), ("left", b.left), ("right", b.right)].into_iter().enumerate() { let (Some(e), Some(m)) = (edge.filter(|e| e.is_visible()), out.get_mut(i)) else { continue }; - let at = if e.space.is_finite() { e.space.clamp(0.0, *m) } else { 0.0 }; + // A hostile margin (negative or not a number) leaves no room, so clamp cannot panic. + let room = if m.is_finite() { m.max(0.0) } else { 0.0 }; + let at = if e.space.is_finite() { e.space.clamp(0.0, room) } else { 0.0 }; let pad = (*m - at - e.width).max(0.0); *m = at; attrs.push_str(&format!(" fo:border-{side}=\"{}\" fo:padding-{side}=\"{}\"", super::writer::odf_border(Some(e)), pt(pad))); @@ -1422,6 +1424,21 @@ fn font_kind(family: &str) -> (&'static str, &'static str) { #[cfg(test)] mod setting_tests { + /// A page border round a page whose margin is negative or not a number is written at no + /// distance, instead of panicking. + #[test] + fn a_page_border_round_a_hostile_margin_is_written_at_no_distance() { + let mut edge = wordcraft_doc::props::Border::single(1.0); + edge.space = 12.0; + let b = wordcraft_doc::props::Borders::all(edge); + let (attrs, out) = super::page_borders(Some(b), [-10.0, f32::NAN, 36.0, 5.0]); + assert!(attrs.contains("fo:border-top="), "{attrs}"); + assert_eq!(out[0], 0.0); + assert_eq!(out[1], 0.0); + assert_eq!(out[2], 12.0); + assert_eq!(out[3], 5.0); + } + /// A self-closed setting is replaced whole, leaving the item after it as it was. #[test] fn a_self_closed_setting_is_replaced_whole() { diff --git a/crates/formats/src/odt/tests_more/lossless.rs b/crates/formats/src/odt/tests_more/lossless.rs index a61bf4808a..fa31bba839 100644 --- a/crates/formats/src/odt/tests_more/lossless.rs +++ b/crates/formats/src/odt/tests_more/lossless.rs @@ -438,6 +438,16 @@ fn content_of(bytes: &[u8]) -> String { s } +/// The declarations at the start of the text are written in the schema's order: variables, +/// sequences, then user fields. +#[test] +fn the_text_declarations_are_written_in_the_schemas_order() { + let c = content_of(&export(&import(&package(&DECLS)).unwrap()).unwrap()); + let at = |el: &str| c.find(el).unwrap_or_else(|| panic!("no {el} in {c}")); + assert!(at("