From a0067a4e19e5f650231a5751802b6e639f0cea26 Mon Sep 17 00:00:00 2001 From: Josh Hadley Date: Fri, 6 Oct 2023 11:32:20 -0700 Subject: [PATCH 1/5] Split postscript name checks Separate the "disallowed characters" and "multiple hyphen" checks. --- Lib/openbakery/profiles/adobefonts.py | 5 ++- Lib/openbakery/profiles/name.py | 61 +++++++++++++++------------ Lib/openbakery/profiles/opentype.py | 3 +- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/Lib/openbakery/profiles/adobefonts.py b/Lib/openbakery/profiles/adobefonts.py index 2a7182b..09b4646 100644 --- a/Lib/openbakery/profiles/adobefonts.py +++ b/Lib/openbakery/profiles/adobefonts.py @@ -26,7 +26,7 @@ SET_EXPLICIT_CHECKS = { # This is the set of explict checks that will be invoked # when openbakery is run with the 'adobefonts' subcommand. - # The contents of this set were last updated on September 6, 2023. + # The contents of this set were last updated on October 6, 2023. # # ======================================= # From adobefonts.py (this file) @@ -132,9 +132,10 @@ "com.adobe.fonts/check/family/max_4_fonts_per_family_name", "com.adobe.fonts/check/family/consistent_family_name", "com.adobe.fonts/check/name/empty_records", - "com.adobe.fonts/check/postscript_name", "com.adobe.fonts/check/name/postscript_name_consistency", "com.adobe.fonts/check/name/postscript_vs_cff", + "com.adobe.fonts/check/postscript_name_characters", + "com.adobe.fonts/check/postscript_name_hyphens", "com.google.fonts/check/family_naming_recommendations", "com.google.fonts/check/monospace", # diff --git a/Lib/openbakery/profiles/name.py b/Lib/openbakery/profiles/name.py index 3b606a7..590b202 100644 --- a/Lib/openbakery/profiles/name.py +++ b/Lib/openbakery/profiles/name.py @@ -419,48 +419,55 @@ def com_google_fonts_check_name_match_familyname_fullfont(ttFont): @check( - id="com.adobe.fonts/check/postscript_name", + id="com.adobe.fonts/check/postscript_name_characters", proposal="https://github.com/miguelsousa/openbakery/issues/62", ) -def com_adobe_fonts_check_postscript_name(ttFont): - """PostScript name follows OpenType specification requirements?""" +def com_adobe_fonts_check_postscript_name_characters(ttFont): + """PostScript name contains only allowed characters a-ZA-Z and hyphen?""" import re from openbakery.utils import get_name_entry_strings - bad_entries = [] + bad_entry_count = 0 # may contain only a-zA-Z0-9 - # and one hyphen bad_psname = re.compile("[^A-Za-z0-9-]") for string in get_name_entry_strings(ttFont, NameID.POSTSCRIPT_NAME): if bad_psname.search(string): - bad_entries.append( - { - "field": "PostScript Name", - "value": string, - "rec": ("May contain only a-zA-Z0-9 characters and a hyphen."), - } + bad_entry_count += 1 + yield FAIL, Message( + "bad-psname-characters", + f"PostScript name {string} contains disallowed characters.", ) + + if bad_entry_count == 0: + yield PASS, Message( + "psname-chracters-ok", "PostScript name contains only allowed characters." + ) + + +@check( + id="com.adobe.fonts/check/postscript_name_hyphens", + proposal="https://github.com/miguelsousa/openbakery/issues/62", +) +def com_adobe_fonts_check_postscript_name_hyphens(ttFont): + """PostScript name contains at most one hyphen?""" + from openbakery.utils import get_name_entry_strings + + bad_entry_count = 0 + + # may contain only one hyphen + for string in get_name_entry_strings(ttFont, NameID.POSTSCRIPT_NAME): if string.count("-") > 1: - bad_entries.append( - { - "field": "Postscript Name", - "value": string, - "rec": ("May contain not more than a single hyphen."), - } + bad_entry_count += 1 + yield FAIL, Message( + "bad-psname-hyphens", + f"PostScript name {string} contains more than one hyphen.", ) - if len(bad_entries) > 0: - table = "| Field | Value | Recommendation |\n" - table += "|:----- |:----- |:-------------- |\n" - for bad in bad_entries: - table += "| {} | {} | {} |\n".format(bad["field"], bad["value"], bad["rec"]) - yield FAIL, Message( - "bad-psname-entries", - f"PostScript name does not follow requirements:\n\n{table}", + if bad_entry_count == 0: + yield PASS, Message( + "psname-hyphens-ok", "PostScript name contains at most one hyphen." ) - else: - yield PASS, Message("psname-ok", "PostScript name follows requirements.") @check( diff --git a/Lib/openbakery/profiles/opentype.py b/Lib/openbakery/profiles/opentype.py index 849cc19..24839c4 100644 --- a/Lib/openbakery/profiles/opentype.py +++ b/Lib/openbakery/profiles/opentype.py @@ -70,7 +70,8 @@ "com.google.fonts/check/glyf_unused_data", "com.google.fonts/check/family_naming_recommendations", "com.google.fonts/check/maxadvancewidth", - "com.adobe.fonts/check/postscript_name", + "com.adobe.fonts/check/postscript_name_characters", + "com.adobe.fonts/check/postscript_name_hyphens", "com.google.fonts/check/points_out_of_bounds", "com.google.fonts/check/glyf_non_transformed_duplicate_components", "com.google.fonts/check/code_pages", From 341d4b0d706f644c28a8558542c168806dcfa3f6 Mon Sep 17 00:00:00 2001 From: Josh Hadley Date: Fri, 6 Oct 2023 11:32:34 -0700 Subject: [PATCH 2/5] Update tests for split psname checks --- tests/profiles/adobefonts_test.py | 2 +- tests/profiles/name_test.py | 36 ++++++++++++++++++++----------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/profiles/adobefonts_test.py b/tests/profiles/adobefonts_test.py index 494db79..e7e1091 100644 --- a/tests/profiles/adobefonts_test.py +++ b/tests/profiles/adobefonts_test.py @@ -61,7 +61,7 @@ def test_get_family_checks(): def test_profile_check_set(): """Confirm that the profile has the correct number of checks and the correct set of check IDs.""" - assert len(SET_EXPLICIT_CHECKS) == 81 + assert len(SET_EXPLICIT_CHECKS) == 82 explicit_with_overrides = sorted( f"{check_id}{OVERRIDE_SUFFIX}" if check_id in OVERRIDDEN_CHECKS else check_id for check_id in SET_EXPLICIT_CHECKS diff --git a/tests/profiles/name_test.py b/tests/profiles/name_test.py index 65411a9..b891583 100644 --- a/tests/profiles/name_test.py +++ b/tests/profiles/name_test.py @@ -648,15 +648,17 @@ def set_name(font, nameID, string): assert_results_contain(check(ttFont), FAIL, "bad-typographicsubfamilyname") -def test_check_name_postscript(): - check = CheckTester(opentype_profile, "com.adobe.fonts/check/postscript_name") +def test_check_name_postscript_characters(): + check = CheckTester( + opentype_profile, "com.adobe.fonts/check/postscript_name_characters" + ) - # Test a font that has OK psname. Check should PASS. + # Test a font that has psname with allowed characters. Check should PASS. ttFont = TTFont(TEST_FILE("source-sans-pro/OTF/SourceSansPro-Bold.otf")) assert_PASS(check(ttFont), "psname-ok") - # Change the PostScript name string to more than one hyphen. Should FAIL. - bad_ps_name = "more-than-one-hyphen".encode("utf-16-be") + # Change it to a string with disallowed characters. Should FAIL. + bad_ps_name = "(disallowed) characters".encode("utf-16-be") ttFont["name"].setName( bad_ps_name, NameID.POSTSCRIPT_NAME, @@ -664,12 +666,21 @@ def test_check_name_postscript(): WindowsEncodingID.UNICODE_BMP, WindowsLanguageID.ENGLISH_USA, ) - msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-entries") - assert "PostScript name does not follow requirements" in msg - assert "May contain not more than a single hyphen." in msg + msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-characters") + assert "name (disallowed) characters contains disallowed characters." in msg + - # Now change it to a string with illegal characters. Should FAIL. - bad_ps_name = "(illegal) characters".encode("utf-16-be") +def test_check_name_postscript_hyphens(): + check = CheckTester( + opentype_profile, "com.adobe.fonts/check/postscript_name_hyphens" + ) + + # Test a font that has OK psname. Check should PASS. + ttFont = TTFont(TEST_FILE("source-sans-pro/OTF/SourceSansPro-Bold.otf")) + assert_PASS(check(ttFont), "psname-ok") + + # Change the PostScript name string to more than one hyphen. Should FAIL. + bad_ps_name = "more-than-one-hyphen".encode("utf-16-be") ttFont["name"].setName( bad_ps_name, NameID.POSTSCRIPT_NAME, @@ -677,6 +688,5 @@ def test_check_name_postscript(): WindowsEncodingID.UNICODE_BMP, WindowsLanguageID.ENGLISH_USA, ) - msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-entries") - assert "PostScript name does not follow requirements" in msg - assert "May contain only a-zA-Z0-9 characters and a hyphen." in msg + msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-hyphens") + assert "PostScript name more-than-one-hyphen contains more than one hyphen." in msg From 6eacd47840573767cc6d5e18628bb1890732d839 Mon Sep 17 00:00:00 2001 From: Josh Hadley Date: Fri, 6 Oct 2023 11:32:40 -0700 Subject: [PATCH 3/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a671bb8..dcd467a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `com.google.fonts/check/valid_glyphnames`: The check now takes into account that OpenType-CFF2 fonts with `post` table format 3 contain no glyph names, and will yield SKIP (https://github.com/miguelsousa/openbakery/pull/38). - `com.google.fonts/check/unique_glyphnames`: The check now takes into account that OpenType-CFF2 fonts with `post` table format 3 contain no glyph names, and will yield SKIP (https://github.com/miguelsousa/openbakery/pull/38). - `com.google.fonts/check/STAT_in_statics`: The check now skips fonts that do not have a `STAT` table (https://github.com/miguelsousa/openbakery/pull/38). -- `com.google.fonts/check/family_naming_recommendations`: Two validations of PostScript name were moved out of this check and into `com.adobe.fonts/check/postscript_name` which yields FAIL (https://github.com/miguelsousa/openbakery/pull/62). +- `com.google.fonts/check/family_naming_recommendations`: Two validations of PostScript name were moved out of this check and into separate new checks `com.adobe.fonts/check/postscript_name_characters` and `com.adobe.fonts/postscript_name_hyphens` which yield FAIL (https://github.com/miguelsousa/openbakery/pull/62). - `com.google.fonts/check/cjk_not_enough_glyphs`: This check is now only run when a font has CJK codepages or ranges declared in the `OS/2` table. Other CJK-related checks are run on fonts with a minimum of 150 CJK glyphs (https://github.com/fonttools/fontbakery/issues/3846). - `com.google.fonts/check/family/panose_familytype` and `com.google.fonts/check/family/panose_proportion`: Failures have been downgraded to warnings (https://github.com/fonttools/fontbakery/issues/4192). From f30e7478757baffe701a0c9f3f06e9747d28b2f8 Mon Sep 17 00:00:00 2001 From: Josh Hadley Date: Sat, 7 Oct 2023 17:41:39 -0700 Subject: [PATCH 4/5] Update name.py fix typo in PASS message --- Lib/openbakery/profiles/name.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/openbakery/profiles/name.py b/Lib/openbakery/profiles/name.py index 590b202..71b5016 100644 --- a/Lib/openbakery/profiles/name.py +++ b/Lib/openbakery/profiles/name.py @@ -441,7 +441,7 @@ def com_adobe_fonts_check_postscript_name_characters(ttFont): if bad_entry_count == 0: yield PASS, Message( - "psname-chracters-ok", "PostScript name contains only allowed characters." + "psname-characters-ok", "PostScript name contains only allowed characters." ) From f5a4e7a376a73a61c410503a99f7d7910d09be6c Mon Sep 17 00:00:00 2001 From: Josh Hadley Date: Sat, 7 Oct 2023 17:42:39 -0700 Subject: [PATCH 5/5] Update name_test.py change assert_PASS to assert_results_contain and check for correct msg code (which assert_PASS apparently does not do). --- tests/profiles/name_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/profiles/name_test.py b/tests/profiles/name_test.py index b891583..3ecdc3b 100644 --- a/tests/profiles/name_test.py +++ b/tests/profiles/name_test.py @@ -655,7 +655,7 @@ def test_check_name_postscript_characters(): # Test a font that has psname with allowed characters. Check should PASS. ttFont = TTFont(TEST_FILE("source-sans-pro/OTF/SourceSansPro-Bold.otf")) - assert_PASS(check(ttFont), "psname-ok") + assert_results_contain(check(ttFont), PASS, "psname-characters-ok") # Change it to a string with disallowed characters. Should FAIL. bad_ps_name = "(disallowed) characters".encode("utf-16-be") @@ -677,7 +677,7 @@ def test_check_name_postscript_hyphens(): # Test a font that has OK psname. Check should PASS. ttFont = TTFont(TEST_FILE("source-sans-pro/OTF/SourceSansPro-Bold.otf")) - assert_PASS(check(ttFont), "psname-ok") + assert_results_contain(check(ttFont), PASS, "psname-hyphens-ok") # Change the PostScript name string to more than one hyphen. Should FAIL. bad_ps_name = "more-than-one-hyphen".encode("utf-16-be")