Keep sales PDF item rows from splitting across a page break

app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Creditmemo.php

@@ -210,7 +210,7 @@ public function draw()
                     $lines[][] = ['text' => $text, 'feed' => $leftBound + 5];
                 }
 
-                $drawItems[] = ['lines' => $lines, 'height' => 20, 'shift' => 5];
+                $drawItems[] = ['lines' => $lines, 'height' => 20];
             }
         }
 

app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Invoice.php

@@ -250,7 +250,7 @@ private function drawCustomOptions(array $draw): array
                     $lines[][] = ['text' => $text, 'feed' => 40];
                 }
 
-                $draw[] = ['lines' => $lines, 'height' => 20, 'shift' => 5];
+                $draw[] = ['lines' => $lines, 'height' => 20];
             }
         }
 

app/code/Magento/Bundle/Model/Sales/Order/Pdf/Items/Shipment.php

@@ -186,7 +186,7 @@ public function draw()
                     $lines[][] = ['text' => $text, 'feed' => 115];
                 }
 
-                $drawItems[] = ['lines' => $lines, 'height' => 20, 'shift' => 5];
+                $drawItems[] = ['lines' => $lines, 'height' => 20];
             }
         }
 

app/code/Magento/Sales/Model/Order/Pdf/AbstractPdf.php

@@ -51,6 +51,16 @@ abstract class AbstractPdf extends \Magento\Framework\DataObject
 
     public const XML_PATH_SALES_PDF_CREDITMEMO_PUT_ORDER_ID = 'sales_pdf/creditmemo/put_order_id';
 
+    /**
+     * Y coordinate a new page starts drawing at
+     */
+    private const PAGE_TOP_Y = 800;
+
+    /**
+     * Lowest Y coordinate a line of text may be drawn at
+     */
+    private const MIN_TEXT_Y = 15;
+
     /**
      * Zend PDF object
      *
@@ -142,6 +152,16 @@ abstract public function getPdf();
      */
     private $pageSettings;
 
+    /**
+     * @var \Zend_Pdf_Page|null
+     */
+    private $lastCreatedPage;
+
+    /**
+     * @var int
+     */
+    private $lastCreatedPageTop = self::PAGE_TOP_Y;
+
     /**
      * @var Database
      */
@@ -996,7 +1016,7 @@ public function newPage(array $settings = [])
         $pageSize = !empty($settings['page_size']) ? $settings['page_size'] : \Zend_Pdf_Page::SIZE_A4;
         $page = $this->_getPdf()->newPage($pageSize);
         $this->_getPdf()->pages[] = $page;
-        $this->y = 800;
+        $this->y = self::PAGE_TOP_Y;
 
         return $page;
     }
@@ -1063,8 +1083,10 @@ public function drawLineBlocks(\Zend_Pdf_Page $page, array $draw, array $pageSet
                 $itemsProp['shift'] = $shift;
             }
 
-            if ($this->y - $itemsProp['shift'] < 15) {
-                $page = $this->newPage($pageSettings);
+            if ($this->y - $itemsProp['shift'] < self::MIN_TEXT_Y
+                && $this->fitsOnEmptyPage($page, $itemsProp['shift'])
+            ) {
+                $page = $this->createPage($pageSettings);
             }
             $page = $this->correctLines($lines, $page, $height);
         }
@@ -1120,6 +1142,41 @@ private function correctLines($lines, $page, $height) :\Zend_Pdf_Page
         return $page;
     }
 
+    /**
+     * Create a new page and remember the Y coordinate it starts drawing at.
+     *
+     * @param array $pageSettings
+     * @return \Zend_Pdf_Page
+     */
+    private function createPage(array $pageSettings): \Zend_Pdf_Page
+    {
+        $page = $this->newPage($pageSettings);
+        $this->lastCreatedPage = $page;
+        $this->lastCreatedPageTop = $this->y;
+
+        return $page;
+    }
+
+    /**
+     * Whether a block of the given height still fits when started from the top of an empty page.
+     *
+     * A taller block overflows wherever it starts, so breaking the page for it only leaves the
+     * rest of the current page empty - and leaves the page entirely empty when the block is the
+     * first thing drawn on it.
+     *
+     * @param \Zend_Pdf_Page $page
+     * @param int $blockHeight
+     * @return bool
+     */
+    private function fitsOnEmptyPage(\Zend_Pdf_Page $page, $blockHeight): bool
+    {
+        // Pages this class created start at a known coordinate, which a table header lowers.
+        // For any other page only the un-lowered coordinate newPage() sets is known.
+        $pageTop = $page === $this->lastCreatedPage ? $this->lastCreatedPageTop : self::PAGE_TOP_Y;
+
+        return $blockHeight <= $pageTop - self::MIN_TEXT_Y;
+    }
+
     /**
      * Correct text.
      *
@@ -1137,8 +1194,8 @@ private function correctText($column, $height, $font, $page) :array
         $lineSpacing = !empty($column['height']) ? $column['height'] : $height;
         $fontSize = empty($column['font_size']) ? 10 : $column['font_size'];
         foreach ($column['text'] as $part) {
-            if ($this->y - $top < 15) {
-                $page = $this->newPage($this->pageSettings);
+            if ($this->y - $top < self::MIN_TEXT_Y) {
+                $page = $this->createPage($this->pageSettings);
                 $top = 0;
             }
 

app/code/Magento/Sales/Model/Order/Pdf/Items/Invoice/DefaultInvoice.php

@@ -165,7 +165,7 @@ public function draw()
             }
         }
 
-        $lineBlock = ['lines' => $lines, 'height' => 20, 'shift' => 5];
+        $lineBlock = ['lines' => $lines, 'height' => 20];
 
         $page = $pdf->drawLineBlocks($page, [$lineBlock], ['table_header' => true]);
         $this->setPage($page);

app/code/Magento/Sales/Model/Order/Pdf/Items/Shipment/DefaultShipment.php

@@ -118,7 +118,7 @@ public function draw()
             }
         }
 
-        $lineBlock = ['lines' => $lines, 'height' => 20, 'shift' => 5];
+        $lineBlock = ['lines' => $lines, 'height' => 20];
 
         $page = $pdf->drawLineBlocks($page, [$lineBlock], ['table_header' => true]);
         $this->setPage($page);

app/code/Magento/Sales/Test/Unit/Model/Order/Pdf/AbstractTest.php

@@ -288,6 +288,107 @@ public function testDrawLineBlocksPropagatesNewPageToSiblingColumns(): void
         $this->assertSame(['name-line-2', 'sku-line'], $drawnOnPageTwo);
     }
 
+    /**
+     * A block is moved to a new page only when it can fit on one.
+     *
+     * A block taller than an empty page overflows wherever it starts, so breaking the page for it
+     * leaves the rest of the current page empty without keeping the block together.
+     *
+     * @param int $blockHeight
+     * @param int $expectedNewPages
+     * @return void
+     * @throws \ReflectionException
+     * @dataProvider blockHeightDataProvider
+     */
+    public function testDrawLineBlocksBreaksPageOnlyForABlockThatFits(
+        int $blockHeight,
+        int $expectedNewPages
+    ): void {
+        $paymentData = $this->createMock(Data::class);
+        $string = $this->createMock(StringUtils::class);
+        $scopeConfig = $this->createMock(ScopeConfigInterface::class);
+        $filesystem = $this->createMock(Filesystem::class);
+        $pdfConfig = $this->createMock(Config::class);
+        $pdfTotalFactory = $this->createMock(Factory::class);
+        $pdfItemsFactory = $this->createMock(ItemsFactory::class);
+        $localeMock = $this->createMock(TimezoneInterface::class);
+        $translate = $this->createMock(StateInterface::class);
+        $addressRenderer = $this->createMock(Renderer::class);
+        $taxHelper = $this->createMock(TaxHelper::class);
+        $fileStorageDatabase = $this->createMock(Database::class);
+        $rtlTextHandler = $this->createMock(RtlTextHandler::class);
+        $image = $this->createMock(Image::class);
+
+        $abstractPdfMock = $this->getMockBuilder(AbstractPdf::class)
+            ->setConstructorArgs([
+                $paymentData,
+                $string,
+                $scopeConfig,
+                $filesystem,
+                $pdfConfig,
+                $pdfTotalFactory,
+                $pdfItemsFactory,
+                $localeMock,
+                $translate,
+                $addressRenderer,
+                [],
+                $fileStorageDatabase,
+                $rtlTextHandler,
+                $image,
+                $taxHelper
+            ])
+            ->onlyMethods(['_setFontRegular', '_getPdf', 'getPdf'])
+            ->getMock();
+
+        $pageOne = $this->createMock(\Zend_Pdf_Page::class);
+        $pageTwo = $this->createMock(\Zend_Pdf_Page::class);
+        $zendFont = $this->createMock(\Zend_Pdf_Font::class);
+        $zendPdf = $this->createMock(\Zend_Pdf::class);
+
+        $zendPdf->expects($this->exactly($expectedNewPages))->method('newPage')->willReturn($pageTwo);
+        $abstractPdfMock->expects($this->atLeastOnce())->method('_setFontRegular')->willReturn($zendFont);
+        $abstractPdfMock->expects($this->any())->method('_getPdf')->willReturn($zendPdf);
+
+        // Half way down the page: too little room left for either block.
+        $abstractPdfMock->y = 400;
+
+        $expectedPage = $expectedNewPages === 0 ? $pageOne : $pageTwo;
+        $otherPage = $expectedNewPages === 0 ? $pageTwo : $pageOne;
+        $expectedPage->expects($this->once())
+            ->method('drawText')
+            ->with('single-line', 35, $this->anything(), 'UTF-8');
+        $otherPage->expects($this->never())->method('drawText');
+
+        $drawBlockLineData = [[
+            'lines' => [[['text' => ['single-line'], 'feed' => 35]]],
+            'height' => 20,
+            'shift' => $blockHeight
+        ]];
+
+        $reflectionMethod = new \ReflectionMethod(AbstractPdf::class, 'drawLineBlocks');
+        $resultPage = $reflectionMethod->invoke(
+            $abstractPdfMock,
+            $pageOne,
+            $drawBlockLineData,
+            ['table_header' => true]
+        );
+
+        $this->assertSame($expectedPage, $resultPage);
+    }
+
+    /**
+     * Block height against the 785pt an empty page has room for
+     *
+     * @return array[]
+     */
+    public function blockHeightDataProvider(): array
+    {
+        return [
+            'block that fits on an empty page is moved to one' => [500, 1],
+            'block taller than an empty page stays where it is' => [900, 0],
+        ];
+    }
+
     /**
      * Generate the array for multiline block
      *