Repository navigation
Conversation
…de (issue 22410) The Adobe Glyph List Specification says that everything from the first period onwards in a glyph name is a suffix, e.g. `c.sc`, `T_h.alt` or `one.os`, which doesn't change the character(s) of the glyph. Hence the text of such glyphs was previously extracted as the character code itself, which broke e.g. text selection and search, even though the glyphs were rendered correctly. The existing `issue10640.pdf` test expected exactly this wrong text, i.e. the glyph names `one.os`/`two.os`/`zero.os` ended up as the control characters `\x81`/`\x82`/`\x80` instead of the digits `1`/`2`/`0`, so update it.
Collaborator
|
Sorry, but that issue really does not seem like a good fit for a first-time contributor who appears to just submit fully AI generated code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22410.
The Adobe Glyph List Specification says that everything from the first period onwards in a glyph name is a suffix (for example
c.sc,T_h.altorone.os), which doesn't change the character(s) of the glyph: https://github.com/adobe-type-tools/agl-specification#2-the-mapping-algorithmPartialEvaluator._simpleFontToUnicodedidn't do that, so the text of simple fonts with such/Differencesnames was extracted as the character code itself. The glyphs were rendered correctly, but text selection, search andgetTextContentreturned the wrong characters.This patch looks up the name without the suffix, if the full name isn't in the glyph list, and then continues with the existing heuristics using that name. This means that
c.scisc,uni00E9.scgoes through the existinguniXXXXhandling, andT_h.altreaches the existingT_hligature case. A name that starts with a period (.notdef) is left alone.I did not add the AGL rule for ligature names in general (
a_bisab) since it isn't needed for this issue, and I'd rather keep this change small.Test:
issue22410.pdf(the file from the issue, 3 KB) and anapi_spec.jstest that expectsnicht The café; it fails without the change (ni!ht "e caf#).issue 10640test expected the same bug. That PDF usesone.os,two.osandzero.os, and its test expected the control characters\x81,\x82and\x80where the page has1,2and0(for exampleDirectory [\x81]andversion \x82.\x80). With this patch the text isDirectory [1]andversion 2.0, so I updated that expectation.arbeitgeberbescheinigung-max-mustermann.pdf) now hasNein, trifft nicht zuon pages 2 and 3, where it wasNein, trifft ni!ht zu.masterapart from this (the failures that remain are in tests whose PDFs I hadn't downloaded).eslintis clean.This change was prepared with the help of an AI coding assistant (Devin CLI) and reviewed by me.