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.

Sign up for GitHub

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

Jump to bottom

BUG: Support UTF-16-LE Strings #1884

Merged
merged 1 commit into from
Jul 3, 2023
Merged

BUG: Support UTF-16-LE Strings #1884

merged 1 commit into from
Jul 3, 2023

Conversation

adamchainz
Copy link
Contributor

Fixes #1838.

@adamchainz adamchainz mentioned this pull request Jun 14, 2023
Closed
@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.46 🎉

Comparison is base (dde4c79) 93.40% compared to head (f973c0c) 93.87%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1884      +/-   ##
==========================================
+ Coverage   93.40%   93.87%   +0.46%     
==========================================
  Files          34       34              
  Lines        6627     6854     +227     
  Branches     1299     1352      +53     
==========================================
+ Hits         6190     6434     +244     
+ Misses        285      273      -12     
+ Partials      152      147       -5
Impacted Files Coverage Δ
pypdf/generic/_utils.py 100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adamchainz adamchainz mentioned this pull request Jun 14, 2023
Merged
@MartinThoma
Copy link
Member

MartinThoma commented Jul 3, 2023 •
edited
Loading

@pubpub-zz I have a hard time reviewing this / understanding the impact. Should be fine, right?

@pubpub-zz
Copy link
Collaborator

For me yes
Juste extension to utf16le although not in accordance with standard, acrobat reader can handle utf16le, so aligning to it.

@MartinThoma MartinThoma merged commit 29c79fc into py-pdf:main Jul 3, 2023
@MartinThoma
Copy link
Member

Thank you @pubpub-zz 🙏

@MartinThoma
Copy link
Member

@adamchainz Thank you for your contribution ❤️ I'm sorry for the delay - I'm not a PDF expert and sometimes I need feedback by people who know more about PDF than I do (like pubpub-zz).

Your contribution is now in main and will get in the next release (pypdf>3.12.0). It will be on PyPI latest on Sunday.

If you want, I can add you to https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html as well :-)

@adamchainz adamchainz deleted the issue_1838_utf16le branch July 3, 2023 13:32
@adamchainz
Copy link
Contributor Author

No worries, I'm no expert either!

Don't worry about adding me to the CONTRIBUTORS list.

MartinThoma added a commit that referenced this pull request Jul 9, 2023
@MartinThoma
REL: 3.12.1
a66e9b3 
Bug Fixes (BUG):
-  Prevent updating page contents after merging page (stamping/watermarking) (#1952)
-  % to be hex encoded in names (#1958)
-  Inverse color in CMYK images (#1947)
-  Dates conversion not working with Z00\'00\' (#1946)
-  Support UTF-16-LE Strings (#1884)

[Full Changelog](3.12.0...3.12.1)
MartinThoma added a commit that referenced this pull request Jul 9, 2023
@MartinThoma
REL: 3.12.1
2b6941b 
Bug Fixes (BUG):
-  Prevent updating page contents after merging page (stamping/watermarking) (#1952)
-  % to be hex encoded in names (#1958)
-  Inverse color in CMYK images (#1947)
-  Dates conversion not working with Z00\'00\' (#1946)
-  Support UTF-16-LE Strings (#1884)

[Full Changelog](3.12.0...3.12.1)
@pubpub-zz pubpub-zz mentioned this pull request May 21, 2024
Closed
@pubpub-zz
Copy link
Collaborator

pubpub-zz commented May 21, 2024 •
edited
Loading

@adamchainz
there seems to be an issue with this PR reported in #2660
the use of UTF-16LE BOM should currently not be used.
From what I've read in #1838 the issue was in the information. It would be great if you could make a snapshot of the print-out of the property panel in acrobat reader and in Firefox to observe how this field is interpreted with some reference software

@adamchainz
Copy link
Contributor Author

I’m sorry, I no longer have access to the problem PDF. But yes the issue was in the information metadata, and at least macOS Preview correctly displayed the information: #1838 (comment) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support UTF-16-LE strings
3 participants
@adamchainz @MartinThoma @pubpub-zz