diff --git a/crates/fonts/src/fontdb/fallback.rs b/crates/fonts/src/fontdb/fallback.rs index ad8acc538..13afffd07 100644 --- a/crates/fonts/src/fontdb/fallback.rs +++ b/crates/fonts/src/fontdb/fallback.rs @@ -118,9 +118,9 @@ pub(super) const KOREAN_SANS_FALLBACKS: &[&str] = &[ pub fn sans_by_name(family: &str) -> Option { let lower = family.to_lowercase(); let has = |needles: &[&str]| needles.iter().any(|n| lower.contains(n)); - // Serif first: "Myungjo Gothic" does not exist, but a name holding both reads as the serif - // it names, and "Sans" inside "Noto Sans Myungjo" would otherwise win. - if has(&["batang", "myungjo", "myeongjo", "gungsuh", "serif", "\u{BC14}\u{D5D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) { + // A Korean serif name comes first: "Myungjo Gothic" does not exist, but a name holding both + // reads as the serif it names, and "Sans" inside "Noto Sans Myungjo" would otherwise win. + if has(&["batang", "myungjo", "myeongjo", "gungsuh", "\u{BC14}\u{D5D5}", "\u{BA85}\u{C870}", "\u{AD81}\u{C11C}"]) { return Some(false); } if has(&[ @@ -138,6 +138,11 @@ pub fn sans_by_name(family: &str) -> Option { ]) { return Some(true); } + // Only now the bare word, which "sans serif" and "sans-serif" end with and do not mean + // (noted in review): "Microsoft Sans Serif" is a sans face, and so is the CSS generic. + if lower.contains("serif") { + return Some(false); + } None } diff --git a/crates/fonts/src/fontdb/tests/fallback.rs b/crates/fonts/src/fontdb/tests/fallback.rs index d385a8fd9..d9a5ee71c 100644 --- a/crates/fonts/src/fontdb/tests/fallback.rs +++ b/crates/fonts/src/fontdb/tests/fallback.rs @@ -176,6 +176,26 @@ fn a_korean_family_name_says_sans_or_serif() { } } +/// "sans serif" ends with the word it is not. The bare "serif" needle matched first, so +/// "Microsoft Sans Serif" -- and the CSS generic -- classified as serif and a missing Hangul +/// glyph took the serif-first chain (noted in review of #1692). +#[test] +fn a_sans_serif_name_is_not_read_as_serif() { + for sans in ["sans-serif", "Microsoft Sans Serif", "MS Sans Serif", "Lucida Sans Serif"] { + assert_eq!(sans_by_name(sans), Some(true), "{sans} is sans"); + } + // The bare word still decides a name that holds nothing else. + for serif in ["serif", "Source Serif 4", "DejaVu Serif", "Noto Serif KR"] { + assert_eq!(sans_by_name(serif), Some(false), "{serif} is serif"); + } + // And a Korean serif name still outranks "Sans", which is why it is tested first of all. + assert_eq!(sans_by_name("Noto Sans Myungjo"), Some(false)); + assert_eq!(sans_by_name("Myungjo Gothic"), Some(false)); + // Neither word present, so the caller still decides. + assert_eq!(sans_by_name("monospace"), None); + assert_eq!(sans_by_name("system-ui"), None); +} + /// The reported bug is the order of the chain: on Windows the first installed entry of the /// serif-first chain is Batang, so every missing Dotum face came out serif (#1692). #[test]