Skip to content
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

Fix: Handle Undefined Axis Property Error #317

Closed
wants to merge 2 commits into from
Closed

Fix: Handle Undefined Axis Property Error #317

wants to merge 2 commits into from

Conversation

thierryc
Copy link

@thierryc thierryc commented Aug 8, 2023

Fixed the problem of undefined axis.name.en

fontkit/dist/module.mjs:12699
name: axis.name.en,
                ^
TypeError: Cannot read properties of undefined (reading 'en')

Property by implementing axis tag trimming when the value is not readable.

Related to #297 #272

Font in error exemple :

https://github.com/clauseggers/Playfair/blob/master/fonts/VF-TTF/PlayfairItalicVF.ttf

output:


{
  opsz: { name: 'Optical size', min: 5, default: 1200, max: 1200 },
  wdth: { name: 'Width', min: 88, default: 88, max: 113 },
  wght: { name: 'Weight', min: 300, default: 300, max: 900 },
  ital: { name: 'ital', min: 1, default: 1, max: 1 }
}

use instance.postscriptNameID as fallback name
for namedVariations when the instance.name.en is not available
@Typogram
Copy link

Typogram commented Sep 20, 2023

I don't think this is the right solution. This PR basically set the problemed instance name to the axisTag. This bug happens when the instance name happen to be the subFamilyName, not when it doesn't exist. It should retrieve the correct name from the name table, before fall back to axisTag as the last resort.
Fallback to axisTag as last resort is a nice add-on though!

output:

{
  ...
  ital: { name: 'ital', min: 1, default: 1, max: 1 }
}

the name should be Italic not ital, it should be the subFamilyName

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