Better max_length check

CHANGELOG.md

@@ -3,7 +3,9 @@ A more detailed list of changes is available in the corresponding milestones for
 
 
 ## Upcoming release: 0.10.7 (2023-Dec-??)
-  - ...
+### Changes to existing checks
+#### On the Google Fonts Profile
+  - **[com.google.fonts/check/name/family_and_style_max_length]:** This test has been extensively reworked, thanks to feedback from @vv-monsale, @moontypespace and @RosaWagner (issue #4316, issue #4104)
 
 ### Changes to existing checks
 

Lib/fontbakery/profiles/googlefonts.py

@@ -17,6 +17,7 @@
 from fontbakery.message import Message, KEEP_ORIGINAL_MESSAGE
 from fontbakery.fonts_profile import profile_factory
 from fontbakery.constants import (
+    RIBBI_STYLE_NAMES,
     NameID,
     PlatformID,
     WindowsEncodingID,
@@ -2615,8 +2616,6 @@ def com_google_fonts_check_metadata_valid_name_values(
     style, font_metadata, font_familynames, typographic_familynames
 ):
     """METADATA.pb font.name field contains font name in right format?"""
-    from fontbakery.constants import RIBBI_STYLE_NAMES
-
     if style in RIBBI_STYLE_NAMES:
         familynames = font_familynames
     else:
@@ -2642,8 +2641,6 @@ def com_google_fonts_check_metadata_valid_full_name_values(
     style, font_metadata, font_familynames, typographic_familynames
 ):
     """METADATA.pb font.full_name field contains font name in right format?"""
-    from fontbakery.constants import RIBBI_STYLE_NAMES
-
     if style in RIBBI_STYLE_NAMES:
         familynames = font_familynames
         if familynames == []:
@@ -3951,7 +3948,6 @@ def com_google_fonts_check_metadata_nameid_copyright(ttFont, font_metadata):
 def com_google_fonts_check_name_mandatory_entries(ttFont, style):
     """Font has all mandatory 'name' table entries?"""
     from fontbakery.utils import get_name_entry_strings
-    from fontbakery.constants import RIBBI_STYLE_NAMES
 
     required_nameIDs = [
         NameID.FONT_FAMILY_NAME,
@@ -4906,52 +4902,73 @@ def ligatures_sequences(pairs):
 @check(
     id="com.google.fonts/check/name/family_and_style_max_length",
     rationale="""
-        According to a GlyphsApp tutorial [1], in order to make sure all versions of
-        Windows recognize it as a valid font file, we must make sure that the
-        concatenated length of the familyname (NameID.FONT_FAMILY_NAME) and
-        style (NameID.FONT_SUBFAMILY_NAME) strings in the name table do not
-        exceed 20 characters.
-
-        After discussing the problem in more detail at FontBakery issue #2179 [2] we
-        decided that allowing up to 27 chars would still be on the safe side, though.
+        This check ensures that the length of name table entries is not
+        too long, as this causes problems in some environments. For
+        details as to the latest requirements and the reason for them,
+        please see https://github.com/fonttools/fontbakery/issues/2179
 
-        [1] https://glyphsapp.com/tutorials/multiple-masters-part-3-setting-up-instances
-        [2] https://github.com/fonttools/fontbakery/issues/2179
     """,
     proposal="https://github.com/fonttools/fontbakery/issues/1488",
     misc_metadata={
-        # Somebody with access to Windows should make some experiments
-        # and confirm that this is really the case.
         "affects": [("Windows", "unspecified")],
     },
 )
 def com_google_fonts_check_name_family_and_style_max_length(ttFont):
-    """Combined length of family and style must not exceed 27 characters."""
-    from fontbakery.utils import get_name_entries, get_name_entry_strings
+    """Combined length of family and style must not exceed 31 characters."""
+    from fontbakery.utils import get_name_entry_strings
+    import re
+
+    ribbi_re = " (" + "|".join(RIBBI_STYLE_NAMES) + ")$"
 
     passed = True
-    for familyname in get_name_entries(ttFont, NameID.FONT_FAMILY_NAME):
-        # we'll only match family/style name entries with the same platform ID:
-        plat = familyname.platformID
-        familyname_str = familyname.string.decode(familyname.getEncoding())
-        for stylename_str in get_name_entry_strings(
-            ttFont, NameID.FONT_SUBFAMILY_NAME, platformID=plat
-        ):
-            if len(familyname_str + stylename_str) > 27:
+
+    def strip_ribbi(x):
+        return re.sub(ribbi_re, "", x)
+
+    checks = [
+        [
+            NameID.FULL_FONT_NAME,
+            31,
+            "with the dropdown menu in old versions of Microsoft Word",
+            strip_ribbi,
+        ],
+        [
+            NameID.POSTSCRIPT_NAME,
+            27,
+            "with PostScript printers, especially on Mac platforms",
+            lambda x: x,
+        ],
+    ]
+    for nameid, maxlen, reason, transform in checks:
+        for the_name in get_name_entry_strings(ttFont, nameid):
+            the_name = transform(the_name)
+            if len(the_name) > maxlen:
                 passed = False
                 yield WARN, Message(
-                    "too-long",
-                    f"The combined length of family and style"
-                    f" exceeds 27 chars in the following"
-                    f" '{PlatformID(plat).name}' entries:\n"
-                    f" FONT_FAMILY_NAME = '{familyname_str}' /"
-                    f" SUBFAMILY_NAME = '{stylename_str}'\n"
-                    f"\n"
-                    f"Please take a look at the conversation at"
-                    f" https://github.com/fonttools/fontbakery/issues/2179"
-                    f" in order to understand the reasoning behind these"
-                    f" name table records max-length criteria.",
+                    f"nameid{nameid}-too-long",
+                    f"Name ID {nameid} '{the_name}' exceeds"
+                    f" {maxlen} characters. This has been found to"
+                    f" cause problems {reason}",
                 )
+
+    # name ID 1 + fvar instance name > 31 : WARN : problems with Windows
+    if "fvar" in ttFont:
+        for instance in ttFont["fvar"].instances:
+            for instance_name in get_name_entry_strings(
+                ttFont, instance.subfamilyNameID
+            ):
+                for family_name in get_name_entry_strings(
+                    ttFont, NameID.FONT_FAMILY_NAME
+                ):
+                    full_instance_name = instance_name + " " + family_name
+                    if len(full_instance_name) > 31:
+                        yield WARN, Message(
+                            "instance-too-long",
+                            f"Variable font instance name {full_instance_name}"
+                            " exceeds 31 characters. This has been found to "
+                            " cause problems in Microsoft Windows 11",
+                        )
+
     if passed:
         yield PASS, "All name entries are good."
 

tests/profiles/googlefonts_test.py

@@ -409,43 +409,50 @@ def test_check_description_eof_linebreak():
 
 
 def test_check_name_family_and_style_max_length():
-    """Combined length of family and style must not exceed 27 characters."""
+    """Name table entries should not be too long."""
     check = CheckTester(
         googlefonts_profile, "com.google.fonts/check/name/family_and_style_max_length"
     )
 
     # Our reference Cabin Regular is known to be good
-    ttFont = TTFont(TEST_FILE("cabin/Cabin-Regular.ttf"))
+    ttFont = TTFont(TEST_FILE("cabinvf/Cabin[wdth,wght].ttf"))
 
     # So it must PASS the check:
     assert_PASS(check(ttFont), "with a good font...")
 
     # Then we emit a WARNing with long family/style names
-    # Originaly these were based on the example on the glyphs tutorial
-    # (at https://glyphsapp.com/tutorials/multiple-masters-part-3-setting-up-instances)
-    # but later we increased a bit the max allowed length.
+    # See https://github.com/fonttools/fontbakery/issues/2179 for
+    # a discussion of the requirements
 
-    # First we expect a WARN with a bad FAMILY NAME
     for index, name in enumerate(ttFont["name"].names):
-        if name.nameID == NameID.FONT_FAMILY_NAME:
-            # This has 28 chars, while the max currently allowed is 27.
-            bad = "AnAbsurdlyLongFamilyNameFont"
-            assert len(bad) == 28
+        if name.nameID == NameID.FULL_FONT_NAME:
+            # This has 33 chars, while the max currently allowed is 31
+            bad = "An Absurdly Long Family Name Font"
+            assert len(bad) == 33
+            ttFont["name"].names[index].string = bad.encode(name.getEncoding())
+        if name.nameID == NameID.POSTSCRIPT_NAME:
+            bad = "AnAbsurdlyLongFontName-Regular"
+            assert len(bad) == 30
             ttFont["name"].names[index].string = bad.encode(name.getEncoding())
-            break
-    assert_results_contain(check(ttFont), WARN, "too-long", "with a bad font...")
 
-    # Now let's restore the good Cabin Regular...
-    ttFont = TTFont(TEST_FILE("cabin/Cabin-Regular.ttf"))
+    results = check(ttFont)
+    assert_results_contain(results, WARN, "nameid4-too-long", "with a bad font...")
+    assert_results_contain(results, WARN, "nameid6-too-long", "with a bad font...")
 
-    # ...and break the check again with a bad SUBFAMILY NAME:
+    # Restore the original VF
+    ttFont = TTFont(TEST_FILE("cabinvf/Cabin[wdth,wght].ttf"))
+
+    # ...and break the check again with a bad fvar instance name:
+    nameid_to_break = ttFont["fvar"].instances[0].subfamilyNameID
     for index, name in enumerate(ttFont["name"].names):
-        if name.nameID == NameID.FONT_SUBFAMILY_NAME:
+        if name.nameID == nameid_to_break:
             bad = "WithAVeryLongAndBadStyleName"
             assert len(bad) == 28
             ttFont["name"].names[index].string = bad.encode(name.getEncoding())
             break
-    assert_results_contain(check(ttFont), WARN, "too-long", "with a bad font...")
+    assert_results_contain(
+        check(ttFont), WARN, "instance-too-long", "with a bad font..."
+    )
 
 
 def DISABLED_test_check_glyphs_file_name_family_and_style_max_length():