graphicore / specimenTools

Apache License 2.0
29 stars 5 forks source link

onLoadFont errors are no longer fatal to instantiation #10

Closed kontur closed 7 years ago

kontur commented 7 years ago

Previously a faulty passed in font (not a opentype font, invalid file name) would cause opentype.js fail and halt script execution. I slightly modified onLoadFont to handle those cases more gracefully with a console error message and proceeding to still load the other fonts. Not perfect, but better than failing outright.

kontur commented 7 years ago

One shortcoming in particular is that the fontSet originally passed in to other components is oblivious of any failed fonts.

kontur commented 7 years ago

Here are different errors that are causing the confusion in structuring that function, and also in the assumption that if either error or fontArrayBuffer are not null you can assume something about the other:

I added a check to prevent the second kind in https://github.com/graphicore/specimenTools/pull/10/commits/5059d2bc5ce5b9394d144587b99b512da1af24d0#diff-13b0f15736fe8862fee9435ca901a862R79 One question that remains for me is if ever any error !== null is passed in, won't it already be listed in the console? At least the 404 is already, but I don't know if loading from file input would be different?

graphicore commented 7 years ago
  • passed in an empty string as URL: error === null, fontArrayBuffer !== null

...

I added a check to prevent the second kind in 5059d2b#diff-13b0f15736fe8862fee9435ca901a862R79 One question that remains for me is if ever any error !== null is passed in, won't it already be listed in the console? At least the 404 is already, but I don't know if loading from file input would be different?

...

if (fontFiles[i]) {

You don't want to silence this kind of user/input error. You could raise an error though, if you are concerned about this:

if (!fontFiles[i])
    throw new Error('The url at index '+i+' appears to be invalid.');
graphicore commented 7 years ago
    function onLoadFont(i, fontFileName, err, fontArraybuffer) {
        /* jshint validthis: true */
        var font;
        if(!err)
            try {
                font = opentype.parse(fontArraybuffer);
            }
            catch (parseError) {
                err = parseError;
            }

        if(err) {
            console.warn('Can\'t load font', fontFileName, ' with error:', err);
            this.countAll--;
        }
        else {
            this.pubsub.publish('loadFont', i, fontFileName, font, fontArraybuffer);
            this.countLoaded += 1;
        }

        if(this.countLoaded === this.countAll)
            this.pubsub.publish('allFontsLoaded', this.countAll);

    }