Skip to content

Compute the next ParentTree key correctly - #21714

Merged
calixteman merged 1 commit into
mozilla:masterfrom
calixteman:fix/pdf-editor-parent-tree-next-key
Aug 7, 2026
Merged

Compute the next ParentTree key correctly#21714
calixteman merged 1 commit into
mozilla:masterfrom
calixteman:fix/pdf-editor-parent-tree-next-key

Conversation

@calixteman

@calixteman calixteman commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

/ParentTreeNextKey was set to parentTree.size, but the map is sparse: extractPages deletes the entries of the struct elements that were never cloned, so size is no longer a free key. PDF 32000-1 Table 322 requires "an integer greater than any key in the parent tree", hence max(key) + 1.

@calixteman
Compute the next ParentTree key correctly
0f4ec62 
`/ParentTreeNextKey` was set to `parentTree.size`, but the map is sparse:
`extractPages` deletes the entries of the struct elements that were never
cloned, so `size` is no longer a free key. PDF 32000-1 Table 322 requires
"an integer greater than any key in the parent tree", hence `max(key) + 1`.
@codecov-commenter

codecov-commenter commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.03%. Comparing base (e57a464) to head (0f4ec62).
⚠️ Report is 22 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #21714      +/-   ##
==========================================
- Coverage   90.04%   90.03%   -0.01%     
==========================================
  Files         264      264              
  Lines       66894    66942      +48     
==========================================
+ Hits        60232    60273      +41     
- Misses       6662     6669       +7
Flag Coverage Δ
browsertest 66.50% <ø> (-0.02%) ⬇️
fonttest 9.03% <ø> (ø)
integrationtest 69.34% <ø> (-0.02%) ⬇️
unittest 57.99% <ø> (+0.11%) ⬆️
unittestcli 56.70% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

SamyCookie
SamyCookie reviewed Aug 5, 2026
View reviewed changes
Comment thread src/core/editor/pdf_editor.js
timvandermeij
timvandermeij approved these changes Aug 6, 2026
View reviewed changes

@timvandermeij timvandermeij left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me; thanks!

@calixteman
calixteman merged commit 821a741 into mozilla:master Aug 7, 2026
20 of 21 checks passed
@calixteman
calixteman deleted the fix/pdf-editor-parent-tree-next-key branch August 7, 2026 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timvandermeij timvandermeij timvandermeij approved these changes
+1 more reviewer
@SamyCookie SamyCookie SamyCookie left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

split-merge-organize

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@calixteman @codecov-commenter @timvandermeij @SamyCookie