Update check/colorfont_tables

to match the color-font requirements from the Google Fonts Guide at
https://googlefonts.github.io/gf-guide/color.html
(issue #4131)

CHANGELOG.md

@@ -25,6 +25,7 @@ A more detailed list of changes is available in the corresponding milestones for
   - **[com.google.fonts/check/metadata/broken_links]:** We should not keep email addresses on METADATA.pb files (issue #4110)
   - **[com.google.fonts/check/description/broken_links]:** We should not keep email addresses on DESCRIPTION files (issue #4110)
   - **[com.google.fonts/check/metadata/nameid/font_name]:** Use name id 16, when present, to compute the expected font family name (issue #4086)
+  - **[com.google.fonts/check/check/colorfont_tables]:** Update checking criteria according to gf-guide (issue #4131)
 
 #### On the Universal Profile
   - **[com.google.fonts/check/soft_hyphen]:** Improve wording of the rationale. (issue #4095)

Lib/fontbakery/profiles/googlefonts.py

@@ -6395,43 +6395,66 @@ def com_google_fonts_check_metadata_category_hint(family_metadata):
 @check(
     id = "com.google.fonts/check/colorfont_tables",
     rationale = """
-        Colr v0 fonts are widely supported in most browsers so they do not require
-        an SVG color table. However, Colr v1 is only well supported in Chrome so
-        we need to add an SVG table to these fonts.
-
-        To add an SVG table, run the maximum_color tool in Nano Emoji,
+        COLR v0 fonts are widely supported in most browsers so they do not require
+        an SVG color table. However, some environments (e.g. Safari, Adobe apps)
+        do not currently support COLR v1 so we need to add an SVG table to these fonts,
+        except in the case of variable fonts, since SVG does not support
+        OpenType Variations.
+
+        To automatically generate compatible SVG/COLR tables,
+        run the maximum_color tool in nanoemoji:
         https://github.com/googlefonts/nanoemoji
     """,
-    proposal = 'https://github.com/googlefonts/fontbakery/issues/3886'
+    proposal = ['https://googlefonts.github.io/gf-guide/color.html',
+                'https://github.com/googlefonts/fontbakery/issues/3886',
+                'https://github.com/googlefonts/fontbakery/issues/3888',
+                'https://github.com/googlefonts/fontbakery/pull/3889',
+                'https://github.com/googlefonts/fontbakery/issues/4131']
 )
 def com_google_fonts_check_colorfont_tables(ttFont):
-    """Check font has the expected color font tables"""
+    """Check font has the expected color font tables."""
+    from .shared_conditions import is_variable_font
+
+    passed = True
+    NANOEMOJI_ADVICE = ("You can do it by using the maximum_color tool provided by"
+                        " the nanoemoji project:\n"
+                        "https://github.com/googlefonts/nanoemoji")
+
     if "COLR" in ttFont:
-        colr_table = ttFont["COLR"]
-        if colr_table.version == 0 and "SVG " in ttFont:
-            yield FAIL, Message(
-                "drop-svg",
-                "Font has a COLR v0 table, which is already widely supported, "
-                "so the SVG table isn't needed."
-            )
-            return
-        elif colr_table.version == 1 and "SVG " not in ttFont:
-            yield FAIL, Message(
-                "add-svg",
-                "Font has COLRv1 but no SVG table; for CORLv1, we require "
-                "that an SVG table is present to support environments where "
-                "the former is not supported yet."
-            )
-            return
-    elif "SVG " in ttFont:
+        if ttFont["COLR"].version == 0 and "SVG " in ttFont:
+            passed = False
+            yield FAIL,\
+                  Message("drop-svg",
+                          "Font has a COLR v0 table, which is already widely supported,"
+                          " so the SVG table isn't needed.")
+
+        elif ttFont["COLR"].version == 1 and "SVG " not in ttFont\
+             and not is_variable_font(ttFont):
+            passed = False
+            yield FAIL,\
+                  Message("add-svg",
+                          "Font has COLRv1 but no SVG table; for CORLv1, we require"
+                          " that an SVG table is present to support environments where"
+                          " the former is not supported yet.\n" + NANOEMOJI_ADVICE)
+
+    if "SVG " in ttFont:
+        if is_variable_font(ttFont):
+            passed = False
+            yield FAIL,\
+                  Message("variable-svg",
+                          "This is a variable font and SVG does not support"
+                          " OpenType Variations.\n"
+                          "Please remove the SVG table from this font.")
+
         if "COLR" not in ttFont:
-            yield FAIL, Message(
-                "add-colr",
-                "Font only has an SVG table. Please add a COLR table as well. "
-            )
-            return
-    yield PASS, "Looks Good!"
+            passed = False
+            yield FAIL,\
+                  Message("add-colr",
+                          "Font only has an SVG table."
+                          " Please add a COLR table as well.\n" + NANOEMOJI_ADVICE)
 
+    if passed:
+        yield PASS, "Looks Good!"
 
 
 @check(

tests/profiles/googlefonts_test.py

@@ -4328,26 +4328,51 @@ def test_check_override_freetype_rasterizer(mock_import_error):
 
 
 def test_check_colorfont_tables():
-    """Fonts must have neither or both the tables 'COLR' and 'SVG'."""
+    """Check font has the expected color font tables."""
     from fontTools.ttLib import newTable
+    from fontbakery.profiles.shared_conditions import is_variable_font
+
     check = CheckTester(googlefonts_profile,
                         "com.google.fonts/check/colorfont_tables")
 
     ttFont = TTFont(TEST_FILE("color_fonts/noto-glyf_colr_1.ttf"))
     assert 'SVG ' not in ttFont.keys()
     assert 'COLR' in ttFont.keys()
-    # Check colr v1 font has an svg table. Will fail since font
-    # doesn't have one.
+    assert ttFont["COLR"].version == 1
+    # Check colr v1 static font has an svg table (since v1 is not yet broadly supported).
+    # Will fail since font doesn't have one.
     assert_results_contain(check(ttFont),
                            FAIL, 'add-svg',
-                           'with a color font lacking SVG table')
+                           'with a static colr v1 font lacking SVG table')
+
+    # Fake a variable font by adding an fvar table.
+    ttFont["fvar"] = newTable('fvar')
+    assert 'fvar' in ttFont.keys()
+    assert is_variable_font(ttFont)
+
+    # SVG does not support OpenType Variations
+    assert_PASS(check(ttFont),
+                'with a variable color font without SVG table')
 
     # Fake an SVG table:
     ttFont["SVG "] = newTable('SVG ')
+    assert 'SVG ' in ttFont.keys()
+
+    assert_results_contain(check(ttFont),
+                           FAIL, 'variable-svg',
+                           'with a variable color font with SVG table')
+
+    # Make it a static again:
+    del ttFont["fvar"]
+    assert 'fvar' not in ttFont.keys()
+    assert is_variable_font(ttFont) == False
+
+
     assert 'SVG ' in ttFont.keys()
     assert 'COLR' in ttFont.keys()
+    assert ttFont["COLR"].version == 1
     assert_PASS(check(ttFont),
-                f'with a font containing both tables.')
+                'with a static colr v1 font containing both tables.')
 
     # Now downgrade to colr table to v0:
     ttFont["COLR"].version = 0
@@ -4364,7 +4389,7 @@ def test_check_colorfont_tables():
                            FAIL, 'add-colr',
                            'with a font which should have a COLR table')
 
-    #finally delete both color font tables
+    # Finally delete both color font tables
     del ttFont["SVG "]
     assert 'SVG ' not in ttFont.keys()
     assert 'COLR' not in ttFont.keys()