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 ERROR on ligatures conditional #1596

Closed
felipesanches opened this issue Sep 30, 2017 · 2 comments
Closed

Fix ERROR on ligatures conditional #1596

felipesanches opened this issue Sep 30, 2017 · 2 comments
Assignees
Labels

Comments

@felipesanches
Copy link
Collaborator

in a few rare occasions, we get an ERROR on the ligatures conditional (which is required in tests such as com.google.fonts/test/064, the caret positioning test). This happens on only 7 of the 2400 TTFs in Google Fonts collection:

screenshot at 2017-09-29 21 27 53

The actual error is this:
screenshot at 2017-09-29 23 01 57

The condition implementation fails at line 2291:
https://github.com/googlefonts/fontbakery/blob/527dda3e1e5b3364512852a6d63dd7745273a6cc/Lib/fontbakery/specifications/googlefonts.py#L2287-L2300

Interestingly, when trying to use ttx to inspect the file contents I get a similar error, which makes me believe this is a case of corrupted TTF files:
screenshot at 2017-09-29 23 01 33

This may also be a fontTools bug. Can you confirm that, @behdad ?

@felipesanches felipesanches self-assigned this Sep 30, 2017
@felipesanches felipesanches added this to the 0.3.3 - Future minor release milestone Sep 30, 2017
@felipesanches
Copy link
Collaborator Author

felipesanches commented Sep 30, 2017

Some example of offending files from the Google Fonts collection are:

  • ofl/bhavuka/Bhavuka-Regular.ttf
  • ofl/seoulhangang/SeoulHangang-Medium.ttf
  • ofl/seoulhangang/SeoulHangang-Light.ttf

@davelab6
Copy link
Contributor

Given the errors happen directly when TTX'ing the fonts, I propose to update the checker runner so that if fontTools fails to unpack a font, that is a FAIL of the check, not an ERROR, because its not a problem with our code, its a problem with the font that this check has uncovered.

I see in the runner's output the string, unpack requires - perhaps you can find that string in the error and if found, return a FAIL?

felipesanches added a commit to felipesanches/fontbakery that referenced this issue Oct 16, 2017
felipesanches added a commit to felipesanches/fontbakery that referenced this issue Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants