diff --git a/profile-opentype/src/checks/opentype/fsselection.rs b/profile-opentype/src/checks/opentype/fsselection.rs index d0a51392..f1f6312a 100644 --- a/profile-opentype/src/checks/opentype/fsselection.rs +++ b/profile-opentype/src/checks/opentype/fsselection.rs @@ -1,7 +1,11 @@ use fontations::{ - skrifa::raw::{ - tables::{head::MacStyle, os2::SelectionFlags}, - TableProvider, + skrifa::{ + raw::{ + tables::{head::MacStyle, os2::SelectionFlags}, + TableProvider, + }, + string::StringId, + MetadataProvider, }, write::from_obj::ToOwnedTable, }; @@ -24,6 +28,7 @@ use fontspector_checkapi::{prelude::*, testfont, FileTypeConvert}; the bold and italic bits in head.macStyle per the OpenType spec. ", proposal = "https://github.com/fonttools/fontbakery/issues/4829", // legacy check + proposal = "https://github.com/fonttools/fontspector/issues/577", hotfix = fix_fsselection, )] fn fsselection(f: &Testable, _context: &Context) -> CheckFnResult { @@ -66,6 +71,22 @@ fn fsselection(f: &Testable, _context: &Context) -> CheckFnResult { )); } } + + let wws_seen = fs_flags.contains(SelectionFlags::WWS); + let has_name_id_21 = font + .font() + .localized_strings(StringId::WWS_FAMILY_NAME) + .english_or_first(); + let has_name_id_22 = font + .font() + .localized_strings(StringId::WWS_SUBFAMILY_NAME) + .english_or_first(); + if has_name_id_21.is_none() && has_name_id_22.is_none() && !wws_seen { + problems.push(Status::warn( + "bad-fsSelection-wws-bit", + "If no name id 21 and 22 (WWS Family Name/WWS Subfamily Name), 'OS/2' fsSelection flag for WWS should be set.", + )); + } return_result(problems) } @@ -92,3 +113,32 @@ fn fix_fsselection(t: &mut Testable) -> FixFnResult { t.set(f.rebuild_with_new_table(&os2)?); Ok(true) } + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used)] + + use fontspector_checkapi::{ + codetesting::{ + assert_messages_contain, assert_results_contain, run_check_with_config, test_able, + }, + StatusCode, TestableType, + }; + use std::collections::HashMap; + + #[test] + fn test_fsselection() { + let testable = test_able("mada/Mada-Regular.ttf"); + let results = run_check_with_config( + super::fsselection, + TestableType::Single(&testable), + HashMap::new(), + ); + assert_messages_contain(&results, "fsSelection flag for WWS should be set."); + assert_results_contain( + &results, + StatusCode::Warn, + Some("bad-fsSelection-wws-bit".to_string()), + ); + } +} diff --git a/profile-universal/src/checks/fsselection_wws.rs b/profile-universal/src/checks/fsselection_wws.rs new file mode 100644 index 00000000..62eee245 --- /dev/null +++ b/profile-universal/src/checks/fsselection_wws.rs @@ -0,0 +1,76 @@ +use fontations::skrifa::{ + raw::{tables::os2::SelectionFlags, TableProvider}, + string::StringId, + MetadataProvider, +}; +use fontspector_checkapi::{prelude::*, skip, testfont, FileTypeConvert}; + +// NOTE: This check is not part of OpenType profile, because we do not check +// if the names are consistent with weight/width/slope – therefore we cannot +// be 100% sure if WWS should be set or not. +// It's based on best practices -> therefore in Universal profile. + +#[check( + id = "universal/fsselection_wws", + title = "Checking OS/2 fsSelection WWS bit.", + rationale = " + According to the opentype spec fsSelection bit 8 should be set if: + + The font has 'name' table strings consistent with a + weight/width/slope family without requiring use of name IDs 21 and 22. + ", + proposal = "https://github.com/fonttools/fontspector/issues/577" +)] +fn fsselection_wws(f: &Testable, _context: &Context) -> CheckFnResult { + let font = testfont!(f); + skip!(!font.has_table(b"name"), "no-name", "No name table."); + + let mut problems = vec![]; + + let fs_flags = font.font().os2()?.fs_selection(); + let wws_seen = fs_flags.contains(SelectionFlags::WWS); + let has_name_id_21 = font + .font() + .localized_strings(StringId::WWS_FAMILY_NAME) + .english_or_first(); + let has_name_id_22 = font + .font() + .localized_strings(StringId::WWS_SUBFAMILY_NAME) + .english_or_first(); + if has_name_id_21.is_none() && has_name_id_22.is_none() && !wws_seen { + problems.push(Status::warn( + "bad-fsSelection-wws-bit", + "If the font has 'name' table strings consistent with a weight/width/slope family without requiring use of name IDs 21 and 22, 'OS/2' fsSelection flag for WWS should be set.", + )); + } + return_result(problems) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used)] + + use fontspector_checkapi::{ + codetesting::{ + assert_messages_contain, assert_results_contain, run_check_with_config, test_able, + }, + StatusCode, TestableType, + }; + use std::collections::HashMap; + + #[test] + fn test_fsselection_wws() { + let testable = test_able("mada/Mada-Regular.ttf"); + let results = run_check_with_config( + super::fsselection_wws, + TestableType::Single(&testable), + HashMap::new(), + ); + assert_messages_contain(&results, "fsSelection flag for WWS should be set."); + assert_results_contain( + &results, + StatusCode::Warn, + Some("bad-fsSelection-wws-bit".to_string()), + ); + } +} diff --git a/profile-universal/src/checks/mod.rs b/profile-universal/src/checks/mod.rs index c378d149..528a60ae 100644 --- a/profile-universal/src/checks/mod.rs +++ b/profile-universal/src/checks/mod.rs @@ -25,6 +25,7 @@ mod file_size; mod fontdata_namecheck; #[cfg(not(target_family = "wasm"))] mod freetype_rasterizer; +mod fsselection_wws; mod fvar_name_entries; mod gpos7; mod gpos_kerning_info; @@ -96,6 +97,7 @@ pub use file_size::file_size; pub use fontdata_namecheck::fontdata_namecheck; #[cfg(not(target_family = "wasm"))] pub use freetype_rasterizer::freetype_rasterizer; +pub use fsselection_wws::fsselection_wws; pub use fvar_name_entries::fvar_name_entries; pub use gpos7::gpos7; pub use gpos_kerning_info::gpos_kerning_info; diff --git a/profile-universal/src/lib.rs b/profile-universal/src/lib.rs index 37c99f42..9c4d6736 100644 --- a/profile-universal/src/lib.rs +++ b/profile-universal/src/lib.rs @@ -98,6 +98,7 @@ impl fontspector_checkapi::Plugin for Universal { .add_and_register_check(checks::has_glyphs) .add_and_register_check(checks::has_unicodes) .add_and_register_check(checks::required_name_ids) + .add_and_register_check(checks::fsselection_wws) .build("universal", cr) // Checks which don't make sense any more