ODT: table and row backgrounds show behind their cells (gap 19) (#5727)

* ODT gap 19: table and row backgrounds show behind their cells

fo:background-color on a table or row wasn't read, so it was missing on screen
and gone in LibreOffice after a save. WordCraft's tables have their backgrounds
on their cells, so it goes on the cells with none of their own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ODT: a column's default cell style shows, and transparent clears a background (gap 19)

A cell with no style took the row's or table's colour over its column's default
cell style, and `transparent` on a table or row fell through to its parent style's.
From CodeRabbit on this PR, checked in LibreOffice 26.8.2.2: a row's default cell
style isn't applied there, so it isn't here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ODT: a column's default background is read once, not its style's name (gap 19)

The name was cloned for every repeat of a column, which a hostile file could make
use to run out of memory, and allocation failure can't be caught.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ODT: a transparent cell style stops its parent's colour (gap 19)

A cell style saying transparent over a coloured parent took the parent's colour and hid the row behind it. LibreOffice 26.8 shows the row, from a column's default cell style or the cell's own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Gap 19: docs status lines are main's, with this change in front

Merging main had kept this branch's older status lines in ROADMAP.md and
docs/file-format-parity.md, which set main's 2026-11-08 date back and dropped
main's newer entries and the parity doc's CLI path note.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Aleksander Helgaker
2026-11-09 15:56:09 +01:00
committed by GitHub
co-authored by Claude Opus 5.5
parent d102b2f08b
commit d75364067d
6 changed files with 144 additions and 7 deletions
+3 -1
View File
File diff suppressed because one or more lines are too long
+25 -2
View File
@@ -1300,10 +1300,12 @@ impl Body<'_> {
}
"table-column" => {
let w = get(a, "table:style-name").and_then(|n| self.styles.column_width(n)).unwrap_or(0.0);
let cells = get(a, "table:default-cell-style-name").and_then(|s| self.styles.cell_background(s));
let n = get(a, "table:number-columns-repeated").and_then(|v| v.trim().parse::<usize>().ok()).unwrap_or(1).max(1);
if let Some(t) = self.tables.last_mut() {
let room = wordcraft_doc::table::MAX_COLS.saturating_sub(t.widths.len());
t.widths.extend(std::iter::repeat_n(w, n.min(room)));
t.column_backgrounds.extend(std::iter::repeat_n(cells, n.min(room)));
}
}
"table-row" => self.start_row(a),
@@ -1311,7 +1313,16 @@ impl Body<'_> {
self.end_para();
let span = get(a, "table:number-columns-spanned").and_then(|v| v.trim().parse::<u32>().ok()).unwrap_or(1).clamp(1, 63);
let rows = get(a, "table:number-rows-spanned").and_then(|v| v.trim().parse::<u32>().ok()).unwrap_or(1).clamp(1, 1000);
let shading = get(a, "table:style-name").and_then(|s| self.styles.cell_background(s));
// WordCraft's tables have their backgrounds on their cells: a row's or table's on
// those with none of their own, as it shows behind them. A cell with no style has
// its column's default cell style; LibreOffice doesn't apply a row's.
let behind = self.tables.last().and_then(|t| t.row_background.or(t.background));
let in_column = self.tables.last().and_then(|t| {
let at: usize = t.row.iter().map(|c| c.colspan.max(1) as usize).sum();
t.column_backgrounds.get(at).copied().flatten()
});
let own = get(a, "table:style-name").map_or(in_column, |s| self.styles.cell_background(s));
let shading = own.or(behind);
let valign = get(a, "table:style-name").and_then(|s| self.styles.cell_valign(s));
let borders = get(a, "table:style-name").and_then(|s| self.styles.cell_borders(s));
let margins = get(a, "table:style-name").and_then(|s| self.styles.cell_padding(s));
@@ -1393,14 +1404,26 @@ impl Body<'_> {
let space = get(a, "table:style-name").map(|s| self.styles.table_space(s)).unwrap_or_default();
let extra =
model::TableExtra { align, width: None, width_pct, heights: Vec::new(), keep_rows: Vec::new(), fixed: true, space, header_rows: 0 };
self.tables.push(TableB { rows: Vec::new(), row: Vec::new(), hcover: 0, widths: Vec::new(), extra });
let background = get(a, "table:style-name").and_then(|s| self.styles.table_background(s));
self.tables.push(TableB {
rows: Vec::new(),
row: Vec::new(),
hcover: 0,
widths: Vec::new(),
column_backgrounds: Vec::new(),
extra,
background,
row_background: None,
});
}
/// A table row (`table:table-row`) begins: its height, kept as rows are (see the row's end).
fn start_row(&mut self, a: &Attrs) {
let height = get(a, "table:style-name").and_then(|s| self.styles.row_height(s));
let background = get(a, "table:style-name").and_then(|s| self.styles.table_background(s));
let keep = self.styles.row_keep(get(a, "table:style-name"));
if let Some(t) = self.tables.last_mut() {
t.row_background = background;
t.row.clear();
t.hcover = 0;
if t.extra.heights.len() < wordcraft_doc::table::MAX_ROWS {
+5
View File
@@ -75,6 +75,11 @@ pub(in crate::odt) struct TableB {
pub(in crate::odt) widths: Vec<f32>,
/// Its alignment and width, and each row's height so far (see [`crate::model::TableExtra`]).
pub(in crate::odt) extra: crate::model::TableExtra,
/// The background of each column's default cell style (`table:default-cell-style-name`).
pub(in crate::odt) column_backgrounds: Vec<Option<wordcraft_doc::props::Rgb>>,
/// The table's background, and the row's being read.
pub(in crate::odt) background: Option<wordcraft_doc::props::Rgb>,
pub(in crate::odt) row_background: Option<wordcraft_doc::props::Rgb>,
}
impl TableB {
+19 -3
View File
@@ -229,7 +229,7 @@ struct StyleInfo {
outline: Option<u8>,
border_bottom: bool,
/// `fo:background-color` of a table-cell style's `style:table-cell-properties`.
cell_background: Option<Rgb>,
cell_background: Option<Option<Rgb>>,
/// A table-cell style's background picture (§17.3 `style:background-image`): its package
/// path, `style:repeat` and `style:position`.
cell_picture: Option<(String, String, String)>,
@@ -252,6 +252,9 @@ struct StyleInfo {
/// A table-row style's `style:table-row-properties`: its height (`style:row-height`, exact)
/// or least height (`style:min-row-height`), points.
row_height: Option<(f32, bool)>,
/// `fo:background-color` of a table or table-row style (§20.175): what shows behind cells
/// with none of their own. `Some(None)`: `transparent`, which clears a parent style's.
table_background: Option<Option<Rgb>>,
/// `fo:keep-together` of a table-row style: the row doesn't break across pages.
row_keep: Option<bool>,
/// `style:master-page-name` of a paragraph or table style: its paragraphs (tables) start a
@@ -573,6 +576,12 @@ pub(super) fn parse_border(v: &str) -> Option<Border> {
/// A property element `name` (`q` in full) of the style being read, into `s`: true when it is
/// one (paragraph, table, column, cell, section and graphic properties, tab stops).
/// What `fo:background-color` says of a table or row: a colour, or none (`transparent`).
fn table_background_of(a: &Attrs) -> Option<Option<Rgb>> {
let bg = get(a, "fo:background-color")?;
if bg.trim() == "transparent" { Some(None) } else { model::parse_color(bg).map(Some) }
}
fn style_child(name: &str, q: &str, a: &Attrs, s: &mut StyleInfo) -> bool {
match name {
"paragraph-properties" => paragraph_properties(a, s),
@@ -599,6 +608,7 @@ fn style_child(name: &str, q: &str, a: &Attrs, s: &mut StyleInfo) -> bool {
s.page_number = Some(v.trim().parse::<u32>().ok().map(|n| n.min(32_767)));
}
// §20.300 `table:align`: `margins` fills the text width, as no alignment does.
s.table_background = table_background_of(a).or(s.table_background);
s.table_align = match get(a, "table:align").map(str::trim) {
Some("left") => Some(Align::Left),
Some("center") => Some(Align::Center),
@@ -620,6 +630,7 @@ fn style_child(name: &str, q: &str, a: &Attrs, s: &mut StyleInfo) -> bool {
}
}
"table-row-properties" => {
s.table_background = table_background_of(a).or(s.table_background);
if let Some(h) = get(a, "style:row-height").and_then(length).filter(|h| h.is_finite() && *h > 0.0) {
s.row_height = Some((h.min(MAX_LEN), true));
} else if let Some(h) = get(a, "style:min-row-height").and_then(length).filter(|h| h.is_finite() && *h > 0.0) {
@@ -677,7 +688,7 @@ pub(super) fn writing_direction(m: &str) -> wordcraft_doc::props::TextDirection
/// A table-cell style's `style:table-cell-properties` (ODF 1.3 §17.18): background, borders,
/// padding and vertical alignment.
fn table_cell_properties(a: &Attrs, s: &mut StyleInfo) {
if let Some(c) = get(a, "fo:background-color").and_then(model::parse_color) {
if let Some(c) = table_background_of(a) {
s.cell_background = Some(c);
}
cell_borders_and_padding(a, s);
@@ -1379,8 +1390,13 @@ impl Styles {
self.chain(name).iter().find_map(|(_, s)| s.cell_picture.clone())
}
pub(super) fn table_background(&self, name: &str) -> Option<Rgb> {
self.chain(name).iter().find_map(|(_, s)| s.table_background).flatten()
}
/// Background of a table-cell style (whole chain).
pub(super) fn cell_background(&self, name: &str) -> Option<Rgb> {
self.chain(name).iter().find_map(|(_, s)| s.cell_background)
self.chain(name).iter().find_map(|(_, s)| s.cell_background).flatten()
}
/// Borders of a table-cell style (whole chain).
@@ -160,6 +160,96 @@ fn a_table_starting_a_page_starts_it() {
assert_eq!(c.matches(r#"fo:break-before="page""#).count(), 2, "{c}");
}
#[test]
fn a_tables_and_rows_background_show_behind_their_cells() {
// Table Properties › Background: it was dropped, on screen and in LibreOffice after a save.
let styles = r##"<style:style style:name="T" style:family="table"><style:table-properties fo:background-color="#ddffdd"/></style:style><style:style style:name="R" style:family="table-row"><style:table-row-properties fo:background-color="#ffdddd"/></style:style><style:style style:name="C" style:family="table-cell"><style:table-cell-properties fo:background-color="#ddddff"/></style:style>"##;
let table = r#"<table:table table:style-name="T"><table:table-row><table:table-cell><text:p>table</text:p></table:table-cell><table:table-cell table:style-name="C"><text:p>cell</text:p></table:table-cell></table:table-row><table:table-row table:style-name="R"><table:table-cell><text:p>row</text:p></table:table-cell><table:table-cell table:style-name="C"><text:p>cell</text:p></table:table-cell></table:table-row></table:table>"#;
let colours = |d: &wordcraft_doc::Document| {
d.body
.iter()
.find_map(|b| b.as_table())
.map(|t| t.rows.iter().flat_map(|r| r.cells.iter().map(|c| c.props.shading.map(|s| s.hex().to_ascii_lowercase()))).collect::<Vec<_>>())
.unwrap_or_default()
};
let want = |t: &str, r: &str, c: &str| vec![Some(t.to_string()), Some(c.to_string()), Some(r.to_string()), Some(c.to_string())];
let d = doc(styles, table);
assert_eq!(colours(&d), want("ddffdd", "ffdddd", "ddddff"));
assert_eq!(colours(&import(&crate::odt::tests::export_model(&d).unwrap()).unwrap()), want("ddffdd", "ffdddd", "ddddff"));
}
/// As LibreOffice shows them: a cell's own style, else its column's default cell style (a row's
/// isn't applied), else the row's background, else the table's. A row or table clearing its
/// background (`transparent`) doesn't take its parent's; the table's shows behind that row.
#[test]
fn a_cells_background_comes_from_its_column_then_row_then_table() {
let cell = |n: &str, c: &str| {
format!(
r##"<style:style style:name="{n}" style:family="table-cell"><style:table-cell-properties fo:background-color="#{c}"/></style:style>"##
)
};
let styles = format!(
r##"{}{}<style:style style:name="T" style:family="table"><style:table-properties fo:background-color="#ffff00"/></style:style><style:style style:name="R" style:family="table-row"><style:table-row-properties fo:background-color="#0000ff"/></style:style><style:style style:name="Clear" style:family="table-row" style:parent-style-name="R"><style:table-row-properties fo:background-color="transparent"/></style:style>"##,
cell("InRow", "ff0000"),
cell("InColumn", "00ff00"),
);
let c = "<table:table-cell><text:p>x</text:p></table:table-cell>";
let table = format!(
r#"<table:table table:style-name="T"><table:table-column table:default-cell-style-name="InColumn"/><table:table-column/><table:table-row table:style-name="R" table:default-cell-style-name="InRow">{c}{c}</table:table-row><table:table-row>{c}{c}</table:table-row><table:table-row table:style-name="Clear">{c}{c}</table:table-row></table:table>"#
);
let colours = |d: &wordcraft_doc::Document| {
d.body
.iter()
.find_map(|b| b.as_table())
.map(|t| t.rows.iter().flat_map(|r| r.cells.iter().map(|c| c.props.shading.map(|s| s.hex().to_ascii_lowercase()))).collect::<Vec<_>>())
.unwrap_or_default()
};
let s = |v: &str| Some(v.to_string());
let want = vec![s("00ff00"), s("0000ff"), s("00ff00"), s("ffff00"), s("00ff00"), s("ffff00")];
let d = doc(&styles, &table);
assert_eq!(colours(&d), want);
assert_eq!(colours(&import(&crate::odt::tests::export_model(&d).unwrap()).unwrap()), want);
}
/// A column default after a spanning cell belongs to the column the cell is in.
#[test]
fn a_column_default_counts_spanned_columns() {
let styles =
r##"<style:style style:name="C" style:family="table-cell"><style:table-cell-properties fo:background-color="#00ff00"/></style:style>"##;
let table = r#"<table:table><table:table-column/><table:table-column/><table:table-column table:default-cell-style-name="C"/><table:table-row><table:table-cell table:number-columns-spanned="2"><text:p>a</text:p></table:table-cell><table:covered-table-cell/><table:table-cell><text:p>b</text:p></table:table-cell></table:table-row></table:table>"#;
let d = doc(styles, table);
let t = d.body.iter().find_map(|b| b.as_table()).unwrap();
let last = t.rows[0].cells.iter().rev().find(|c| !c.blocks.is_empty()).unwrap();
assert_eq!(last.props.shading.map(|s| s.hex().to_ascii_lowercase()).as_deref(), Some("00ff00"));
}
/// A column's default cell style that names nothing there is no background, however long its name.
#[test]
fn a_column_default_naming_no_style_is_no_background() {
let long = "x".repeat(100_000);
let table = format!(
r#"<table:table><table:table-column table:number-columns-repeated="63" table:default-cell-style-name="{long}"/><table:table-row><table:table-cell><text:p>a</text:p></table:table-cell></table:table-row></table:table>"#
);
let d = doc("", &table);
let t = d.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.rows[0].cells[0].props.shading, None);
}
/// A cell style saying `transparent` over a coloured parent has no background of its own: what's
/// behind it shows, in LibreOffice too, from a column's default cell style or the cell's own.
#[test]
fn a_transparent_cell_style_stops_its_parents_colour() {
let styles = r##"<style:style style:name="PC" style:family="table-cell"><style:table-cell-properties fo:background-color="#ff0000"/></style:style><style:style style:name="KC" style:family="table-cell" style:parent-style-name="PC"><style:table-cell-properties fo:background-color="transparent"/></style:style><style:style style:name="R" style:family="table-row"><style:table-row-properties fo:background-color="#0000ff"/></style:style>"##;
for (column, cell) in [(r#" table:default-cell-style-name="KC""#, ""), ("", r#" table:style-name="KC""#)] {
let table = format!(
r#"<table:table><table:table-column{column}/><table:table-row table:style-name="R"><table:table-cell{cell}><text:p>a</text:p></table:table-cell></table:table-row></table:table>"#
);
let d = doc(styles, &table);
let t = d.body.iter().find_map(|b| b.as_table()).unwrap();
assert_eq!(t.rows[0].cells[0].props.shading, crate::model::parse_color("#0000ff"), "{column}{cell}");
}
}
#[test]
fn a_row_that_keeps_together_says_so() {
// Table › Text Flow: a row not to break across pages. It came back from a save free to break.
File diff suppressed because one or more lines are too long