Skip to content

Fix typographic family name#5012

Merged
felipesanches merged 5 commits intofonttools:mainfrom
guidoferreyra:fix-typographic_family_name
Apr 1, 2025
Merged

Fix typographic family name#5012
felipesanches merged 5 commits intofonttools:mainfrom
guidoferreyra:fix-typographic_family_name

Conversation

@guidoferreyra
Copy link
Copy Markdown
Contributor

@guidoferreyra guidoferreyra commented Mar 27, 2025

Description

When checking a typical family with RIBBI and non-RIBBI styles fonts can have or not ID 16.
In the current state of the check, is ignoring this case adding the empty value to the list of family names.
This is creating a false positive in most of the families I check. From my point of view, the check should try to get ID 16 and if is not present get ID 1 and check if there is more than one.

🔥 FAIL
Name ID 16 (Typographic Family name) is not consistent across fonts. Values found: ['', 'MyFamily']

The test for this check was incomplete so I did an attempt to complete it, please take look and let me know if it’s good.

Checklist

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

@guidoferreyra
Copy link
Copy Markdown
Contributor Author

@felipesanches could you please review?

@felipesanches
Copy link
Copy Markdown
Collaborator

I'm not sure about this one. @simoncozens can you please take a look?

@guidoferreyra, in the mean time, could you please add an entry to CHANGELOG.md describing this?

@simoncozens
Copy link
Copy Markdown
Collaborator

I think this is right. We want both RIBBI and non-RIBBI fonts to work. Initially there was only RIBBI and to support non-RIBBI fonts in applications that only knew about RIBBI fonts, the workaround was to put family name + non-RIBBI component into name ID 1 and then put the "real" family name in name ID 16. But RIBBI fonts don't need the workaround and so don't have anything in name ID 16, they just have the real family name in ID 1. So yeah, look at 16 if it's non-RIBBI and if there's nothing there, assume it's RIBBI and look in 1. Horrible hack but the name table is a horrible hack these days...

@guidoferreyra
Copy link
Copy Markdown
Contributor Author

Just added a note to the changelog. Is the test good enough? This was the first time I wrote a test for a check 🤣 please take a look.

@felipesanches
Copy link
Copy Markdown
Collaborator

It is still unclear to me what's going wrong on some of the CI jobs on Windows:

Screenshot From 2025-04-01 12-16-22

Screenshot From 2025-04-01 13-20-02

@guidoferreyra
Copy link
Copy Markdown
Contributor Author

It is still unclear to me what's going wrong on some of the CI jobs on Windows:

It only failed in today‘s commit 🤷‍♂️

@simoncozens
Copy link
Copy Markdown
Collaborator

It's not fontbakery related, I'm seeing it on other CIs that use pip install.

@felipesanches felipesanches merged commit 5e5b8f6 into fonttools:main Apr 1, 2025
44 of 49 checks passed
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