[family_and_style_max_length] Modernize check

[arrowtype]

Description

Relates to issue #2179, specifically later comments and testing, like #2179 (comment)

Changes:

• Change main static font check to consider NameID 1 (FONT_FAMILY_NAME) rather than NameID 4 (FULL_FONT_NAME)
• Change VF check to test NameID 16 + STAT particles
• Consider removing test of NameID 6 ... I ended up leaving this, but noting in its warning message that the issue is probably only with pre-Mac OS X systems.
• update tests to pass with new check criteria

Checklist

• update CHANGELOG.md
• wait for the tests to pass
• request a review

Process notes

Questions to answer

• Is the maximum acceptable length 32, or 31? The previous check tested for 32 or fewer, but testing showed definitively that the requirement is 31 or fewer characters.
• Does nameID 4 really matter? No, not in my testing.
• Does a long nameID 6 really "cause issues with PostScript printers, especially on Mac platforms"? What is the basis for this check? Neither of the listed issues seem to say anything about this. This could be a later adjustment.

Question for @felipesanches

I’ve left the prior check for VF family name + fvar entry, but I’m not quite sure whether it’s actually useful. This may depend on a question: does FontBakery “allow” VFs that don’t have STAT tables, or is there a common check for that? It looks like there is:

fontbakery/Lib/fontbakery/checks/required_tables.py

Lines 87 to 90 in 49dcd6a

	if is_variable_font:
	# According to https://github.com/fonttools/fontbakery/issues/1671
	# STAT table is required on WebKit on MacOS 10.12 for variable fonts.
	REQUIRED_TABLES.append("STAT")

So, the required_tables check fails if a VF doen’t include a STAT table, and this check is part of the Universal profile. And, it does look like the Universal profile is included in almost all of the other profiles, either directly or via googlefonts/adobefonts profiles. Does that mean it’s fairly safe to assume that we shouldn’t bother with checking VFs that lack a STAT table, or is it better to keep the old fvar condition, anyway?

Process screenshots

Static fonts

MS Word, Windows 11:

• When nameID 1 is "Proxima Nova Thai Looped Extrabold" Thai glyphs appear in fallback font ("Proxima Nova Thai Looped Light", in this case).
• If nameID 1 is "Proxima Nova Thai Looped Extrabo" (32 characters) it also fails
• "Proxima Nova Thai Looped Blaack" (31 characters) works.
• "Proxima Nova Thai Looped Extrab" (31 characters) works.
• All of these test fonts had names 4 & 6 that are longer than 32 characters, so those don't seem to make a difference.
• The results are the same between OTF and TTF fonts.

Variable fonts

Works:

• NameID 16: Proxima Nova Thai Loop Var
  • STAT style: Blck (Roman is flagged as elidable, and therefore, not counted), 31 total characters
  • STAT style: Thin, 31 total characters

Fails:

• NameID 16: Proxima Nova Thai Loop Var
  • STAT style: Extrabold (Roman is flagged as elidable, and therefore, not counted), 36 total characters
  • STAT style: Light, 32 total characters
• No STAT table: only the base Variable instance appears, but without a style name (so, perhaps it is pulling NameID 16, but not even getting fvar instances? This is unclear.)

[aaronbell]

Some of your questions are answered here: google/fonts#9185

I did research into the family name length problem and documented my findings there.

[felipesanches]

@arrowtype, I'll review this soon. I just did not get to it yet.

In the mean-time, I'm curious to hear from @simoncozens about his views on these changes, as it will likely be something to be updated on Fontspector as well, since that's gradually turning into our preferred QA tool.

side-note: I think that Fontbakery's python codebase can still be useful as this prototyping stage both for new checks as well as for updating checking criteria. But users are strongly encouraged to try Fontspector, as it is more at the leading-edge of QA tooling.

[arrowtype]

Thanks, @felipesanches! All good, and thanks for the quick comment.

I think I’ve done what I can, so far. There is still a test failing, but it’s for a file that isn’t related to the changes made in this PR (so far as I can tell).

The failing test is from tests/test_checks_unique_glyphnames.py, which had its latest change in November 2024.

FAILED tests/test_checks_unique_glyphnames.py::test_check_unique_glyphnames[unique_glyphnames] - Exception: Expected to find <Status FAIL>, [code: duplicated-glyph-names]
But did not find it in:
[Subresult(status=<Status PASS>, message=Glyph names are all unique.)]
= 1 failed, 892 passed, 2 skipped, 2 xfailed, 1 xpassed, 1663 warnings in 152.63s (0:02:32) =

[arrowtype]

Also, that sounds like a good plan, prototyping checks here, then moving them into Fontspector. That system is blazing fast, so it will be great to use on larger projects, especially. Also, having that step be almost instant will help projects of any size.

[arrowtype]

Made a small refactor of the check based on a review from a colleague.

Also merged the CHANGELOG, and made a couple of typos, so I had to revise that commit and force push.

[arrowtype]

Thanks for approving these changes, @RamsesAupart!

I don’t have authorization to push to this branch... is there anything needed from me, before we merge this?

[arrowtype]

I almost hesitated to add anything else here, as I worry it could further delay this update. However, I’ve run into a small issue in my updated check, and it was easily fixed:

The earlier state of this check would falsely fail on a variable + STAT name like “Longish Family Name Regular Italic” (34 characters), but MS Word doesn’t actually exhibit its name length issue in this style, because it elides STAT names “Regular” and “Italic” (even if they aren’t marked as elidable in the STAT table).

So, my latest commit doesn’t count those STAT name particles towards the length limit. I’ve tested to verify that it does still appropriately fail on other long STAT names, and everything still seems to be working well.

[arrowtype]

Reading through the issue again – especially this comment — and constantly hitting warnings for long postscript names, I think maybe this PR really should remove the NameID 6 check.

I also want to double-check that nameID 4 doesn’t come into play. I don’t think it does, but I don’t see a record of this being tested. UPDATE: I’ve tested it here. The static checks in this PR are still solid.

So, planning to update this once more.

[arrowtype]

Okay, I’ve removed a note about nameID 21 (WWS family name) in variable fonts, after additional testing.

[arrowtype]

The automated checks are currently failing, but I don’t think it’s due to anything in my changes. It seems like a dependency issue? I imagine someone else would be better equipped to handle this, so I won’t attempt a fix unless I hear otherwise.

Error log from the automated GitHub workflow checks:

Building wheels for collected packages: fontbakery
  Building wheel for fontbakery (pyproject.toml): started
  Building wheel for fontbakery (pyproject.toml): finished with status 'done'
  Created wheel for fontbakery: filename=fontbakery-1.0.2.dev28+g3a2c1653e-py3-none-any.whl size=563315 sha256=0afd62a9b183653825f56d8217f623b0731908f2f723cb0d2bee3f24d41f0d43
  Stored in directory: C:\Users\runneradmin\AppData\Local\Temp\pip-ephem-wheel-cache-qr5c74ql\wheels\96\d6\7f\f9365dda37d667bd3b0b04674b40bc2d8536c12c865d7a2220
Successfully built fontbakery
Installing collected packages: sre-yield, tabulate, soupsieve, skia-pathops, shaperglot, rstr, protobuf, platformdirs, orjson, openstep-plist, lxml, kurbopy, importlib-resources, filelock, colorama, attrs, youseedee, ufoLib2, tqdm, stringbrewer, gfsubsets, gflanguages, beautifulsoup4, axisregistry, ufomerge, glyphsLib, fontbakery, glyphsets, babelfont, collidoscope
ERROR: Exception:
Traceback (most recent call last):
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\cli\base_command.py", line 107, in _run_wrapper
    status = _inner_run()
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\cli\base_command.py", line 98, in _inner_run
    return self.run(options, args)
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\cli\req_command.py", line 71, in wrapper
    return func(self, options, args)
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\commands\install.py", line 460, in run
    installed = install_given_reqs(
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\req\__init__.py", line 85, in install_given_reqs
    requirement.install(
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\req\req_install.py", line 870, in install
    install_wheel(
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\operations\install\wheel.py", line 737, in install_wheel
    _install_wheel(
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\operations\install\wheel.py", line 595, in _install_wheel
    file.save()
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\pip\_internal\operations\install\wheel.py", line 372, in save
    shutil.copyfileobj(f, dest, blocksize)
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\shutil.py", line 195, in copyfileobj
    buf = fsrc_read(length)
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\zipfile.py", line 927, in read
    data = self._read1(n)
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\zipfile.py", line 1003, in _read1
    data = self._decompressor.decompress(data, n)
zlib.error: Error -3 while decompressing data: invalid distance too far back

Error: Process completed with exit code 1.

[felipesanches]

Fontbakery is closing doors. We have just moved all "New Check Proposals" to the Fontspector issue tracker. All future check proposals must be made directly there. I am not putting effort to implement new checks in python, but it is still fine (for a short while) to receive PRs like this one here.

@simoncozens Could you plese take a look at this PR? I believe this PR should be (1) reviewed to make sure it is correct and (2) ported to Fontspector.

If you approve this on Fontspector, I'll also merge it here in Fontbakery for a final release.

If anyone submits a PR with a Python check implementation to Fontbakery, I'll be sure to submit a corresponding check proposal issue on Fontspector.

[felipesanches]

Work on this will continue at PR #5049