Review fixes: kept object names skipped (headers too), self-closed settings, STYLEREF/XE after a leading space, RTL padding, split cell backgrounds, PAGE in a table moved by a page break, written-as-read doc

This commit is contained in:
FredApps
2026-11-04 13:38:55 +01:00
parent d2f504e29a
commit b6afa0ba3c
8 changed files with 181 additions and 15 deletions
+16 -2
View File
@@ -212,7 +212,7 @@ pub(super) fn element(instr: &str, result: &str, data_style: Option<&str>) -> Op
}
// `STYLEREF "Heading 2"` or `STYLEREF 2`: the chapter of that level.
"STYLEREF" => {
let target = instr.get(code.len()..)?.trim_start();
let target = instr.trim_start().get(code.len()..)?.trim_start();
let target = target.strip_prefix('"').and_then(|t| t.split('"').next()).unwrap_or_else(|| target.split_whitespace().next().unwrap_or(""));
let level = target.trim().strip_prefix("Heading").unwrap_or(target).trim().parse::<u32>().ok().filter(|l| (1..=10).contains(l))?;
let number = [" \\n", " \\r", " \\w"].iter().any(|s| instr.contains(s));
@@ -232,7 +232,7 @@ pub(super) fn element(instr: &str, result: &str, data_style: Option<&str>) -> Op
}
// An index entry: a mark at its place, its first levels the keys (§8.1.10).
"XE" => {
let entry = instr.get(code.len()..)?.trim_start();
let entry = instr.trim_start().get(code.len()..)?.trim_start();
let entry = entry.strip_prefix('"').and_then(|t| t.split('"').next())?;
let mut levels: Vec<&str> = entry.split(':').map(str::trim).filter(|l| !l.is_empty()).collect();
let value = levels.pop()?;
@@ -527,3 +527,17 @@ pub(super) fn set_of(instr: &str) -> Option<(&str, String)> {
};
Some((name, value))
}
#[cfg(test)]
mod tests {
/// Word writes field codes with a leading space (` STYLEREF 2 `); the argument is read after
/// the code all the same.
#[test]
fn a_field_code_with_leading_space_keeps_its_argument() {
let chapter = super::element(" STYLEREF \"Heading 2\" ", "Intro", None).unwrap_or_default();
assert!(chapter.contains("text:outline-level=\"2\""), "{chapter}");
assert_eq!(super::element("STYLEREF 2", "Intro", None), super::element(" STYLEREF 2", "Intro", None));
assert_eq!(super::element(" XE \"Apple\" ", "", None), super::element("XE \"Apple\"", "", None));
assert!(super::element(" XE \"Apple\" ", "", None).is_some_and(|e| e.contains("Apple")));
}
}
+25 -1
View File
@@ -625,7 +625,14 @@ fn with_setting(own: &str, name: &str, on: bool, item: &str) -> Option<String> {
let key = format!("config:name=\"{name}\"");
if let Some(at) = own.find(&key) {
let rest = own.get(at..)?;
let (gt, lt) = (rest.find('>')?, rest.find("</config:config-item>")?);
let gt = rest.find('>')?;
// A self-closed item (another application's, or a hand-edited file's) is replaced whole:
// its close tag would be the next item's.
if rest.get(..gt).is_some_and(|t| t.ends_with('/')) {
let start = own.get(..at)?.rfind('<')?;
return Some(format!("{}{item}{}", own.get(..start).unwrap_or(""), own.get(at + gt + 1..).unwrap_or("")));
}
let lt = rest.find("</config:config-item>")?;
return (gt < lt).then(|| format!("{}{on}{}", own.get(..at + gt + 1).unwrap_or(""), own.get(at + lt..).unwrap_or("")));
}
let set = "config:name=\"ooo:configuration-settings\"";
@@ -1056,6 +1063,8 @@ fn export_as(doc: &Document, mime: &str) -> Result<Vec<u8>, String> {
super::kept::merge_forms(kept.and_then(|k| k.defs.get("forms")).map(|d| &**d), super::forms::forms_xml(&w.forms, &hidden_controls(doc)))
);
let mut hw = Writer::with_prefix(if kept.is_some() { "wcM" } else { "M" });
// Headers' and footers' new pictures and objects skip the kept files' names too.
hw.reserved = std::sync::Arc::clone(&w.reserved);
hw.accents = accents;
hw.variables.clone_from(&w.variables);
let parts: Vec<PageParts> = pages.iter().map(|(_, sec)| page_parts(doc, sec, &mut hw)).collect();
@@ -1410,3 +1419,18 @@ fn font_kind(family: &str) -> (&'static str, &'static str) {
}
("swiss", "variable")
}
#[cfg(test)]
mod setting_tests {
/// A self-closed setting is replaced whole, leaving the item after it as it was.
#[test]
fn a_self_closed_setting_is_replaced_whole() {
let item = "<config:config-item config:name=\"A\" config:type=\"boolean\">true</config:config-item>";
let own = "<s><config:config-item config:name=\"A\" config:type=\"boolean\"/><config:config-item config:name=\"B\" config:type=\"boolean\">false</config:config-item></s>";
let out = super::with_setting(own, "A", true, item).unwrap_or_default();
assert_eq!(out, format!("<s>{item}<config:config-item config:name=\"B\" config:type=\"boolean\">false</config:config-item></s>"));
// An item with a value has just its value changed.
let own = "<s><config:config-item config:name=\"A\" config:type=\"boolean\">false</config:config-item></s>";
assert_eq!(super::with_setting(own, "A", true, item).as_deref(), Some(format!("<s>{item}</s>").as_str()));
}
}
+23 -2
View File
@@ -221,9 +221,16 @@ impl Writer {
/// A picture (ODF 1.3 §10.4.4 `draw:image` in a `draw:frame`) at `place`: a floating one or a
/// group's in a graphic style that places and wraps it.
/// The next object's number and name (`Object 3`, `Object M3` in headers and footers).
/// Not a name the file's own objects, kept as read, have (`Object wc1/content.xml`).
pub(in crate::odt) fn object_name(&mut self) -> (usize, String) {
self.object_names += 1;
(self.object_names, format!("Object {}{}", self.prefix, self.object_names))
loop {
self.object_names = self.object_names.saturating_add(1);
let name = format!("Object {}{}", self.prefix, self.object_names);
let dir = format!("{name}/");
if self.object_names == usize::MAX || !self.reserved.iter().any(|r| *r == name || r.starts_with(&dir)) {
return (self.object_names, name);
}
}
}
/// Object `o`'s files in the package under `name`, and the link to it with its replacement
@@ -624,6 +631,20 @@ impl Writer {
mod tests {
use super::*;
#[test]
fn an_object_never_takes_a_kept_objects_name() {
// Re-saving a file WordCraft saved: its `Object wc1` is kept as read, so a new object
// (an edited paragraph's chart or formula) takes `Object wc2`, not the kept one's files.
let mut w = Writer::with_prefix("wc");
w.reserved = std::sync::Arc::new(["Object wc1/content.xml".to_string(), "Object wc3".to_string()].into_iter().collect());
assert_eq!(w.object_name(), (2, "Object wc2".to_string()));
assert_eq!(w.object_name(), (4, "Object wc4".to_string()));
// Not `Object wc10` for `Object wc1`'s files: only the name itself or its folder.
let mut w = Writer::with_prefix("wc");
w.reserved = std::sync::Arc::new(std::iter::once("Object wc10/content.xml".to_string()).collect());
assert_eq!(w.object_name().1, "Object wc1");
}
#[test]
fn a_picture_never_takes_a_name_an_earlier_one_took() {
// Re-saving a file WordCraft saved: its `wcimage1.png` is kept as read, so the first new
+27 -2
View File
@@ -113,8 +113,11 @@ fn odf_padding(rp: &mut ResolvedPara) {
let [top, left, bottom, right] = rp.padding;
rp.space_before += top;
rp.space_after += bottom;
rp.indent_left += left;
rp.indent_right += right;
// Padding is on a physical side, the indents on a logical one: in a right-to-left paragraph
// the left padding is at its end.
let (start, end) = if rp.bidi { (right, left) } else { (left, right) };
rp.indent_left += start;
rp.indent_right += end;
}
/// List level indents apply unless the paragraph sets its own, or, for a list from the style,
@@ -800,3 +803,25 @@ fn break_positions(text: &str, rp: &wordcraft_doc::resolve::ResolvedPara) -> Vec
crate::kinsoku::apply(text, &mut found, rp.kinsoku, rp.word_wrap);
found
}
#[cfg(test)]
mod padding_tests {
/// ODF padding is physical: a right-to-left paragraph's left padding widens its end indent
/// (the indents being logical), a left-to-right one's its start indent.
#[test]
fn a_right_to_left_paragraphs_left_padding_is_at_its_end() {
let doc = wordcraft_doc::Document::new();
let mut rp = doc.styles.resolve_para(&wordcraft_doc::props::ParaProps::default());
rp.padding = [1.0, 10.0, 2.0, 3.0];
let (left, right) = (rp.indent_left, rp.indent_right);
let mut ltr = rp.clone();
super::odf_padding(&mut ltr);
assert_eq!((ltr.indent_left - left, ltr.indent_right - right), (10.0, 3.0));
rp.bidi = true;
super::odf_padding(&mut rp);
assert_eq!((rp.indent_left - left, rp.indent_right - right), (3.0, 10.0));
// Shown on the page: the left padding still on the left.
assert_eq!(rp.visual_indents(), (right + 10.0, left + 3.0));
assert_eq!((rp.space_before, rp.space_after), (ltr.space_before, ltr.space_after));
}
}
+25 -1
View File
@@ -901,6 +901,11 @@ fn split_row_with(row: &RowLayout, cut: f32, whole_cells: bool) -> Option<(RowLa
}
let limit = cut - row.bottom.clamp(0.0, cut);
let fits = |bottom: f32| bottom <= limit + 0.01;
// A cell's background picture (a clip as big as the cell's area): split with the area like
// shading, not moved on like a picture in the text.
let areas: Vec<Rect> =
row.items.iter().filter_map(|it| if let Placed::Cell { rect, covered: false, .. } = it { Some(*rect) } else { None }).collect();
let background = |it: &Placed| matches!(it, Placed::Clip { rect, .. } if areas.contains(rect));
// Each cell's items: every item of a row is in one of its cells.
let (mut cells, mut ends, realign) = cells_from_top(row, limit);
// A nested table's cell is deeper than the row's own.
@@ -950,6 +955,9 @@ fn split_row_with(row: &RowLayout, cut: f32, whole_cells: bool) -> Option<(RowLa
let mut cut_through = vec![false; cells.len()];
for ((((items, splits), m), kept), through) in cells.iter().zip(&splits).zip(&mut moved).zip(&mut kept_cells).zip(&mut cut_through) {
for (it, &split) in items.iter().zip(splits) {
if background(it) {
continue;
}
let mut runs_over = |top: f32, bottom: f32| *through |= top < limit - 0.01 && bottom > limit + 0.01;
let top = if let Some(r) = it.turned_bounds() {
// Turned text moves as a whole, like a picture.
@@ -1024,6 +1032,22 @@ fn split_row_with(row: &RowLayout, cut: f32, whole_cells: bool) -> Option<(RowLa
for ((((c, items), splits), &shift), end) in row.cells.iter().zip(&cells).zip(&splits).zip(&shifts).zip(&ends) {
let start = b.len();
for (it, &split) in items.iter().zip(splits) {
// A background picture: the part above the cut stays, and the continuation's area
// shows the rest of it, from the cut down.
if let Placed::Clip { rect, spin, items: pictures } = it
&& background(it)
{
let (above, below) = split_rect(*rect, cut, geo_shift(rect.bottom(), shift));
a.extend(above.map(|rect| Placed::Clip { rect, spin: *spin, items: pictures.clone() }));
b.extend(below.map(|r| {
let mut pictures = pictures.clone();
for p in &mut pictures {
p.translate(0.0, r.y - rect.y.max(cut));
}
Placed::Clip { rect: r, spin: *spin, items: pictures }
}));
continue;
}
if let Some(r) = it.turned_bounds() {
let mut it = it.clone();
if fits(r.bottom()) {
@@ -1292,7 +1316,7 @@ fn cell_picture(pic: &wordcraft_doc::props::CellPicture, rect: Rect, story: Stor
}
}
/// The parts of `r` above `cut` and below it/// The parts of `r` above `cut` and below it, the latter moved up by `shift` (onto the
/// The parts of `r` above `cut` and below it, the latter moved up by `shift` (onto the
/// continuation of a row split at `cut`).
fn split_rect(r: Rect, cut: f32, shift: f32) -> (Option<Rect>, Option<Rect>) {
let above = (r.y < cut).then(|| Rect::new(r.x, r.y, r.w, (r.bottom().min(cut) - r.y).max(0.0)));
+5 -3
View File
@@ -109,14 +109,16 @@ fn row_can_start(row: &wordcraft_doc::Row, laid: &table::RowLayout, room: f32, p
pub(crate) fn place_table(ctx: &mut Ctx, pb: &mut PageBuilder, t: &wordcraft_doc::Table, block: usize, body_top: f32) {
pb.prev = None;
let width = pb.col_w();
let mut tl = table::layout_table(ctx, StoryRef::Body, t, &[block as u32], width, 1);
// A page break before the first row's paragraph moves the whole table to a new page, as
// Word does (and as an ODT table starting a page opens).
// Word does (and as an ODT table starting a page opens). Before the table is laid out, so
// its cells' fields (PAGE) are those of the page it lands on.
let first = t.rows.first().and_then(|r| r.cells.first()).and_then(|c| c.blocks.first()).and_then(|b| b.as_para());
if t.props.float.is_none() && !pb.at_top() && !pb.web && first.is_some_and(|p| ctx.doc.styles.resolve_para(&p.props).page_break_before) {
pb.new_page(block, body_top);
ctx.fields.page = pb.number;
}
let width = pb.col_w();
let mut tl = table::layout_table(ctx, StoryRef::Body, t, &[block as u32], width, 1);
if let Some(f) = t.props.float.filter(|_| !pb.web) {
// Before Word 2013 layout (compatibility mode 15) an offset places the first cell's text,
// so the edge sits a cell margin further out.
+53
View File
@@ -13263,3 +13263,56 @@ fn header_margin_table_area() {
}
mod shape_alpha;
/// A row with a background picture split over two pages: each part draws its own piece of the
/// picture inside its own part of the cell, and the text moving on starts at the continuation's
/// top (the picture is the cell's background, not content moving on with the text).
#[test]
fn a_split_cells_background_picture_is_drawn_on_both_parts() {
use wordcraft_doc::props::{CellPicture, PictureRepeat};
let mut d = Document::new();
let mut t = wordcraft_doc::Table::new(1, 1, 200.0);
let media = d.add_media(vec![0u8; 8], "png");
t.rows[0].cells[0].props.picture = Some(Box::new(CellPicture { media, size: [30.0, 6.0], repeat: PictureRepeat::Tile, align: [0.0, 0.0] }));
t.rows[0].cells[0].blocks =
(0..90).map(|i| wordcraft_doc::para_block(wordcraft_doc::Paragraph::with_text(&format!("Line {i}"), Default::default()))).collect();
d.body = vec![std::sync::Arc::new(wordcraft_doc::Block::Table(t)), wordcraft_doc::para_block(wordcraft_doc::Paragraph::new())];
let l = lay(&d);
assert!(l.pages.len() >= 2, "the row splits");
for (n, page) in l.pages.iter().take(2).enumerate() {
let clips: Vec<Rect> = page.items.iter().filter_map(|it| if let Placed::Clip { rect, .. } = it { Some(*rect) } else { None }).collect();
assert_eq!(clips.len(), 1, "page {n}: {clips:?}");
let clip = clips[0];
let cells: Vec<Rect> = page.items.iter().filter_map(|it| if let Placed::Cell { rect, .. } = it { Some(*rect) } else { None }).collect();
assert!(cells.iter().any(|c| (c.y - clip.y).abs() < 0.01 && (c.h - clip.h).abs() < 0.01), "page {n}: {clip:?} is its cell part {cells:?}");
assert!(clip.y >= 0.0 && clip.bottom() <= page.h + 0.01, "page {n}: {clip:?}");
// The text starts near the top of its part of the cell.
let first = page.items.iter().find_map(|it| if let Placed::Lines { y, .. } = it { Some(*y) } else { None }).unwrap_or(f32::MAX);
assert!(first - clip.y < 20.0, "page {n}: text at {first}, cell at {clip:?}");
}
}
/// A table moved to a new page by a page break before its first paragraph shows the number of
/// the page it lands on in that cell's PAGE field, not the page it was laid out for.
#[test]
fn a_table_moved_by_a_page_break_shows_its_new_page_number() {
let mut d = Document::from_text("Before the table.");
let mut table = Table::new(1, 1, 468.0);
let mut first = wordcraft_doc::Paragraph::with_text("Page ", Default::default());
first.props.page_break_before = Some(true);
let page = InlineObject::Field { instr: "PAGE".into(), result: "9".into(), locked: false, dirty: true, code: false, form: String::new() };
first.insert_object(first.len(), page, &Default::default()).unwrap();
table.rows[0].cells[0].blocks = vec![wordcraft_doc::para_block(first)];
d.body.push(std::sync::Arc::new(wordcraft_doc::Block::Table(table)));
let l = lay(&d);
assert_eq!(l.pages.len(), 2);
let shown: Vec<String> = l.pages[1]
.items
.iter()
.flat_map(|it| match it {
Placed::Lines { para, .. } => para.shown.iter().map(|s| s.1.clone()).collect::<Vec<_>>(),
_ => Vec::new(),
})
.collect();
assert_eq!(shown, ["2"]);
}
+7 -4
View File
@@ -1,6 +1,6 @@
# How WordCraft interprets OpenDocument files
> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-04 · **Change:** minor (the two presets are Microsoft Word style and LibreOffice style; CLI values listed one by one); previously minor (deviations from the standard listed; ODF 1.2 → 1.3 checked); previously major (new: the ODF interpretation profile and the list of every case)
> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-04 · **Change:** trivial (what is written as read: unchanged paragraphs with drawings, comments, changes or marks too); previously minor (the two presets are Microsoft Word style and LibreOffice style; CLI values listed one by one); previously minor (deviations from the standard listed; ODF 1.2 → 1.3 checked); previously major (new: the ODF interpretation profile and the list of every case)
OpenDocument (ODF 1.3, OASIS) leaves some of a document's look to the application. Part 3 §16.2
says that when no style sets a property and the standard gives no default, "an
@@ -97,9 +97,11 @@ a save that doesn't touch it:
- named sections around unedited paragraphs, the document properties WordCraft doesn't model,
the declarations opening the text and the document's scripts.
Paragraphs and tables holding drawings, comments, tracked changes or ranged marks are written from
the model, as is everything edited. `odt::tests_more::lossless` saves samples without edits and
fails on anything lost.
Paragraphs and tables holding drawings, comments, tracked changes, notes or marks are written as
read too while those are unchanged; paragraphs tied together (a comment, change or mark reaching
into another, linked text boxes) are kept as a group. What is edited is written from the model, as
are the cases listed under "Left" in [odt-element-audit.md](odt-element-audit.md).
`odt::tests_more::lossless` saves samples without edits and fails on anything lost.
Files are saved as ODF 1.3, whatever version they were read as (as LibreOffice does). The changes
from ODF 1.2 (ODF 1.3 Part 3, Appendix G, with their OASIS issues) were checked: none gives a
@@ -178,6 +180,7 @@ one more `OdfPreset` with its values, and the cases above stay the same.
| Date | Change |
|---|---|
| 2026-11-04 | How WordCraft writes ODF: unchanged paragraphs and tables with drawings, comments, tracked changes, notes or marks are written as read too (the text said from the model); what is edited, and the cases left in the element audit, from the model. |
| 2026-11-04 | The two presets are the two styles a document can be shown in: Microsoft Word style (`word`, the default; `wordcraft`, its earlier id, still taken) and LibreOffice style (`libreoffice`). The CLI's values listed one by one. |
| 2026-11-03 | Deviations from the standard listed: unitless gradient and hatch angles in files older than ODF 1.3 are read as tenths of a degree, as their producers wrote them, out of necessity (in ODF 1.3 files, degrees, as the standard says). The ODF 1.2 → 1.3 changes checked. |
| 2026-11-03 | More written as read (tables, pages, sections, properties, declarations); files saved as ODF 1.3 with older markup brought to 1.3's meaning; drawings state `style:flow-with-text`. |