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 hasChar() for CMAP fonts and add tests (fix #330) #632

Merged
merged 1 commit into from
Nov 5, 2023

Conversation

Connum
Copy link
Contributor

@Connum Connum commented Oct 30, 2023

Description

For fonts using CMAP, hasChar() would always return true, even if the char was not contained. This wasn't caught because there was no corresponding test.

Motivation and Context

Fixes #330

How Has This Been Tested?

Added a new test loading the existing CMAP test font and checking the hasChar() return values for existing and non-existing chars.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I did npm run test and all tests passed green (including code styling checks).
  • I have added tests to cover my changes.
  • My change requires a change to the documentation.
  • I have updated the README accordingly.
  • I have read the CONTRIBUTING document.

@Connum Connum requested review from yne and ILOVEPIE November 5, 2023 15:50
@Connum Connum added this to the Release 2.0.0 milestone Nov 5, 2023
@Connum Connum merged commit 55cf26c into master Nov 5, 2023
2 checks passed
@Connum Connum deleted the fix/330-hasChar branch November 5, 2023 16:39
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.

charToGlyphIndex() in encoding.js never returns null
2 participants