Merge pull request #4244 from oleibman/issue4241

Fix Minor Break Handling Drawings
PHPOffice · Nov 30, 2024 · a069960 · a069960
2 parents d4ffa3e + 8efb0fc
commit a069960
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ### Fixed
 
+- Fix minor break handling drawings. [Issue #4241](https://github.com/PHPOffice/PhpSpreadsheet/issues/4241) [PR #4244](https://github.com/PHPOffice/PhpSpreadsheet/pull/4244)
 - Ignore cell formatting when the format is a single @. [Issue #4242](https://github.com/PHPOffice/PhpSpreadsheet/issues/4242) [PR #4243](https://github.com/PHPOffice/PhpSpreadsheet/pull/4243)
 
 ## 2024-11-22 - 3.5.0

diff --git a/samples/Basic4/53_ImageOpacity.php b/samples/Basic4/53_ImageOpacity.php
@@ -5,9 +5,6 @@
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
 
-//var_dump(realpath(__DIR__ . '/../images/blue_square.png'));
-//exit();
-
 $path = __DIR__ . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . 'images/blue_square.png';
 $spreadsheet = new Spreadsheet();
 $spreadsheet->getProperties()->setTitle('53_ImageOpacity');
@@ -33,12 +30,12 @@
 $drawing->setWorksheet($sheet);
 
 $drawing = new Drawing();
+$drawing->setWorksheet($sheet);
 $drawing->setName('Blue Square opacity 60%');
 $drawing->setPath($path);
 $drawing->setCoordinates('E1');
 $drawing->setCoordinates2('F5');
 $drawing->setOpacity(60000);
-$drawing->setWorksheet($sheet);
 
 $drawing = new Drawing();
 $drawing->setName('Blue Square opacity 40%');
@@ -57,12 +54,12 @@
 $drawing->setWorksheet($sheet);
 
 $drawing = new Drawing();
+$drawing->setWorksheet($sheet);
 $drawing->setName('Blue Square opacity 0%');
 $drawing->setPath($path);
 $drawing->setCoordinates('E8');
 $drawing->setCoordinates2('F12');
 $drawing->setOpacity(0);
-$drawing->setWorksheet($sheet);
 
 // Save
 $helper->write($spreadsheet, __FILE__, ['Xlsx', 'Html', 'Dompdf', 'Mpdf']);

diff --git a/src/PhpSpreadsheet/Worksheet/BaseDrawing.php b/src/PhpSpreadsheet/Worksheet/BaseDrawing.php
@@ -197,15 +197,18 @@ public function getWorksheet(): ?Worksheet
     public function setWorksheet(?Worksheet $worksheet = null, bool $overrideOld = false): self
     {
         if ($this->worksheet === null) {
-            // Add drawing to \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet
-            if ($worksheet !== null && !($this instanceof Drawing && $this->getPath() === '')) {
+            // Add drawing to Worksheet
+            if ($worksheet !== null) {
                 $this->worksheet = $worksheet;
-                $this->worksheet->getCell($this->coordinates);
-                $this->worksheet->getDrawingCollection()->append($this);
+                if (!($this instanceof Drawing && $this->getPath() === '')) {
+                    $this->worksheet->getCell($this->coordinates);
+                }
+                $this->worksheet->getDrawingCollection()
+                    ->append($this);
             }
         } else {
             if ($overrideOld) {
-                // Remove drawing from old \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet
+                // Remove drawing from old Worksheet
                 $iterator = $this->worksheet->getDrawingCollection()->getIterator();
 
                 while ($iterator->valid()) {
@@ -217,10 +220,10 @@ public function setWorksheet(?Worksheet $worksheet = null, bool $overrideOld = f
                     }
                 }
 
-                // Set new \PhpOffice\PhpSpreadsheet\Worksheet\Worksheet
+                // Set new Worksheet
                 $this->setWorksheet($worksheet);
             } else {
-                throw new PhpSpreadsheetException('A Worksheet has already been assigned. Drawings can only exist on one \\PhpOffice\\PhpSpreadsheet\\Worksheet.');
+                throw new PhpSpreadsheetException('A Worksheet has already been assigned. Drawings can only exist on one Worksheet.');
             }
         }
 
@@ -235,6 +238,11 @@ public function getCoordinates(): string
     public function setCoordinates(string $coordinates): self
     {
         $this->coordinates = $coordinates;
+        if ($this->worksheet !== null) {
+            if (!($this instanceof Drawing && $this->getPath() === '')) {
+                $this->worksheet->getCell($this->coordinates);
+            }
+        }
 
         return $this;
     }

diff --git a/src/PhpSpreadsheet/Worksheet/Drawing.php b/src/PhpSpreadsheet/Worksheet/Drawing.php
@@ -148,6 +148,12 @@ public function setPath(string $path, bool $verifyFile = true, ?ZipArchive $zip
             throw new PhpSpreadsheetException("File $path not found!");
         }
 
+        if ($this->worksheet !== null) {
+            if ($this->path !== '') {
+                $this->worksheet->getCell($this->coordinates);
+            }
+        }
+
         return $this;
     }
 

diff --git a/tests/PhpSpreadsheetTests/Worksheet/Issue4241Test.php b/tests/PhpSpreadsheetTests/Worksheet/Issue4241Test.php
@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Worksheet\Drawing;
+use PHPUnit\Framework\TestCase;
+
+class Issue4241Test extends TestCase
+{
+    public function testIssue4241(): void
+    {
+        // setWorksheet needed to come after setPath
+        $badPath = 'tests/data/Writer/XLSX/xgreen_square.gif';
+        $goodPath = 'tests/data/Writer/XLSX/green_square.gif';
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $drawing = new Drawing();
+        $drawing->setName('Green Square');
+        $drawing->setWorksheet($sheet);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        $drawing0 = $drawings[0];
+        self::assertInstanceOf(Drawing::class, $drawing0);
+        self::assertSame('', $drawing0->getPath());
+        self::assertSame('A1', $drawing0->getCoordinates());
+        $maxRow = $sheet->getHighestDataRow();
+        $maxCol = $sheet->getHighestDataColumn();
+        self::assertSame(1, $maxRow);
+        self::assertSame('A', $maxCol);
+
+        $drawing->setCoordinates('E5');
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        $drawing0 = $drawings[0];
+        self::assertInstanceOf(Drawing::class, $drawing0);
+        self::assertSame('', $drawing0->getPath());
+        self::assertSame('E5', $drawing0->getCoordinates());
+        $maxRow = $sheet->getHighestDataRow();
+        $maxCol = $sheet->getHighestDataColumn();
+        self::assertSame(1, $maxRow);
+        self::assertSame('A', $maxCol);
+
+        $drawing->setPath($badPath, false);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        $drawing0 = $drawings[0];
+        self::assertInstanceOf(Drawing::class, $drawing0);
+        self::assertSame('', $drawing0->getPath());
+        self::assertSame('E5', $drawing0->getCoordinates());
+        $maxRow = $sheet->getHighestDataRow();
+        $maxCol = $sheet->getHighestDataColumn();
+        self::assertSame(1, $maxRow);
+        self::assertSame('A', $maxCol);
+
+        $drawing->setPath($goodPath);
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        $drawing0 = $drawings[0];
+        self::assertInstanceOf(Drawing::class, $drawing0);
+        self::assertSame($goodPath, $drawing0->getPath());
+        self::assertSame('E5', $drawing0->getCoordinates());
+        $maxRow = $sheet->getHighestDataRow();
+        $maxCol = $sheet->getHighestDataColumn();
+        self::assertSame(5, $maxRow);
+        self::assertSame('E', $maxCol);
+
+        $drawing->setCoordinates('G3');
+        $drawings = $sheet->getDrawingCollection();
+        self::assertCount(1, $drawings);
+        $drawing0 = $drawings[0];
+        self::assertInstanceOf(Drawing::class, $drawing0);
+        self::assertSame($goodPath, $drawing0->getPath());
+        self::assertSame('G3', $drawing0->getCoordinates());
+        $maxRow = $sheet->getHighestDataRow();
+        $maxCol = $sheet->getHighestDataColumn();
+        self::assertSame(5, $maxRow);
+        self::assertSame('G', $maxCol);
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}