Skip to content

BUG: PdfWriter.add_uri was setting the wrong type#2406

Merged
MartinThoma merged 1 commit intopy-pdf:mainfrom
pmiller66:pmiller66-patch-1
Jan 17, 2024
Merged

BUG: PdfWriter.add_uri was setting the wrong type#2406
MartinThoma merged 1 commit intopy-pdf:mainfrom
pmiller66:pmiller66-patch-1

Conversation

@pmiller66
Copy link
Copy Markdown
Contributor

@pmiller66 pmiller66 commented Jan 12, 2024

BUG: /TYPE for a link should be "/Annot" not "/Annots" This change makes add_uri work; previously, it did not.

writer = pypdf.PdfWriter()
writer.add_blank_page(width=500, height=500)
writer.add_uri(0, 'https://cnn.com', [0, 0, 499, 499])
print ('writing demo.pdf')
writer.write('demo.pdf')

/TYPE for a link should be "/Annot" not "/Annots"

This change makes add_uri work;  previously, it did not.
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fc893d5) 94.42% compared to head (8e461c0) 94.42%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2406   +/-   ##
=======================================
  Coverage   94.42%   94.42%           
=======================================
  Files          49       49           
  Lines        7961     7961           
  Branches     1608     1608           
=======================================
  Hits         7517     7517           
  Misses        274      274           
  Partials      170      170           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pmiller66 pmiller66 changed the title Update _writer.py to fix broken add_uri BUG: Update _writer.py to fix broken add_uri Jan 13, 2024
@stefan6419846
Copy link
Copy Markdown
Collaborator

Thanks. Do we have a chance to add a corresponding test as well?

@pmiller66
Copy link
Copy Markdown
Contributor Author

pmiller66 commented Jan 16, 2024 via email

@MartinThoma MartinThoma added the is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF label Jan 17, 2024
@MartinThoma
Copy link
Copy Markdown
Member

Table 30 – Entries in a page object clearly says /Annots, so at least the PG constant is not wrong.

However, Table 164 – Entries common to all annotation dictionaries is the relevant part. There it is /Annot

Good catch! 👍

@MartinThoma MartinThoma changed the title BUG: Update _writer.py to fix broken add_uri BUG: The /Link subtype has the type /Annot, not /Annots Jan 17, 2024
@MartinThoma MartinThoma changed the title BUG: The /Link subtype has the type /Annot, not /Annots BUG: Link creation with add_uri was setting the wrong type Jan 17, 2024
@MartinThoma MartinThoma changed the title BUG: Link creation with add_uri was setting the wrong type BUG: PdfWriter.add_uri was setting the wrong type Jan 17, 2024
@MartinThoma MartinThoma merged commit 5365720 into py-pdf:main Jan 17, 2024
@MartinThoma
Copy link
Copy Markdown
Member

@pmiller66 Thank you for your PR 🙏 It will be part of pypdf==4.0.0. As this is a major release, it might take a bit longer than usual for the release on PyPI. I want to get some breaking changes into it. My guess is that we will publish pypdf==4.0.0 end of January/early February

@MartinThoma
Copy link
Copy Markdown
Member

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

@MartinThoma
Copy link
Copy Markdown
Member

Regarding the testing: I don't see an easy way to meaningfully test this. Essentially, we would either (1) need to rely on another library and make a comparison between the generated PDFs or (2) have some RPA-like process and thus relying on a PDF viewer + the RPA software.

Nothing you need to do in this PR as it's quite obvious that this change is correct and the PR improves pypdf :-)

@pmiller66
Copy link
Copy Markdown
Contributor Author

pmiller66 commented Jan 18, 2024 via email

MartinThoma added a commit that referenced this pull request Jan 18, 2024
MartinThoma added a commit that referenced this pull request Jan 19, 2024
## What's new

pypdf==4.0.0 is a big milestone forward:

* We finally have a layout-mode text extraction.
  This enables users who want to detect / extract tables
  with heuristics to give it a try.
* We deprecated a lot of the old PyPDF2 API that was either
  not following PEP8 naming styles or was not using a
  property. Users comming from PyPDF2 might want to switch
  first to pypdf<4.0.0 to get helpful error messages
  that show the new API in their speicific cases.

A big 'Thank you!' the the whole pypdf community for your
work. Thanks to you, pypdf is better than ever.

Kudos to @shartzog who added the layout-mode with his first
contribution!

### Deprecations (DEP)
-  Drop Python 3.6 support (#2369) by @MartinThoma
-  Remove deprecated code (#2367) by @MartinThoma
-  Remove deprecated XMP properties (#2386) by @stefan6419846

### New Features (ENH)
-  Add "layout" mode for text extraction (#2388) by @shartzog
-  Add Jupyter Notebook integration for PdfReader (#2375) by @MartinThoma
-  Improve/rewrite PDF permission retrieval (#2400) by @stefan6419846

### Bug Fixes (BUG)
-  PdfWriter.add_uri was setting the wrong type (#2406) by @pmiller66
-  Add support for GBK2K cmaps (#2385) by @stefan6419846

### Documentation (DOC)
-  Add pmiller66 for #2406 as a contributor by @MartinThoma
-  Add missing expand parameter (#2393) by @Atomnp
-  Resolve build warnings (#2380) by @stefan6419846
-  Fix testing prerequisites (#2381) by @stefan6419846
-  Improve formatting of contributors page (#2383) by @stefan6419846
-  Add Tobeabellwether as a contributor for #2341 by @MartinThoma

### Developer Experience (DEV)
-  Make dependabot aware of our PR prefixes (#2415) by @stefan6419846
-  Fail on Sphinx issues (#2405) by @stefan6419846
-  Move title check to own workflow (#2384) by @MasterOdin
-  Write to temporary files instead of the working directory (#2379) by @stefan6419846
-  Ensure that the PR titles have the correct format (#2378) by @stefan6419846

### Maintenance (MAINT)
-  Complete FileSpecificationDictionaryEntries constants (#2416) by @MartinThoma
-  Return None instead of -1 when page is not attached (#2376) by @MartinThoma
-  Replace warning with logging.error (#2377) by @MartinThoma

### Testing (TST)
-  Add missing pytest.mark.samples annotations (#2412) by @kitterma
-  Correctly close temporary files (#2396) by @stefan6419846
-  Fix  side effect #2379 (#2395) by @pubpub-zz
-  Add test for layout extraction mode (#2390) by @MartinThoma

### Code Style (STY)
-  Use the UserAccessPermissions enum (#2398) by @MartinThoma
-  Run black (#2370) by @MartinThoma

[Full Changelog](3.17.4...4.0.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants