Skip to content

Commit 1c8ac19

Browse files
tomconroyclaude
authored andcommitted
Treat empty guideline names as no name
The UFO spec marks a guideline's name as optional, and some real-world fonts write it as an explicit empty string (e.g. five UFOs in Google's Roboto Serif sources). Name validation rejected these, failing the whole fontinfo.plist (or .glif) load. Treat an empty name as an absent one when reading, matching fontTools; writing is unchanged and cannot produce an empty name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a345dd4 commit 1c8ac19

5 files changed

Lines changed: 81 additions & 1 deletion

File tree

src/fontinfo.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,6 +1724,16 @@ mod tests {
17241724
assert_eq!(font_info.open_type_os2_vendor_id, Some("LTTR".into()));
17251725
}
17261726

1727+
#[test]
1728+
fn fontinfo_empty_guideline_name() {
1729+
let path = "testdata/fontinfo_empty_guideline_name.plist";
1730+
let font_info: FontInfo = plist::from_file(path).expect("failed to load fontinfo");
1731+
let guidelines = font_info.guidelines.expect("guidelines should be present");
1732+
assert_eq!(guidelines.len(), 2);
1733+
assert_eq!(guidelines[0].name, None);
1734+
assert_eq!(guidelines[1].name, Some(Name::new_raw("ascender")));
1735+
}
1736+
17271737
#[test]
17281738
fn fontinfo2() {
17291739
let path = "testdata/fontinfotest.ufo/fontinfo.plist";

src/glyph/parse.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,9 @@ impl GlifParser {
504504
}
505505
angle = Some(angle_value);
506506
}
507+
// The name is optional, and some real-world fonts write it as
508+
// an explicit empty string; treat that as no name.
509+
b"name" if value.is_empty() => (),
507510
b"name" => name = Some(Name::new(&value).map_err(|_| ErrorKind::InvalidName)?),
508511
b"color" => color = Some(value.parse().map_err(|_| ErrorKind::BadColor)?),
509512
b"identifier" => {

src/glyph/tests.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,19 @@ fn guidelines() {
253253
assert_eq!(glyph.width, 364.);
254254
}
255255

256+
#[test]
257+
fn empty_guideline_name() {
258+
let data = r#"
259+
<?xml version="1.0" encoding="UTF-8"?>
260+
<glyph name="period" format="2">
261+
<guideline y="495" name=""/>
262+
</glyph>
263+
"#;
264+
let glyph = parse_glyph(data.as_bytes()).unwrap();
265+
assert_eq!(glyph.guidelines.len(), 1);
266+
assert_eq!(glyph.guidelines[0].name, None);
267+
}
268+
256269
#[test]
257270
#[should_panic(expected = "MissingClose")]
258271
fn missing_close() {

src/guideline.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,24 @@ struct RawGuideline {
9494
x: Option<f64>,
9595
y: Option<f64>,
9696
angle: Option<f64>,
97+
#[serde(default, deserialize_with = "deserialize_optional_name")]
9798
name: Option<Name>,
9899
color: Option<Color>,
99100
identifier: Option<Identifier>,
100101
}
101102

103+
// The name is optional, and some real-world fonts write it as an explicit
104+
// empty string; treat that as no name instead of failing the load.
105+
fn deserialize_optional_name<'de, D>(deserializer: D) -> Result<Option<Name>, D::Error>
106+
where
107+
D: Deserializer<'de>,
108+
{
109+
match Option::<String>::deserialize(deserializer)? {
110+
Some(name) if !name.is_empty() => name.parse::<Name>().map(Some).map_err(de::Error::custom),
111+
_ => Ok(None),
112+
}
113+
}
114+
102115
impl Serialize for Guideline {
103116
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
104117
where
@@ -171,7 +184,7 @@ impl<'de> Deserialize<'de> for Guideline {
171184
mod tests {
172185
use super::*;
173186

174-
use serde_test::{assert_tokens, Token};
187+
use serde_test::{assert_de_tokens, assert_tokens, Token};
175188

176189
#[test]
177190
fn guideline_parsing() {
@@ -207,4 +220,22 @@ mod tests {
207220
],
208221
);
209222
}
223+
224+
#[test]
225+
fn empty_guideline_name_is_no_name() {
226+
let expected = Guideline::new(Line::Vertical(495.0), None, None, None);
227+
assert_de_tokens(
228+
&expected,
229+
&[
230+
Token::Struct { name: "RawGuideline", len: 2 },
231+
Token::Str("x"),
232+
Token::Some,
233+
Token::F64(495.0),
234+
Token::Str("name"),
235+
Token::Some,
236+
Token::Str(""),
237+
Token::StructEnd,
238+
],
239+
);
240+
}
210241
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
3+
<plist version="1.0">
4+
<dict>
5+
<key>familyName</key>
6+
<string>Test</string>
7+
<key>guidelines</key>
8+
<array>
9+
<dict>
10+
<key>name</key>
11+
<string></string>
12+
<key>y</key>
13+
<integer>495</integer>
14+
</dict>
15+
<dict>
16+
<key>name</key>
17+
<string>ascender</string>
18+
<key>y</key>
19+
<integer>750</integer>
20+
</dict>
21+
</array>
22+
</dict>
23+
</plist>

0 commit comments

Comments
 (0)