fonttools / fontbakery

🧁 A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
550 stars 102 forks source link

com.google.fonts/check/font_version - FAIL message prints version number incorrectly #2403

Closed thundernixon closed 5 years ago

thundernixon commented 5 years ago

This seems to be distinct from the earlier issue of parsing in check 044.

Observed behavior

Check 044 seems to be incorrectly parsing versions numbers incorrectly.

🔥 FAIL: Checking font version fields (head and name table).

com.google.fonts/check/044 🔥 FAIL head version is ('3', '004'), name version string for platform 3, encoding 1, is ('3', '400') [code: mismatch]

When the font is TTXed, the specific code looks like the following.

Head table:

<fontRevision value="3.004"/>

Name table:

<namerecord nameID="5" platformID="3" platEncID="1" langID="0x409">
      3.4;862d1a785
</namerecord>

The problem

3.4 from the name table is incorrectly reported as 3.400 by the fontbakery output.

Expected behavior

I do think that the fail happens to be true in this case (though I'm not 100% sure on that), however the fail message is inaccurate.

It reports name version string for platform 3, encoding 1, is ('3', '400') when it should say name version string for platform 3, encoding 1, is ('3', '4').

Resources and exact process needed to replicate

More details at: https://github.com/rsms/inter/issues/138

GH markdown check with fail: https://github.com/thundernixon/inter/blob/862d1a78572ce58dd6a85e8215235dac79ef09fe/misc/googlefonts-qa/checks/Inter-Regular.checks.md

Exact version and instructions to reproduce, if desired: https://github.com/thundernixon/inter/tree/862d1a78572ce58dd6a85e8215235dac79ef09fe/misc/googlefonts-qa

felipesanches commented 5 years ago

I cloned the Inter repo and tried to build the font binaries, but got a problem when setting up the buid environment:

Screenshot at 2019-03-22 19:03:15

@thundernixon, could you please send me a copy of the binaries so that it is easier for me to replicate the fontbakery issue?

thundernixon commented 5 years ago

bin.zip

Note: this goes into build/venv

image

felipesanches commented 5 years ago

no, sorry... What I meant was a copy of the font binaries, meaning the pre-built OpenType font files that result from running these build scripts.

felipesanches commented 5 years ago

I used the term "binary" in opposition to "source" just like in typical software dev speak

thundernixon commented 5 years ago

Ohhh, haha, my bad.

Here's the upright variable font binary, at that exact state:

Inter-upright.var.ttf.zip

I just ran:

 fontbakery check-googlefonts <font_path>/Inter-upright.var.ttf

and once again, got:

>> com.google.fonts/check/044 with (('font[0]', 'build/fonts/var/Inter-upright.var.ttf'),)
   Checking font version fields (head and name table).
 * FAIL: head version is ('3', '004'), name version string for platform 3, encoding 1, is ('3', '400') [code: mismatch]

   Result: FAIL

In terms of Inter, this is low-priority. Rasmus made a padding hack on his end, so this fail is no longer triggered. However, this seems probably worth fixing for other fonts.

felipesanches commented 5 years ago

Screenshot at 2019-03-22 21:32:00