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

Fix unique glyph codes check in getSubsetText #92

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

Moustachos
Copy link
Contributor

Don't use getUniqText() for glyphs subset, check for codes uniqueness in string2unicodes() instead.

getUniqueText() process passed string as a regular one. In some cases it doesn't handle unicode properly and will remove code points parts when several code points share the same numeric parts.

Ultimately, it will result in wrong code points being pushed to output.

In some cases, the glyphs count will match, not the codes themselves (tested and confirmed with FontAwesome's Duotone font variation).

Fix UTF-16 glyphs subset (and probably others), tested and confirmed with FontAwesome's Duotone font variation where glyphs code points look like this: \10f6ae (https://fontawesome.com/v5.15/icons/acorn?style=duotone).

Keeping getUniqText() method for backward compatibility / third party packages relying on it.

… in string2unicodes() instead

getUniqueText() process passed string as a regular one, it doesn't handle unicode properly and will remove code points parts when several code points share the same numeric parts.

Ultimately, it will result in wrong code points being pushed to output.

The glyphs count will match, not the codes themselves.

Fix subset of UTF-16 glyphs (and probably others).

Keeping getUniqText() method for backward compatibility / third party packages relying on it.
@junmer junmer merged commit 293351e into ecomfe:master Sep 10, 2021
@junmer
Copy link
Contributor

junmer commented Sep 10, 2021

@Moustachos thank you 😄
bump fontmin@0.9.9

@Moustachos
Copy link
Contributor Author

Nice :D

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

Successfully merging this pull request may close these issues.

2 participants