DOMException: A network error occurred while loading sans-serif font face

[suresh-gangumalla]

Observed an exception while loading sans-serif as default font face.

https://github.com/lightning-js/renderer/blob/main/src/core/text-rendering/renderers/CanvasTextRenderer.ts#L148

[uguraslan]

It appears that the variable fontUrlWithoutParentheses, as defined here, is assigned an empty string when using the default sans-serif font. So the fontFace.load() method, called here, tries to load a font from this URL, resulting in a failure since the URL is an empty string.

The FontFace constructor requires the source parameter to be either a valid URL or binary font face data.

[wouterlucas]

Need to retest if this still happens with #319

[suresh-gangumalla]

@wouterlucas
the issue is still reproducing. I have verified this against latest dev branch, 1.0.0-rc.3.

[elsassph]

@wouterlucas I was investigating it and indeed as @suresh-gangumalla pointed out, the CanvasTextRenderer "loads" a default sans-serif font using WebTrFontFace, but the DOM FontFace will throw this error when the URL is empty.

It is harmless in itself but unnecessary. I presume we could avoid calling FontFace.load and just flag it as loaded.

Also regarding system fonts in general, L3 has a copy of some L2 code which include a bug/limitation fixed since in L2: rdkcentral/Lightning#539

[elsassph]

Generally canvas font loading needs to be revisited: the systematic call to globalFontSet.load looks problematic IMHO as it makes all text metrics/rendering asynchronous, even when fonts have been long loaded.

[frank-weindel]

@elsassph One of the #322 canvas experiments addresses that and provides a path for loading the texture completely synchronously if the font is already loaded.

[wouterlucas]

Let's do a temp fix on the current code base until #322 is mature enough to move along.

[elsassph]

I'm concerned about whether #322 will progress while Frank is away. Some improvements should be extracted into granular PRs.

[wouterlucas]

It will, I just want a stopgap solution in there so that the error is gone and so we can do our due diligence on #322

Agree that chopping it up in smaller PRs is a good idea.