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

Don't blindly trust toUnicode when building toFontChar for non-standard fonts without a font file (issue 4934) #4944

Merged
merged 1 commit into from
Jun 24, 2014

Conversation

Snuffleupagus
Copy link
Collaborator

This PR does fix the issue, and it passes all tests locally, however I am really not sure if this is the correct way to solve the issue.

For non-standard fonts without font files, relying solely on toUnicode when building toFontChar can be problematic since toUnicode may contain strange/bogus entries.
In the PDF file in the issue referenced below, toUnicode actually maps most upper-case letters to their lower-case versions.
I've attempted to solve this by instead checking if differences or defaultEncoding exist and, if so, use them before accepting a toUnicode entry.

Please note: this breaks the character mapping when used with CIDFontType fonts, hence it's not used in those cases. This distinction seemed somewhat arbitrary, but before PR #4259 those fonts were apparently handled slightly differently than other fonts e.g. in charToGlyph.

Fixes #4934.

@Snuffleupagus
Copy link
Collaborator Author

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/c119493971abd15/output.txt

@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/d40e48ca2b05a7b/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/61ec97f4cb43baa/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/61ec97f4cb43baa/output.txt

Total script time: 20.94 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/d40e48ca2b05a7b/output.txt

Total script time: 24.63 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@yurydelendik
Copy link
Contributor

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/74f4f98365f0c81/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/5ccb3ca5a34ef7e/output.txt

@yurydelendik yurydelendik reopened this Jun 24, 2014
@yurydelendik
Copy link
Contributor

Thank you for the fix

yurydelendik added a commit that referenced this pull request Jun 24, 2014
Don't blindly trust toUnicode when building toFontChar for non-standard fonts without a font file (issue 4934)
@yurydelendik yurydelendik merged commit c28839b into mozilla:master Jun 24, 2014
@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/5ccb3ca5a34ef7e/output.txt

Total script time: 21.16 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/74f4f98365f0c81/output.txt

Total script time: 23.83 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@Snuffleupagus Snuffleupagus deleted the issue-4934 branch June 24, 2014 08:18
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.

Text not capitalized
3 participants