diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d2d173..93ea6be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `shaping` subcommand and extra (#36). - `fontwerk` extra (#37). - `notofonts` extra (#37). +- `com.thetypefounders/check/features_default_languagesystem`: Checks if a default languagesystem statement is present in feature files and warns if the compiler will not insert one automatically (https://github.com/fonttools/fontbakery/issues/4011) ### Changed @@ -31,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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 (#38). - `com.google.fonts/check/STAT_in_statics`: The check now skips fonts that do not have a `STAT` table (#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 (#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). ### Fixed @@ -38,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `com.google.fonts/check/fontvalidator`: ERROR caused by attempting to run FontValidator before checking if it's installed (#30). - `com.google.fonts/check/mandatory_glyphs`: Improved the check's resilience to edge cases that could result in ERRORs (#38). - `-L`/`--list-checks` option that can be used with subcommands. Previously this option only worked if a path to an input file was also provided in the command line (#35). +- Summary statistics on HTML reporter (https://github.com/fonttools/fontbakery/issues/3997). ## [0.1.0] - 2023-06-11 diff --git a/Lib/openbakery/constants.py b/Lib/openbakery/constants.py index f5c0c14..91a4a57 100644 --- a/Lib/openbakery/constants.py +++ b/Lib/openbakery/constants.py @@ -575,7 +575,6 @@ class WindowsLanguageID(enum.IntEnum): "Hangul Syllables": 56, "CJK Unified Ideographs": 59, "CJK Strokes": 61, - "Yi Syllables": 83, } @@ -602,8 +601,6 @@ class WindowsLanguageID(enum.IntEnum): [0x31C0, 0x31EF], # CJK Strokes [0xF900, 0xFAFF], # CJK Compatibility Ideographs (CJK Strokes) [0x2F800, 0x2FA1F], # CJK Compatibility Ideographs Supplement (CJK Strokes) - [0xA000, 0xA48F], # Yi Syllables - [0xA490, 0xA4CF], # Yi Radicals ] OFL_BODY_TEXT = """ diff --git a/Lib/openbakery/profiles/fontwerk.py b/Lib/openbakery/profiles/fontwerk.py index 486c61b..8c5b33f 100644 --- a/Lib/openbakery/profiles/fontwerk.py +++ b/Lib/openbakery/profiles/fontwerk.py @@ -39,6 +39,8 @@ def leave_this_one_out(checkid): "com.google.fonts/check/version_bump", "com.google.fonts/check/production_glyphs_similarity", "com.google.fonts/check/name/line_breaks", + "com.google.fonts/check/fontdata_namecheck", + "com.google.fonts/check/meta/script_lang_tags", # The following check they may need some improvements # before we decide to include it: "com.google.fonts/check/family/italics_have_roman_counterparts", diff --git a/Lib/openbakery/profiles/googlefonts.py b/Lib/openbakery/profiles/googlefonts.py index a8f5cb0..8cb8de5 100644 --- a/Lib/openbakery/profiles/googlefonts.py +++ b/Lib/openbakery/profiles/googlefonts.py @@ -5650,9 +5650,9 @@ def com_google_fonts_check_cjk_vertical_metrics_regressions( @check( id="com.google.fonts/check/cjk_not_enough_glyphs", - conditions=["is_cjk_font"], + conditions=["is_claiming_to_be_cjk_font"], rationale=""" - Hangul has 40 characters and it's the smallest CJK writing system. + Kana has 150 characters and it's the smallest CJK writing system. If a font contains less CJK glyphs than this writing system, we inform the user that some glyphs may be encoded incorrectly. @@ -5660,12 +5660,12 @@ def com_google_fonts_check_cjk_vertical_metrics_regressions( proposal="https://github.com/googlefonts/fontbakery/pull/3214", ) def com_google_fonts_check_cjk_not_enough_glyphs(ttFont): - """Does the font contain less than 40 CJK characters?""" + """Does the font contain less than 150 CJK characters?""" from .shared_conditions import get_cjk_glyphs cjk_glyphs = get_cjk_glyphs(ttFont) cjk_glyph_count = len(cjk_glyphs) - if cjk_glyph_count > 0 and cjk_glyph_count < 40: + if cjk_glyph_count > 0 and cjk_glyph_count < 150: if cjk_glyph_count == 1: N_CJK_glyphs = "There is only one CJK glyph" else: @@ -5673,8 +5673,8 @@ def com_google_fonts_check_cjk_not_enough_glyphs(ttFont): yield WARN, Message( "cjk-not-enough-glyphs", - f"{N_CJK_glyphs} when there needs to be at least 40" - f" in order to support the smallest CJK writing system, Hangul.\n" + f"{N_CJK_glyphs} when there needs to be at least 150" + f" in order to support the smallest CJK writing system, Kana.\n" f"The following CJK glyphs were found:\n" f"{cjk_glyphs}\n" f"Please check that these glyphs have the correct unicodes.", diff --git a/Lib/openbakery/profiles/os2.py b/Lib/openbakery/profiles/os2.py index d05e295..0f63af8 100644 --- a/Lib/openbakery/profiles/os2.py +++ b/Lib/openbakery/profiles/os2.py @@ -35,7 +35,7 @@ def com_google_fonts_check_family_panose_proportion(ttFonts): ) if not passed: - yield FAIL, Message( + yield WARN, Message( "inconsistency", "PANOSE proportion is not the same across this family." " In order to fix this, please make sure that" @@ -71,7 +71,7 @@ def com_google_fonts_check_family_panose_familytype(ttFonts): ) if not passed: - yield FAIL, Message( + yield WARN, Message( "inconsistency", "PANOSE family type is not the same across this family." " In order to fix this, please make sure that" diff --git a/Lib/openbakery/profiles/shared_conditions.py b/Lib/openbakery/profiles/shared_conditions.py index e3a5559..f7e7bc5 100644 --- a/Lib/openbakery/profiles/shared_conditions.py +++ b/Lib/openbakery/profiles/shared_conditions.py @@ -503,18 +503,21 @@ def unicoderange(ttFont): @condition -def is_cjk_font(ttFont): +def is_claiming_to_be_cjk_font(ttFont): """Test font object to confirm that it meets our definition of a CJK font file. - The definition is met if any of the following conditions are True: + We do this in two ways: in some cases, we are testing the *metadata*, + i.e. what the font claims about itself, in which case the definition is + met if any of the following conditions are True: + 1. The font has a CJK code page bit set in the OS/2 table 2. The font has a CJK Unicode range bit set in the OS/2 table - 3. The font has any CJK Unicode code points defined in the cmap table + + See below for another way of testing this. """ from openbakery.constants import ( CJK_CODEPAGE_BITS, CJK_UNICODE_RANGE_BITS, - CJK_UNICODE_RANGES, ) if not has_os2_table(ttFont): @@ -541,17 +544,23 @@ def is_cjk_font(ttFont): if os2.ulUnicodeRange3 & (1 << (bit - 64)): return True - # defined CJK Unicode code point in cmap table checks - cmap = ttFont.getBestCmap() - for unicode_range in CJK_UNICODE_RANGES: - for x in range(unicode_range[0], unicode_range[1] + 1): - if int(x) in cmap: - return True - # default, return False if the above checks did not identify a CJK font return False +@condition +def is_cjk_font(ttFont): + """ + The `is_claiming_to_be_cjk_font` condition looks up the font's metadata to see if + it is claiming to be a CJK font. But the metadata may be wrong, and the correctness + of the metadata is something what we want to check! + We also want to know if the font really is a CJK font, i.e. it contains a + significant number of CJK characters. We say that *this* definition is met if the + font has more than 150 CJK Unicode code points defined in the cmap table. + """ + return len(get_cjk_glyphs(ttFont)) > 150 + + @condition def get_cjk_glyphs(ttFont): """Return all glyphs which belong to a CJK unicode block""" diff --git a/Lib/openbakery/profiles/ufo_sources.py b/Lib/openbakery/profiles/ufo_sources.py index 8bd364f..97f8b99 100644 --- a/Lib/openbakery/profiles/ufo_sources.py +++ b/Lib/openbakery/profiles/ufo_sources.py @@ -1,10 +1,13 @@ +import re + from openbakery.callable import check, condition -from openbakery.status import ERROR, FAIL, PASS, WARN +from openbakery.status import ERROR, FAIL, PASS, SKIP, WARN from openbakery.section import Section from openbakery.message import Message from openbakery.fonts_profile import profile_factory from openbakery.utils import exit_with_install_instructions + profile = profile_factory(default_section=Section("UFO Sources")) UFO_PROFILE_CHECKS = [ @@ -16,6 +19,7 @@ "com.google.fonts/check/designspace_has_default_master", "com.google.fonts/check/designspace_has_consistent_glyphset", "com.google.fonts/check/designspace_has_consistent_codepoints", + "com.thetypefounders/check/features_default_languagesystem", ] @@ -303,5 +307,48 @@ def com_google_fonts_check_designspace_has_consistent_codepoints(designSpace, co yield PASS, "Unicode assignments were consistent." +@check( + id="com.thetypefounders/check/features_default_languagesystem", + conditions=["ufo_font"], + rationale=""" + The feature file specification strongly recommends to use a + `languagesystem DFLT dflt` statement in your feature file. This + statement is automatically inserted when no `languagesystem` + statements are present in the feature file, *unless* there is + another `languagesystem` statement already present. If this is + the case, this behaviour could lead to unintended side effects. + + This check only WARNs when this happen as there are cases where + not having a `languagesystem DFLT dflt` statement in your feature + file is technically correct. + + http://adobe-type-tools.github.io/afdko/OpenTypeFeatureFileSpecification.html#4b-language-system + """, + proposal="https://github.com/googlefonts/fontbakery/issues/4011", +) +def com_thetypefounders_check_features_default_languagesystem(ufo_font): + """Check that languagesystem DFLT dflt is present in the features.fea file.""" + + if ufo_font.features.text is None: + yield SKIP, "No features.fea file in font." + elif not ufo_font.features.text.strip(): + yield PASS, "Default languagesystem inserted by compiler." + else: + tags = re.findall( + # pylint: disable-next=line-too-long + r"languagesystem\s+([A-Za-z0-9\._!$%&*+:?^'|~]{1,4})\s+([A-Za-z0-9\._!$%&*+:?^'|~]{1,4})", # noqa E501 + ufo_font.features.text, + ) + + if len(tags) > 0 and ("DFLT", "dflt") != tags[0]: + tags_str = ", ".join([" ".join(t) for t in tags]) + yield WARN, Message( + "default-languagesystem", + f"Default languagesystem not found in: {tags_str}.", + ) + else: + yield PASS, "Default languagesystem present or automatically inserted." + + profile.auto_register(globals()) profile.test_expected_checks(UFO_PROFILE_CHECKS, exclusive=True) diff --git a/Lib/openbakery/reporters/html.py b/Lib/openbakery/reporters/html.py index c92faa9..920eb0a 100644 --- a/Lib/openbakery/reporters/html.py +++ b/Lib/openbakery/reporters/html.py @@ -43,12 +43,11 @@ def get_html(self) -> str: section_stati_of_note = ( e for e in section["result"].elements() if e != "PASS" ) - if all([self.omit_loglevel(s) for s in section["result"].elements()]): - continue section_stati = "".join( EMOTICON[s] for s in sorted(section_stati_of_note, key=LOGLEVELS.index) ) - body_elements.append(f"

{section_name} {section_stati}

") + if not all([self.omit_loglevel(s) for s in section["result"].elements()]): + body_elements.append(f"

{section_name} {section_stati}

") checks_by_id: Dict[str, List[Dict[str, str]]] = collections.defaultdict( list diff --git a/tests/profiles/googlefonts_test.py b/tests/profiles/googlefonts_test.py index cd5ada0..095bd5d 100644 --- a/tests/profiles/googlefonts_test.py +++ b/tests/profiles/googlefonts_test.py @@ -4171,12 +4171,15 @@ def test_check_cjk_not_enough_glyphs(): ttFont = TTFont(TEST_FILE("montserrat/Montserrat-Regular.ttf")) msg = assert_results_contain(check(ttFont), SKIP, "unfulfilled-conditions") - assert msg == "Unfulfilled Conditions: is_cjk_font" + assert msg == "Unfulfilled Conditions: is_claiming_to_be_cjk_font" # Let's modify Montserrat's cmap so there's a cjk glyph cmap = ttFont["cmap"].getcmap(3, 1) # Add first character of the CJK unified Ideographs cmap.cmap[0x4E00] = "A" + # And let's declare that we are a CJK font + ttFont["OS/2"].ulCodePageRange1 |= 1 << 17 + msg = assert_results_contain(check(ttFont), WARN, "cjk-not-enough-glyphs") assert msg.startswith("There is only one CJK glyph") diff --git a/tests/profiles/os2_test.py b/tests/profiles/os2_test.py index 6bf64ba..3706f34 100644 --- a/tests/profiles/os2_test.py +++ b/tests/profiles/os2_test.py @@ -60,7 +60,7 @@ def test_check_family_panose_proportion(mada_ttFonts): mada_ttFonts[0]["OS/2"].panose.bProportion = incorrect_value assert_results_contain( - check(mada_ttFonts), FAIL, "inconsistency", "with inconsistent family." + check(mada_ttFonts), WARN, "inconsistency", "with inconsistent family." ) @@ -78,7 +78,7 @@ def test_check_family_panose_familytype(mada_ttFonts): mada_ttFonts[0]["OS/2"].panose.bFamilyType = incorrect_value assert_results_contain( - check(mada_ttFonts), FAIL, "inconsistency", "with inconsistent family." + check(mada_ttFonts), WARN, "inconsistency", "with inconsistent family." ) diff --git a/tests/profiles/ufo_sources_test.py b/tests/profiles/ufo_sources_test.py index fc681e1..725acc6 100644 --- a/tests/profiles/ufo_sources_test.py +++ b/tests/profiles/ufo_sources_test.py @@ -8,6 +8,7 @@ from openbakery.codetesting import ( assert_PASS, assert_results_contain, + assert_SKIP, CheckTester, TEST_FILE, ) @@ -171,3 +172,73 @@ def test_check_designspace_has_consistent_codepoints(): assert_results_contain(check(designspace), FAIL, "inconsistent-codepoints") # TODO: Fix it and ensure it passes the check + + +def test_check_default_languagesystem_pass_without_features(empty_ufo_font): + """Pass if the UFO source has no features.""" + check = CheckTester( + ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem" + ) + + ufo, _ = empty_ufo_font + + assert_SKIP(check(ufo), "No features.fea file in font.") + + +def test_check_default_languagesystem_pass_with_empty_features(empty_ufo_font): + """Pass if the UFO source has a feature file but it is empty.""" + check = CheckTester( + ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem" + ) + + ufo, _ = empty_ufo_font + + ufo.features.text = "" + + assert_PASS(check(ufo)) + + +def test_check_default_languagesystem_pass_with_features(empty_ufo_font): + """Pass if the font has features and no default languagesystem statements.""" + check = CheckTester( + ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem" + ) + + ufo, _ = empty_ufo_font + + ufo.features.text = "feature liga { sub f i by f_i; } liga;" + + assert_PASS(check(ufo)) + + +def test_check_default_languagesystem_warn_without_default_languagesystem( + empty_ufo_font, +): + """Warn if `languagesystem DFLT dflt` is not present in the feature file, + but other languagesystem statements are.""" + check = CheckTester( + ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem" + ) + + ufo, _ = empty_ufo_font + + ufo.features.text = ( + "languagesystem latn dflt; feature liga { sub f i by f_i; } liga;" + ) + + assert_results_contain(check(ufo), WARN, "default-languagesystem") + + +def test_check_default_languagesystem_pass_with_default_languagesystem(empty_ufo_font): + """Pass if `languagesystem DFLT dflt` is explicitly used in the features.""" + check = CheckTester( + ufo_sources_profile, "com.thetypefounders/check/features_default_languagesystem" + ) + + ufo, _ = empty_ufo_font + + ufo.features.text = """languagesystem DFLT dflt; +languagesystem latn dflt; +feature liga { sub f i by f_i; } liga;""" + + assert_PASS(check(ufo))