googlefonts / googlefonts/glyphsLib

default "gasp Table" custom parameter (empty "") crashes conversion

Open
#944 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
201
Forks
56
Avg merge
17m
Merged PRs (30d)
1

Description

There are a few issues with the "gasp Table" custom parameter.

First of all, the table name is lowecase, at least in the Glyphs.app version I'm currently looking at which is 3.2 (3217).
Was it ever called "GASP Table" all uppercase in the past? If so we might have to support both. /cc @schriftgestalt

Or are the custom parameter names treated as case-insensitive?

Secondly, if the parameter is set from Glyphs.app with default values, it is serialised to .glyphs file as an empty "" string, not a dict as glyphsLib expects it... So we get an AttributeError crash by the time we attempt to convert to UFO:

```
File "/Users/clupo/.local/pipx/venvs/fontmake/lib/python3.10/site-packages/glyphsLib/builder/custom_params.py", line 528, in to_ufo_gasp_table
value = {int(k): int(v) for k, v in value.items()}
AttributeError: 'str' object has no attribute 'items'
```

Finally, if the "gasp Table" is set to non-default values, it is written out by Glyphs.app as a dict in the .glyphs source, e.g.:

```
{
name = "gasp Table";
value = {
9 = 10;
21 = 7;
65535 = 15;
};
```

~~however glyphsLib seems to completely ignore it, I cannot find any trace in the generated UFOs.~~ (CORRECTION: it does go to fontinfo.plist)

We have some unit-tests in custom_params_test.py that deal with "GASP Table" but they do not run end-to-end and may be missing something.

Oh and one more thing, which definitely looks suspiciously wrong is this line:

https://github.com/googlefonts/glyphsLib/blob/8c0157e4fba7a97fc9fbd4e4b532bcf92796c06f/Lib/glyphsLib/classes.py#L1380

should it not be `frozenset(("GASP Table",))` (plus perhaps the lowercased name)? otherwise one gets `frozenset({' ', 'A', 'G', 'P', 'S', 'T', 'a', 'b', 'e', 'l'})`

Contributor guide

Open the contributing guide

Research direction

Start with Lib/glyphsLib/custom_params.py, especially to_ufo_gasp_table, and inspect the related handling in Lib/glyphsLib/classes.py. Run the existing custom_params_test.py tests and add coverage for the lowercase "gasp Table" name and the empty-string default; done means conversion no longer crashes and the parameter is handled consistently.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.