Comma included in load font

[perminder-17]

Resolves #8077

Changes:

Screenshots of the change:

PR Checklist

• npm run lint passes
• Inline reference is included / updated
• Unit tests are included / updated

[perminder-17]

I was testing it, it fails somewhere. I'll check it soon.

[perminder-17]

https://editor.p5js.org/permindersingh089/sketches/AW4WJ5Vbv

[dhowe]

Can we included a test case for this ?

[perminder-17]

Hi @davepagurek , when I included only comma with name.includes(',') in this commit bf6cca2 it was working fine locally, but as soon as I uploaded it to web-editor and tried running the same sketch, i got errors saying

Error: Failed to create FontFace for "855afbab-17c2-467e-8cfd-344d8899a254" at createFontFace (https://assets.editor.p5js.org/650a2a90fbcb53001cc5b664/109727ce-328d-4e9d-8382-bec26f5259b6.js?v=1757769773914:119658:13)

I think the web editor renames uploaded font files to UUID-like names (e.g. fbb83b…), which may or may-not start with a digit. Such names need quotes to work as a font-family.

if (!((name.startsWith("'") && name.endsWith("'")) ||
       (name.startsWith('"') && name.endsWith('"')))) {
   name = "'" + name.replace(/'/g, "\\'") + "'";
 }

where we started quoting it everytime but still it was working on firefox correctly but not in chrome, now I am using regex-check and quoting when needed which is working everywhere in chrome, firefox and safari.

Do you think the PR looks good @dhowe @davepagurek @ksen0 ?

[davepagurek]

I think this regex makes sense! The edge cases we're handling could probably use some documentation though so that we don't forget later, so I think some unit tests would be a good idea, are we able to export this function so we can test it directly in a unit test without loading actual fonts? That way we can show in unit tests that ones starting with a digit need quotes, spaces need quotes, commas need quotes, and fully alphabetic (or alphanumeric as long as the first character is alphabetic) don't need quotes.

[perminder-17]

I think this regex makes sense! The edge cases we're handling could probably use some documentation though so that we don't forget later, so I think some unit tests would be a good idea, are we able to export this function so we can test it directly in a unit test without loading actual fonts? That way we can show in unit tests that ones starting with a digit need quotes, spaces need quotes, commas need quotes, and fully alphabetic (or alphanumeric as long as the first character is alphabetic) don't need quotes.

Sure, thanks for the confirmation. I'll see how we can export this function so we can test it directly in the unit test without loading the actual font.

[ksen0]

@perminder-17 If there are blockers with exporting the function, IMO adding to the manual test suite would also be a good alternative, these are checked before release, so it would still be a way to prevent future regression.