From 587acb822527214428b97e8755168ca81d43376e Mon Sep 17 00:00:00 2001 From: Yonghye Kwon Date: Sun, 26 Jul 2026 02:37:09 +0900 Subject: [PATCH] fix(xlsx): read self-closing drawing run properties Excel writes a shape label's run properties as a self-closing element, ``, which quick-xml reports as `Event::Empty`. The drawing reader matched `rPr` only in its `Event::Start` arm, so every attribute on that spelling was discarded and the run fell back to the renderer's default 11pt regular. The default happens to equal the common `sz="1100"`, which hid the defect until a label used another size or weight: a 14pt bold italic label still rendered 11pt regular. Extract the attribute loop into `apply_run_properties` and call it from both arms, so a run's size, bold, and italic resolve identically whether or not its `` has children. No committed fixture is affected: the only self-closing run properties in the corpus use `sz="1100"`. Rendering all 68 XLSX fixtures before and after leaves 67 byte-identical in layout, and the 68th differs only by 1e-5 pt of float noise on two words, from taking Typst's explicit-11pt path instead of its default-11pt path. Fixes #466 Signed-off-by: Yonghye Kwon --- crates/office2pdf/src/parser/xlsx_drawing.rs | 51 +++++++++++-------- .../src/parser/xlsx_drawing_tests.rs | 44 ++++++++++++++++ 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/crates/office2pdf/src/parser/xlsx_drawing.rs b/crates/office2pdf/src/parser/xlsx_drawing.rs index 550ed74b..3dae7b9c 100644 --- a/crates/office2pdf/src/parser/xlsx_drawing.rs +++ b/crates/office2pdf/src/parser/xlsx_drawing.rs @@ -746,6 +746,35 @@ fn resolved_or_legacy( }) } +/// Apply an `` element's size and emphasis attributes to a run +/// style. Called from both the `Start` and `Empty` arms of the drawing +/// reader: Excel writes shape-label run properties as a self-closing +/// element, which quick-xml reports as `Empty`, and reading them on `Start` +/// alone dropped every attribute (issue #466). +fn apply_run_properties( + style: &mut crate::ir::TextStyle, + element: &quick_xml::events::BytesStart<'_>, +) { + for attr in element.attributes().flatten() { + match attr.key.local_name().as_ref() { + b"sz" => { + if let Ok(value) = attr.unescape_value() + && let Ok(hundredths_pt) = value.parse::() + { + style.font_size = Some(hundredths_pt / 100.0); + } + } + b"b" if attr.unescape_value().ok().as_deref() == Some("1") => { + style.bold = Some(true); + } + b"i" if attr.unescape_value().ok().as_deref() == Some("1") => { + style.italic = Some(true); + } + _ => {} + } + } +} + /// Parse `` text boxes from a worksheet drawing, resolving scheme /// colors against the workbook theme palette and run typefaces against its /// font scheme. @@ -846,26 +875,7 @@ pub(super) fn parse_drawing_text_boxes( in_run = true; current_style = TextStyle::default(); } - b"rPr" if in_run => { - for attr in e.attributes().flatten() { - match attr.key.local_name().as_ref() { - b"sz" => { - if let Ok(v) = attr.unescape_value() - && let Ok(sz) = v.parse::() - { - current_style.font_size = Some(sz / 100.0); - } - } - b"b" if attr.unescape_value().ok().as_deref() == Some("1") => { - current_style.bold = Some(true); - } - b"i" if attr.unescape_value().ok().as_deref() == Some("1") => { - current_style.italic = Some(true); - } - _ => {} - } - } - } + b"rPr" if in_run => apply_run_properties(&mut current_style, e), b"t" if in_run => in_text = true, b"srgbClr" | b"schemeClr" => { let parsed = @@ -905,6 +915,7 @@ pub(super) fn parse_drawing_text_boxes( current_style.font_family = Some(family); } } + b"rPr" if in_run => apply_run_properties(&mut current_style, e), b"pPr" if current_para.is_some() => { for attr in e.attributes().flatten() { if attr.key.local_name().as_ref() == b"algn" diff --git a/crates/office2pdf/src/parser/xlsx_drawing_tests.rs b/crates/office2pdf/src/parser/xlsx_drawing_tests.rs index 0edbc9bc..04650033 100644 --- a/crates/office2pdf/src/parser/xlsx_drawing_tests.rs +++ b/crates/office2pdf/src/parser/xlsx_drawing_tests.rs @@ -261,3 +261,47 @@ fn theme_font_scheme_ignores_empty_typefaces() { let fonts = crate::parser::drawingml::parse_theme_font_scheme(theme_xml); assert_eq!(fonts, ThemeFontScheme::default()); } + +/// `(font_size, bold, italic)` of the single run the fixture produces. +fn run_emphasis(run_properties: &str) -> (Option, Option, Option) { + let boxes = parse_drawing_text_boxes( + &drawing_with_run_properties(run_properties), + &HashMap::new(), + &ThemeFontScheme::default(), + ); + assert_eq!(boxes.len(), 1, "fixture should yield one text box"); + let style = &boxes[0].paragraphs[0].runs[0].style; + (style.font_size, style.bold, style.italic) +} + +#[test] +fn self_closing_run_properties_still_carry_size_and_emphasis() { + // Excel writes shape-label run properties as a self-closing element, + // which quick-xml reports as `Event::Empty`; reading them only on + // `Event::Start` dropped every attribute (issue #466). + assert_eq!( + run_emphasis(r#""#), + (Some(14.0), Some(true), Some(true)) + ); +} + +#[test] +fn run_properties_with_children_keep_working() { + // Triangulation: the same attributes on an element that has children + // must resolve identically, so the fix cannot special-case one spelling. + assert_eq!( + run_emphasis( + r#""# + ), + (Some(14.0), Some(true), Some(true)) + ); +} + +#[test] +fn run_properties_omit_emphasis_when_disabled() { + // `b="0"`/`i="0"` mean "not bold"/"not italic"; they must not set the flag. + assert_eq!( + run_emphasis(r#""#), + (Some(9.0), None, None) + ); +}