From bc43965a4587dfe1dba24e7e2d795b9ed4933671 Mon Sep 17 00:00:00 2001 From: Shao Wenbin Date: Thu, 8 Oct 2026 20:14:16 +0800 Subject: [PATCH] CJK-MOJI7: validate spacing data and inspect nested typography rules --- ROADMAP.md | 1 + crates/doc/src/lib.rs | 4 +- crates/doc/src/mojikumi.rs | 15 +++++--- crates/engine/src/cmd/fonts.rs | 2 +- crates/engine/src/cmd/preflight.rs | 27 +++++++++++++- crates/idml/src/import.rs | 59 +++++++++++++++++++++--------- crates/idml/src/tests.rs | 32 ++++++++++++++++ 7 files changed, 114 insertions(+), 26 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 4946a9f1a..72d2186ad 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,5 +1,6 @@ # DesignCraft roadmap +- CJK-MOJI7: reject malformed Mojikumi interchange values and check nested note/cell paragraph rules in preflight. - CJK-MOJI6: apply spacing at shaped-cluster boundaries and distinguish centered Traditional Chinese punctuation from Simplified Chinese side bearings. - CJK-MOJI5: share equal-priority boundary spacing and honor discrete compression endpoints, including leading-only ragged-line fitting. - CJK-MOJI4: resolve all public Mojikumi preset names and their distinct paragraph/line-edge policies; correct consecutive punctuation defaults and small-kana classes. diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 32e0e90cf..33e0c7a9a 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -706,8 +706,8 @@ impl Document { } } for t in &self.styles.mojikumi_tables { - if t.overrides.iter().any(|r| ![r.minimum, r.desired, r.maximum].iter().all(|v| v.is_finite())) { - return Err(DocError::Invalid(format!("non-finite mojikumi spacing: {}", t.name))); + if t.overrides.iter().any(|r| !mojikumi::valid_range(r)) { + return Err(DocError::Invalid(format!("invalid mojikumi spacing: {}", t.name))); } } let mut ids = std::collections::HashSet::new(); diff --git a/crates/doc/src/mojikumi.rs b/crates/doc/src/mojikumi.rs index eda698371..14e4433c3 100644 --- a/crates/doc/src/mojikumi.rs +++ b/crates/doc/src/mojikumi.rs @@ -121,11 +121,7 @@ impl<'a> Rules<'a> { if !valid_class(r.target_class) || !valid_class(r.side_class) { return Err(format!("unsupported character classes {}/{}", r.target_class, r.side_class)); } - if ![r.minimum, r.desired, r.maximum].iter().all(|v| v.is_finite() && (-1.0..=100.0).contains(v)) - || r.minimum > r.desired - || r.desired > r.maximum - || !(0..=9).contains(&r.priority) - { + if !valid_range(r) { return Err("invalid spacing range or priority".into()); } } @@ -181,6 +177,15 @@ impl<'a> Rules<'a> { } } +/// Structural validation shared by native document loading and interchange. +/// Unknown integer classes remain preservable and are diagnosed by the resolver. +pub fn valid_range(r: &MojikumiAki) -> bool { + [r.minimum, r.desired, r.maximum].iter().all(|v| v.is_finite() && (-1.0..=100.0).contains(v)) + && r.minimum <= r.desired + && r.desired <= r.maximum + && (0..=9).contains(&r.priority) +} + pub fn valid_class(c: i16) -> bool { matches!(c, 1..=12 | 18 | 21..=33) } diff --git a/crates/engine/src/cmd/fonts.rs b/crates/engine/src/cmd/fonts.rs index 83acd685b..2184db7c0 100644 --- a/crates/engine/src/cmd/fonts.rs +++ b/crates/engine/src/cmd/fonts.rs @@ -29,7 +29,7 @@ pub fn specs() -> Vec { } /// Every story's text, including table cells and footnotes. -fn for_each_story(d: &Document, f: &mut dyn FnMut(&Story)) { +pub(super) fn for_each_story(d: &Document, f: &mut dyn FnMut(&Story)) { let mut pending: Vec<&Story> = d.stories.values().map(AsRef::as_ref).collect(); while let Some(st) = pending.pop() { f(st); diff --git a/crates/engine/src/cmd/preflight.rs b/crates/engine/src/cmd/preflight.rs index 22a60fa2c..5a69ff1bf 100644 --- a/crates/engine/src/cmd/preflight.rs +++ b/crates/engine/src/cmd/preflight.rs @@ -138,6 +138,9 @@ pub fn check(s: &Session, min_ppi: f64) -> Vec { page: location.and_then(|(_, page)| page), }); } + } + // Nested note and cell text uses paragraph rules too. + super::fonts::for_each_story(d, &mut |story| { let ranges = story.para_ranges(); for (pi, pf) in story.paras.iter().enumerate() { let (para, base) = d.styles.resolve_para(pf); @@ -183,7 +186,7 @@ pub fn check(s: &Session, min_ppi: f64) -> Vec { } } } - } + }); // Use the same effective character-to-font mapping as Find Font, including // anonymous note/cell stories. Unused composite entries are not missing text. for font in super::fonts::used_fonts(d) { @@ -403,3 +406,25 @@ mod generated_glyph_tests { assert_eq!(issue["page"], 0); } } + +#[cfg(test)] +mod mojikumi_nested_tests { + use super::*; + use std::sync::Arc; + #[test] + fn unknown_table_in_a_note_inside_a_cell_is_reported() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("frame.create", &json!({"rect":[20,20,300,400],"content":"text","text":"Body"})).unwrap(); + let d = Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let sid = *d.stories.keys().next().unwrap(); + let mut table = designcraft_doc::Table::new(1, 1, 1, 0, 0, 100.0); + let cell = &mut table.cells[0].text; + let mut pf = designcraft_doc::ParaFormat::default(); + pf.para.mojikumi = Some("MojikumiTable/Unrecognized".into()); + cell.insert_note(0, "Note", pf); + d.story_mut(sid).unwrap().insert_table(0, table); + let issues = check(&s, 150.0); + assert_eq!(issues.iter().filter(|i| i.kind == "unsupportedTypography" && i.message.contains("Unrecognized")).count(), 1); + } +} diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index c45775d98..932062724 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -143,6 +143,43 @@ fn lab_to_color(l: f32, a: f32, b: f32) -> Color { Color::rgb(rgb[0], rgb[1], rgb[2]) } +fn mojikumi_row(e: &El) -> Result { + let number = |key: &str, required: bool| -> Result { + let Some(raw) = e.prop(key) else { + return if required { Err(IdmlError::Invalid(format!("Mojikumi row is missing {key}"))) } else { Ok(0.0) }; + }; + raw.trim().parse::().ok().filter(|v| v.is_finite()).ok_or_else(|| IdmlError::Invalid(format!("Mojikumi row has invalid {key}"))) + }; + let integer = |key: &str, required: bool| -> Result { + let v = number(key, required)?; + if v.fract() != 0.0 || v < f64::from(i16::MIN) || v > f64::from(i16::MAX) { + return Err(IdmlError::Invalid(format!("Mojikumi {key} must be an in-range integer"))); + } + Ok(v as i16) + }; + let boolean = |key: &str| -> Result { + match e.prop(key).as_deref().map(str::trim) { + None | Some("false") => Ok(false), + Some("true") => Ok(true), + _ => Err(IdmlError::Invalid(format!("Mojikumi row has invalid {key}"))), + } + }; + let row = designcraft_doc::cjk::MojikumiAki { + target_class: integer("TargetMojikumiClass", true)?, + side_class: integer("SideMojikumiClass", true)?, + after: boolean("SideIsAfterTarget")?, + minimum: number("Minimum", false)?, + desired: number("Desired", false)?, + maximum: number("Maximum", false)?, + priority: integer("CompressionPriority", false)?, + does_not_float: boolean("AkiDoesNotFloat")?, + }; + if !designcraft_doc::mojikumi::valid_range(&row) { + return Err(IdmlError::Invalid("Mojikumi row has an invalid spacing range or priority".into())); + } + Ok(row) +} + fn nums(s: &str) -> Vec { s.split_whitespace().filter_map(|v| v.parse().ok()).collect() } @@ -316,23 +353,11 @@ impl<'r> Importer<'r> { if let (Some(id), Some(name)) = (e.get("Self"), e.get("Name")) { self.mojikumi_names.insert(id.into(), format!("MojikumiTable/{}", unescape_id(name))); } - let overrides = e - .prop_el("OverrideMojikumiAkiList") - .map(|l| { - l.find_all("OverrideMojikumiAkiType") - .map(|r| designcraft_doc::cjk::MojikumiAki { - target_class: r.num("TargetMojikumiClass").unwrap_or(0.0) as i16, - side_class: r.num("SideMojikumiClass").unwrap_or(0.0) as i16, - after: r.boolean("SideIsAfterTarget").unwrap_or(false), - minimum: r.num("Minimum").unwrap_or(0.0), - desired: r.num("Desired").unwrap_or(0.0), - maximum: r.num("Maximum").unwrap_or(0.0), - priority: r.num("CompressionPriority").unwrap_or(0.0) as i16, - does_not_float: r.boolean("AkiDoesNotFloat").unwrap_or(false), - }) - .collect() - }) - .unwrap_or_default(); + let overrides = if let Some(list) = e.prop_el("OverrideMojikumiAkiList") { + list.find_all("OverrideMojikumiAkiType").map(mojikumi_row).collect::>>()? + } else { + Vec::new() + }; self.styles.mojikumi_tables.push(designcraft_doc::cjk::MojikumiTable { name: unescape_id(e.get("Name").unwrap_or("")), based_on: e.get("BasedOnMojikumiSet").unwrap_or("").into(), diff --git a/crates/idml/src/tests.rs b/crates/idml/src/tests.rs index 651582ac4..79f242483 100644 --- a/crates/idml/src/tests.rs +++ b/crates/idml/src/tests.rs @@ -936,3 +936,35 @@ fn composite_references_resolve_by_identity_and_export_as_objects() { let back = import_idml(&output).unwrap(); assert_eq!(back.styles.composite_fonts, d.styles.composite_fonts); } + +#[test] +fn malformed_mojikumi_rows_are_not_silently_coerced() { + let good = r#""#; + for (key, value) in [ + ("TargetMojikumiClass", "12.9"), + ("SideMojikumiClass", "65536"), + ("Minimum", "NaN"), + ("Desired", "oops"), + ("Minimum", "0.4"), + ("Maximum", "0.1"), + ("CompressionPriority", "10"), + ("AkiDoesNotFloat", "maybe"), + ] { + let mut row = crate::xml::parse(good.as_bytes()).unwrap(); + row.set(key, value); + let mut xml = String::new(); + row.write(&mut xml, 0); + let map = DESIGNMAP.replace("", &format!("{xml}")); + let bytes = zip_files(&[ + ("designmap.xml", &map), + ("Resources/Graphic.xml", GRAPHIC), + ("Resources/Styles.xml", STYLES), + ("Resources/Preferences.xml", PREFS), + ("MasterSpreads/MasterSpread_m1.xml", MASTER), + ("Spreads/Spread_sp1.xml", SPREAD), + ("Stories/Story_s1.xml", STORY), + ]); + let err = import_idml(&bytes).unwrap_err().to_string(); + assert!(err.contains("Mojikumi"), "{key}={value}: {err}"); + } +}