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

Line-breaks in Name Table (move this check to googlefonts profile) #2778

Closed
ClintGoss opened this issue Mar 5, 2020 · 15 comments
Closed

Line-breaks in Name Table (move this check to googlefonts profile) #2778

ClintGoss opened this issue Mar 5, 2020 · 15 comments
Assignees
Milestone

Comments

@ClintGoss
Copy link

@ClintGoss ClintGoss commented Mar 5, 2020

Observed behaviour

Font Bakery complains about name table entries with line-breaks. Could you explain, or point me to, the rationale for this?

Expected behaviour

Line-breaks in the license description field - for example the text of the OFL - seem (to me) to be significant. The OFL without line breaks appears (to me) rather ugly. And might stripping line-breaks possibly change the semantics of the license?

Resources and exact process needed to replicate

@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 5, 2020

Could you please tell us the check-id of the specific check that is giving you this WARN/FAIL message?

I suppose this must be in the googlefonts profile and it means we want the license description field to be very short and simple, instead of the full license text

@ClintGoss
Copy link
Author

@ClintGoss ClintGoss commented Mar 5, 2020

fontbakery check-universal ...

raises errors such as:

  • FAIL: Name entry LICENSE_DESCRIPTION on platform UNICODE contains a line-break. [code: line-break]
@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 6, 2020

hmmm... interesting!
This is check-id com.google.fonts/check/name/line_breaks on the universal profile, indeed!

I think we should move this into the googlefonts profile as I see how this is not necessarily an industry-wide well accepted good-practice, but instead a requirement we decided to have for the Google Fonts collection. Thanks for reporting this ;-)

@felipesanches felipesanches added this to the 0.7.20 milestone Mar 6, 2020
@felipesanches felipesanches self-assigned this Mar 6, 2020
@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 6, 2020

oh! It is even worse than that! It is currently on the opentype profile, which is certainly a wrong place for it. Excuse us for that mistake! This is definitely moving to the googlefonts profile right now! :-|

@felipesanches felipesanches changed the title Line-breaks in Name Table Line-breaks in Name Table (move this check to googlefonts profile) Mar 6, 2020
@felipesanches felipesanches modified the milestones: 0.7.20, 0.7.21 Mar 6, 2020
felipesanches added a commit to felipesanches/fontbakery that referenced this issue Mar 6, 2020
as it is a vendor-specific policy rather than an opentype spec requirement.
(issue googlefonts#2778)
felipesanches added a commit that referenced this issue Mar 6, 2020
as it is a vendor-specific policy rather than an opentype spec requirement.
(issue #2778)
@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 6, 2020

Done. This change will be included in the upcoming release, FontBakery v0.7.21 which should be published tomorrow during the day.

@ClintGoss
Copy link
Author

@ClintGoss ClintGoss commented Mar 6, 2020

Thanks so much for the shockingly fast action on this ...

@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 6, 2020

:-D

@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 7, 2020

@ClintGoss, here it is: https://pypi.org/project/fontbakery/0.7.21/

update with pip install --upgrade fontbakery

Be sure to send further issue reports if you see anything wrong or stuff that could be improved.
While updating the cache of vendor-IDs I saw yours, goss and checked out the website at https://www.goss.com/

Glad to see your fine work. You may want to join our FontBakery chat channel at https://gitter.im/fontbakery/Lobby

Happy Hacking!

@schriftgestalt
Copy link

@schriftgestalt schriftgestalt commented Mar 7, 2020

A test like this could be useful for all the "name" entries (ID 1, 2, 4, 6, 16, 17, 18 ...)? But then check for more control characters.

(But maybe there is one already? I only found 'com.google.fonts/check/name/trailing_spaces')

@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 8, 2020

@schriftgestalt The line_breaks one checks every name entry

@tphinney
Copy link

@tphinney tphinney commented Mar 8, 2020

I am still left wondering WHY Google thinks a line break would be unacceptable in the description field, even in their own fonts.

@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 8, 2020

I am still left wondering WHY Google thinks a line break would be unacceptable in the description field, even in their own fonts.

@tphinney, this is described in the documentation for the check at
https://font-bakery.readthedocs.io/en/latest/fontbakery/profiles/googlefonts.html#com.google.fonts/check/name/line_breaks
Screenshot from 2020-03-08 00-06-01

@schriftgestalt
Copy link

@schriftgestalt schriftgestalt commented Mar 8, 2020

@schriftgestalt The line_breaks one checks every name entry

I know. What I meant was to make a new check that only checks those I mentioned.

@felipesanches
Copy link
Member

@felipesanches felipesanches commented Mar 8, 2020

please open a new issue describing the proposed new check in detail

@m4rc1e
Copy link
Member

@m4rc1e m4rc1e commented Nov 11, 2020

I still don't see the point of this check.

There's nothing technically wrong with having line breaks in name records. I agree there shouldn't be line breaks in nameIDs which are related to familyName and subFamilyName though.

After reading the check description, I reckon this check was implemented because too many fonts included the full license text? if this was the case, I'd prefer us to just fail name records which include license text or contain too much text. Again failing by length may be too arbitrary.

I worry this check will just make designers delete records when they may contain useful information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants