diff --git a/crates/engine/tests/it/font_scheme_minor.rs b/crates/engine/tests/it/font_scheme_minor.rs new file mode 100644 index 000000000..ca9350961 --- /dev/null +++ b/crates/engine/tests/it/font_scheme_minor.rs @@ -0,0 +1,29 @@ +//! A font a formatting command derives from the theme's body font keeps following it (#2517). + +use std::io::Read; + +use gridcraft_engine::Session; +use serde_json::json; + +fn styles_xml(session: &Session) -> String { + let bytes = gridcraft_xlsx::write_xlsx(&session.doc().unwrap().wb).unwrap(); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut xml = String::new(); + zip.by_name("xl/styles.xml").unwrap().read_to_string(&mut xml).unwrap(); + xml +} + +#[test] +fn derived_body_font_keeps_minor_scheme() { + let mut session = Session::new(); + session.new_workbook(); + session.execute("home.bold", json!({"range": "A1"})).unwrap(); + session.execute("home.fontSize", json!({"range": "B1", "size": 14})).unwrap(); + let xml = styles_xml(&session); + let fonts = xml.split("").next().unwrap(); + let fonts: Vec<&str> = fonts.split("").skip(1).collect(); + assert!(fonts.len() >= 3, "{xml}"); + for f in &fonts { + assert!(f.contains(""), "every body-font font follows the theme: {f}"); + } +} diff --git a/crates/engine/tests/it/main.rs b/crates/engine/tests/it/main.rs index 924a95d1b..4c91e58ac 100644 --- a/crates/engine/tests/it/main.rs +++ b/crates/engine/tests/it/main.rs @@ -11,6 +11,7 @@ mod custom_filter_blanks; mod custom_view_freeze; mod dynamicmetadata; mod extreme_selection_moves; +mod font_scheme_minor; mod format_as_table_restyle; mod hidden_sheet_navigation; mod ink_roundtrip; diff --git a/crates/model/src/style.rs b/crates/model/src/style.rs index ac4adac0d..1ded99288 100644 --- a/crates/model/src/style.rs +++ b/crates/model/src/style.rs @@ -193,6 +193,14 @@ pub struct Font { /// Font family class (1 roman, 2 swiss, 3 modern…); `None` is swiss, the default. #[serde(default)] pub family: Option, + /// The font follows the theme's body font (``) while its face is the + /// theme's minor font. Fonts derived from it (bold, a colour, a size) keep the link. + #[serde(default = "default_minor")] + pub minor: bool, +} + +fn default_minor() -> bool { + true } impl Eq for Font {} @@ -200,7 +208,7 @@ impl Hash for Font { fn hash(&self, h: &mut H) { self.name.hash(h); self.size.to_bits().hash(h); - (self.bold, self.italic, self.underline, self.strike, self.color, self.vert, self.charset, self.family).hash(h); + (self.bold, self.italic, self.underline, self.strike, self.color, self.vert, self.charset, self.family, self.minor).hash(h); } } @@ -217,6 +225,7 @@ impl Default for Font { vert: VertAlign::Baseline, charset: None, family: None, + minor: true, } } } diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index 774567bf8..b4587d61e 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -143,7 +143,13 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { let mut wb = Workbook::new(); wb.sheets.clear(); - // Styles and theme. Cell checkboxes live in the feature property bags, which styles refer to. + // Theme, then styles: a font's link to the theme's body font is read against it. + if let Some(r) = find_rel(&wb_rels, "theme").cloned() + && let Some(x) = cx.optional_xml(&r.target)? + { + wb.theme = crate::theme::read_theme(&x); + } + // Cell checkboxes live in the feature property bags, which styles refer to. let mut checkboxes = Vec::new(); if let Some(r) = find_rel(&wb_rels, "FeaturePropertyBag").cloned() && let Some(x) = cx.optional_xml(&r.target)? @@ -152,7 +158,7 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { } if let Some(r) = find_rel(&wb_rels, "styles").cloned() { if let Some(x) = cx.optional_xml(&r.target)? { - let StylesIn { xfs, dxfs, palette, named, num_fmts } = read_styles(&x, &checkboxes); + let StylesIn { xfs, dxfs, palette, named, num_fmts } = read_styles(&x, &checkboxes, &wb.theme.minor_font); cx.num_fmts = num_fmts; // Cell format 0 is the Normal cell format: everything on it (font, number format, // fill, borders, alignment, protection) is what a cell without its own format shows @@ -168,11 +174,6 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { } else { cx.warn("no styles part"); } - if let Some(r) = find_rel(&wb_rels, "theme").cloned() - && let Some(x) = cx.optional_xml(&r.target)? - { - wb.theme = crate::theme::read_theme(&x); - } // Shared strings, streamed item by item. Phonetic (furigana) runs are kept next to the text // they annotate, so an open + save does not delete them. if let Some(r) = find_rel(&wb_rels, "sharedStrings").cloned() { diff --git a/crates/xlsx/src/styles.rs b/crates/xlsx/src/styles.rs index e209d276b..0baff5c0f 100644 --- a/crates/xlsx/src/styles.rs +++ b/crates/xlsx/src/styles.rs @@ -93,6 +93,7 @@ pub fn read_font(e: &El, palette: &[u32]) -> Font { .and_then(|c| c.attr_u32("val")) .and_then(|v| u8::try_from(v).ok()) .filter(|v| *v <= MAX_FONT_FAMILY && *v != DEFAULT_FONT_FAMILY); + f.minor = e.child_val("scheme") == Some("minor"); f.color = opt_color(e.child("color"), palette); // Text 1 (dk1) is what "automatic" font colour means in practice. if f.color == Color::Theme(1, 0) { @@ -312,7 +313,8 @@ fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: & /// `checkboxes`: for each `xfComplement` index, whether it is a checkbox (see /// [`crate::feature_bags::checkbox_complements`]); empty when the file has no feature property bags. -pub fn read_styles(root: &El, checkboxes: &[bool]) -> StylesIn { +/// `minor_font`: the theme's body font, which fonts with `` follow. +pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn { let mut palette: Vec = INDEXED.to_vec(); if let Some(ix) = root.path(&["colors", "indexedColors"]) { let custom: Vec = ix @@ -334,7 +336,13 @@ pub fn read_styles(root: &El, checkboxes: &[bool]) -> StylesIn { } } } - let fonts: Vec = root.child("fonts").map(|f| f.kids("font").map(|e| read_font(e, &palette)).collect()).unwrap_or_default(); + // The scheme link only means something on a font in the body face; elsewhere it stays as a new font has it. + let body_font = |e: &El| { + let mut f = read_font(e, &palette); + f.minor |= f.name != minor_font; + f + }; + let fonts: Vec = root.child("fonts").map(|f| f.kids("font").map(body_font).collect()).unwrap_or_default(); let fills: Vec = root.child("fills").map(|f| f.kids("fill").map(|e| read_fill(e, &palette, false)).collect()).unwrap_or_default(); let borders: Vec = root.child("borders").map(|f| f.kids("border").map(|e| read_border(e, &palette)).collect()).unwrap_or_default(); let xfs = @@ -373,7 +381,8 @@ pub fn read_styles(root: &El, checkboxes: &[bool]) -> StylesIn { st.size = has_size && size == def.size; st.name = has_name && name == def.name; st.scheme = e.child_val("scheme").map(str::to_string); - s.font = Font { name, size, ..f }; + // A differential font's scheme is in `dxf_stated`. + s.font = Font { name, size, minor: s.font.minor, ..f }; } if let Some(n) = x.child("numFmt") { if let Some(code) = n.attr("formatCode") { @@ -753,8 +762,9 @@ pub fn write_styles(wb: &Workbook, dxfs: &[Style], extra_fmts: &[String]) -> (St } let _ = write!(s, "", fonts.len()); for (i, f) in fonts.iter().enumerate() { - // A Normal font in the theme's body face follows the theme, as Excel writes it (#805). - s.push_str(&font_xml(f, None, i == 0 && f.name == wb.theme.minor_font)); + // A Normal font in the theme's body face follows the theme, as Excel writes it (#805); so + // does every other body-face font that kept the scheme link (#2517). + s.push_str(&font_xml(f, None, (i == 0 || f.minor) && f.name == wb.theme.minor_font)); } let _ = write!(s, "", fills.len()); for f in &fills { diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index a68ed44e8..14fc784f9 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -1247,6 +1247,32 @@ fn default_font_survives_a_round_trip() { assert_eq!(font0(&saved), (Some("12".into()), Some("Calibri".into()), Some("2".into()), Some("minor".into()))); } +/// Every font that follows the theme's body font keeps ``, not only font 0, +/// and a body-face font without the scheme does not gain one (#2517). +#[test] +fn minor_scheme_survives_on_every_font() { + let styles = r#" + + + + "#; + let rels = r#""#; + let body = r#"12"#; + let file = super::minimal(body, &[("xl/styles.xml", styles)], rels, ""); + let saved = write_xlsx(&read_xlsx(&file).unwrap().0).unwrap(); + let mut pkg = Package::open(&saved).unwrap(); + let st = pkg.read_xml("xl/styles.xml").unwrap().unwrap(); + let fonts: Vec<(bool, bool, Option)> = st + .child("fonts") + .unwrap() + .kids("font") + .map(|f| (f.child("b").is_some(), f.child("i").is_some(), f.child("scheme").and_then(|e| e.attr("val")).map(str::to_string))) + .collect(); + let scheme = |b: bool, i: bool| fonts.iter().find(|f| f.0 == b && f.1 == i).map(|f| f.2.clone()); + assert_eq!(scheme(true, false), Some(Some("minor".into())), "{fonts:?}"); + assert_eq!(scheme(false, true), Some(None), "{fonts:?}"); +} + fn one_font_workbook(font: &str) -> Vec { let styles = format!( r#"{font}"#