Keep sales PDF item rows from splitting across a page break

[TuVanDev]

Description (*)

AbstractPdf::drawLineBlocks() has a block-level page-break check that is meant to move a whole item block to the next page when it no longer fits in the space left on the current one:

if ($this->y - $itemsProp['shift'] < 15) {
    $page = $this->newPage($pageSettings);
}

shift is documented on the method as "full line height (optional)", and when it is absent drawLineBlocks() computes it from the block's own lines. Every core item renderer, however, passes a hardcoded 'shift' => 5:

• Magento\Sales\Model\Order\Pdf\Items\Invoice\DefaultInvoice
• Magento\Sales\Model\Order\Pdf\Items\Shipment\DefaultShipment
• Magento\Bundle\Model\Sales\Order\Pdf\Items\Invoice
• Magento\Bundle\Model\Sales\Order\Pdf\Items\Shipment
• Magento\Bundle\Model\Sales\Order\Pdf\Items\Creditmemo

With shift = 5 the check is effectively dead: it only fires in the last 20pt of a page. An item block that does not fit in the remaining space is therefore started anyway and broken mid-block by correctText(), so a single item row is split across the page boundary. The product-name and option lines end up on one page while that same item's SKU, qty, price, tax and subtotal columns are drawn on the next one, where the numeric columns then line up against the wrong name fragment.

Magento\Sales\Model\Order\Pdf\Items\Creditmemo\DefaultCreditmemo no longer has the hardcoded value: it was removed as part of ACP2E-4630 (commit 545981c), which fixed the related overlapping-text defect by returning the new page out of correctText() / correctLines(). This pull request finishes that work for the remaining five renderers.

Removing 'shift' => 5 on its own is not enough, because a block taller than one usable page then makes the check true even at the top of a fresh page: newPage() starts drawing at y=800 and the guard is < 15, so a block over ~785pt (less the table-header row) can never satisfy it. The break fires, the page that was just created is left with nothing but its table header, and the block overflows across the boundary anyway. Two additions prevent that:

• fitsOnEmptyPage() suppresses the break when the block cannot fit on an empty page. Breaking gains nothing there, it only leaves the rest of the current page empty.
• createPage() records the Y coordinate each new page starts drawing at, so fitsOnEmptyPage() compares against the real top of the current page (a table header lowers it) rather than an assumed one. For a page created elsewhere, such as the first page of each document built in getPdf(), only the un-lowered coordinate newPage() sets is known, and that is used instead.

Together these make an empty page structurally impossible: on a page that has nothing drawn on it yet $this->y equals that page's recorded top, so $this->y - $shift < 15 and $shift <= $pageTop - 15 are exact negations of each other and the break cannot fire.

That also removes a latent empty-page case in the credit memo renderer, which has had the computed shift since ACP2E-4630 but no such guard: an item whose option block is taller than a page currently gets pushed to a fresh page, leaving the credit memo's first page empty below its table header, and still overflows. Measured locally at 3 pages before this change and 2 after.

The two literals the check depends on (800 from newPage() and 15) are now named constants, since the new guard has to agree with both.

Related Pull Requests

Extends ACP2E-4630 (commit 545981c), which fixed the overlapping-text half of this code path and removed the hardcoded shift from the credit memo renderer only.

Fixed Issues (if relevant)

None linked. Found while investigating multi-page invoice PDFs on 2.4.7-p10.

Manual testing scenarios (*)

1. Create an order with several products whose custom options render a long option list (about 10-12 options per item is enough), so that a single item block is a few hundred points tall.
2. Invoice the order and open Sales > Invoices > View > Print. Repeat for a shipment (packing slip), a credit memo, and the Print Invoices mass action over several orders.
3. Before the change: at each page boundary an item is cut in half. Its option lines continue at the top of the next page with no product name, SKU, qty, price or subtotal row, and those columns for that item are back on the previous page.
4. After the change: every page boundary falls between items. A page begins with a complete item row (name, SKU, qty, price, tax, subtotal) and no item's option lines are separated from its own columns.
5. Add one item with an option list long enough to exceed a whole page (about 25 options). The page count must not increase versus before the change, and no page may be produced that contains only the item-table header row.
6. Print an invoice, packing slip and credit memo that already fit on a single page. They must be unchanged.

Verified on 2.4.7-p10 with the ACP2E-4630 change applied, over real invoices, packing slips, credit memos, order prints and a 5-invoice mass action, plus synthetic option blocks sized just under and just over one page:

scenario	pages before > after	item rows split across a page break, before > after	pages holding only the table header, before > after
invoice, 41 items	9 > 11	8 > 0	0 > 0
packing slip, 21 items	9 > 11	7 > 0	0 > 0
order print, 23 items	3 > 3	1 > 0	0 > 0
credit memo, 18 items	2 > 2	0 > 0	0 > 0
mass action, 5 invoices	17 > 19	10 > 0	0 > 0
item block taller than a page, invoice	2 > 2	1 > 1	0 > 0
item block taller than a page, credit memo	3 > 2	1 > 1	0 > 0
single-item invoice / packing slip / credit memo / order print	1 > 1	0 > 0	0 > 0

The remaining split on a block taller than a page is unavoidable: it cannot fit anywhere, so it is drawn from where it stands instead of costing an extra page. Single-page documents draw exactly the same text at exactly the same coordinates as before. Page counts rise on documents whose items were previously being cut in half, which is what the check is for: the extra pages buy back the item rows that were being broken.

Magento\Sales\Test\Unit\Model\Order\Pdf\AbstractTest::testDrawLineBlocksBreaksPageOnlyForABlockThatFits covers both halves of the new guard. It fails on 2.4-develop as it stands today (Zend_Pdf::newPage(...) was not expected to be called) and passes with this change.

Questions or comments

PAGE_TOP_Y is kept private and used only inside AbstractPdf. The three concrete PDF models (Invoice, Shipment, Creditmemo) override newPage() and set $this->y = 800 themselves; making the constant protected and using it there too would be a reasonable follow-up, but it is outside the scope of this fix.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

Resolved issues:

1. resolves [Issue] Keep sales PDF item rows from splitting across a page break #41064: Keep sales PDF item rows from splitting across a page break

[m2-assistant]

Hi @TuVanDev. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

[engcom-Charlie]

@magento create issue