diff --git a/src/fontinfo.rs b/src/fontinfo.rs index e39a4fa4..2377ec49 100644 --- a/src/fontinfo.rs +++ b/src/fontinfo.rs @@ -1724,6 +1724,16 @@ mod tests { assert_eq!(font_info.open_type_os2_vendor_id, Some("LTTR".into())); } + #[test] + fn fontinfo_empty_guideline_name() { + let path = "testdata/fontinfo_empty_guideline_name.plist"; + let font_info: FontInfo = plist::from_file(path).expect("failed to load fontinfo"); + let guidelines = font_info.guidelines.expect("guidelines should be present"); + assert_eq!(guidelines.len(), 2); + assert_eq!(guidelines[0].name, None); + assert_eq!(guidelines[1].name, Some(Name::new_raw("ascender"))); + } + #[test] fn fontinfo2() { let path = "testdata/fontinfotest.ufo/fontinfo.plist"; diff --git a/src/glyph/parse.rs b/src/glyph/parse.rs index a4e51879..54afc24c 100644 --- a/src/glyph/parse.rs +++ b/src/glyph/parse.rs @@ -504,6 +504,9 @@ impl GlifParser { } angle = Some(angle_value); } + // The name is optional, and some real-world fonts write it as + // an explicit empty string; treat that as no name. + b"name" if value.is_empty() => (), b"name" => name = Some(Name::new(&value).map_err(|_| ErrorKind::InvalidName)?), b"color" => color = Some(value.parse().map_err(|_| ErrorKind::BadColor)?), b"identifier" => { diff --git a/src/glyph/tests.rs b/src/glyph/tests.rs index 73e2c750..71f7da15 100644 --- a/src/glyph/tests.rs +++ b/src/glyph/tests.rs @@ -253,6 +253,19 @@ fn guidelines() { assert_eq!(glyph.width, 364.); } +#[test] +fn empty_guideline_name() { + let data = r#" + + + + +"#; + let glyph = parse_glyph(data.as_bytes()).unwrap(); + assert_eq!(glyph.guidelines.len(), 1); + assert_eq!(glyph.guidelines[0].name, None); +} + #[test] #[should_panic(expected = "MissingClose")] fn missing_close() { diff --git a/src/guideline.rs b/src/guideline.rs index 41ba50e0..b5dadec5 100644 --- a/src/guideline.rs +++ b/src/guideline.rs @@ -94,11 +94,24 @@ struct RawGuideline { x: Option, y: Option, angle: Option, + #[serde(default, deserialize_with = "deserialize_optional_name")] name: Option, color: Option, identifier: Option, } +// The name is optional, and some real-world fonts write it as an explicit +// empty string; treat that as no name instead of failing the load. +fn deserialize_optional_name<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + match Option::::deserialize(deserializer)? { + Some(name) if !name.is_empty() => name.parse::().map(Some).map_err(de::Error::custom), + _ => Ok(None), + } +} + impl Serialize for Guideline { fn serialize(&self, serializer: S) -> Result where @@ -171,7 +184,7 @@ impl<'de> Deserialize<'de> for Guideline { mod tests { use super::*; - use serde_test::{assert_tokens, Token}; + use serde_test::{assert_de_tokens, assert_tokens, Token}; #[test] fn guideline_parsing() { @@ -207,4 +220,22 @@ mod tests { ], ); } + + #[test] + fn empty_guideline_name_is_no_name() { + let expected = Guideline::new(Line::Vertical(495.0), None, None, None); + assert_de_tokens( + &expected, + &[ + Token::Struct { name: "RawGuideline", len: 2 }, + Token::Str("x"), + Token::Some, + Token::F64(495.0), + Token::Str("name"), + Token::Some, + Token::Str(""), + Token::StructEnd, + ], + ); + } } diff --git a/testdata/fontinfo_empty_guideline_name.plist b/testdata/fontinfo_empty_guideline_name.plist new file mode 100644 index 00000000..05fda613 --- /dev/null +++ b/testdata/fontinfo_empty_guideline_name.plist @@ -0,0 +1,23 @@ + + + + + familyName + Test + guidelines + + + name + + y + 495 + + + name + ascender + y + 750 + + + +