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

getEnglishName('fontFamily') returns undefined, font.names.macintosh is incomplete #570

Open
Connum opened this issue Feb 22, 2023 · 6 comments · May be fixed by #701
Open

getEnglishName('fontFamily') returns undefined, font.names.macintosh is incomplete #570

Connum opened this issue Feb 22, 2023 · 6 comments · May be fixed by #701
Assignees
Labels
Milestone

Comments

@Connum
Copy link
Contributor

Connum commented Feb 22, 2023

Expected Behavior

The font download shouldn't fail if font.getEnglishName('fontFamily') returns undefined, but even more so, font.getEnglishName('fontFamily') shouldn't return undefined in the first place.

Current Behavior

For some fonts font.download() fails because font.getEnglishName('fontFamily') returns undefined

Possible Solution

The issues seems to be related to #542. The name table.names.macintosh contains several numeric IDs for, but none of the regular entries. In getEnglishName() we should probably do this (we can't use optional chaining because it cannot be transpiled currently, but just to get the idea):

const translations = (this.names?.unicode[name] || this.names?.macintosh[name] || this.names?.windows[name]);

Also in names.js,

                let platform = name[platformName];
                if (platform === undefined) {
                    platform = name[platformName] = {};
                }
                let translations = platform[property];
                if (translations === undefined) {
                    translations = platform[property] = {};
                }

The name/platform shouldn't be initialized to an empty object, but the default name properties should be copied from one of the existing ones.

But also it shouldn't ever happen that the names objects are in such an irregular state.

Steps to Reproduce (for bugs)

  1. Download this font and load into opentype.js
  2. call font.getEnglishName('fontFamily')
  3. call font.download()

Your Environment

  • Version used: current master
  • Font used: Selawik-variable.ttf
  • Browser Name and version: -
  • Operating System and version (desktop or mobile): -
  • Link to your project: -
@Connum Connum added the bug label Feb 22, 2023
@Connum Connum added this to the Release 2.0.0 milestone Feb 22, 2023
@Connum Connum changed the title getEnglishName('fontFamily') returns undefined, font.names.unicode is incomplete getEnglishName('fontFamily') returns undefined, font.names.macintosh is incomplete Feb 22, 2023
@ILOVEPIE
Copy link
Contributor

ILOVEPIE commented Feb 22, 2023

If font.names.macintosh is incomplete then the font is corrupt, the macintosh names table is required.
At least according to the documentation i've read.

@Connum
Copy link
Contributor Author

Connum commented Feb 23, 2023

OK but then I still think we should handle this more gracefully, by setting the macintosh names to available names, our at least rather log a warning instead of throwing an error

@Connum
Copy link
Contributor Author

Connum commented Feb 23, 2023

It doesn't seem to be too uncommon by the way, I have another variable font with the same issue. Or it might have been the unicode table there, but I'll have to check again. Anyway we should handle this in a better way.

@ollimeier
Copy link
Contributor

If font.names.macintosh is incomplete then the font is corrupt, the macintosh names table is required. At least according to the documentation i've read.

The Mac names are not required anymore. I used to work for Monotype and now work for Fontwerk – and we don't produce any Mac name table entries anymore. Even Apple stopped producing mac names in their fonts.

@ollimeier
Copy link
Contributor

@Connum
Copy link
Contributor Author

Connum commented Apr 20, 2024

This will be fixed by #701

Font.prototype.getEnglishName = function(name) {
    const translations = (this.names.unicode || this.names.macintosh || this.names.windows)[name];
    if(!translations) {
        for(let platform of ['unicode', 'macintosh', 'windows']) {
            if(this.names[platform] && this.names[platform][name]) {
                return this.names[platform][name].en;
            }
        }
    }
    if (translations) {
        return translations.en;
    }
};

getEnglishName() will not only fall back table-wise, but also property-wise

@Connum Connum linked a pull request Apr 20, 2024 that will close this issue
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants