Skip to content

[family_and_style_max_length] Modernize check #5020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

arrowtype
Copy link
Contributor

@arrowtype arrowtype commented May 7, 2025

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:

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.

image

image

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.)

image

image

@aaronbell
Copy link
Contributor

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
Copy link
Collaborator

felipesanches commented May 14, 2025

@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 arrowtype marked this pull request as ready for review May 14, 2025 20:01
@arrowtype
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants