Merge branch 'master' into pr1321

CHANGELOG.md

@@ -29,7 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ### Fixed
 
-- Nothing yet.
+- Better Handling of legacyDrawing Xml. [Issue #4105](https://github.com/PHPOffice/PhpSpreadsheet/issues/4105) [PR #4122](https://github.com/PHPOffice/PhpSpreadsheet/pull/4122)
 
 ## 2024-08-07 - 2.2.2
 

docs/topics/conditional-formatting.md

@@ -71,7 +71,7 @@ The Wizards know which operator types match up with condition types, and provide
 
 ---
 
-Note that `$conditionalStyles` is an array: it is possible to apply several conditions to the same range of cells. If we also wanted to highlight values that were less than 10 in the the A1:A10 range, we can add a second style rule.
+Note that `$conditionalStyles` is an array: it is possible to apply several conditions to the same range of cells. If we also wanted to highlight values that were less than 10 in the A1:A10 range, we can add a second style rule.
 
 In Excel, we would do this by selecting the range again, and going through the same process, this time selecting the "Highlight Cells Rules", then "Less Than" from the "Conditional Styles" menu, entering the value "10" in the prompt box, and selecting the appropriate style.
 

docs/topics/defined-names.md

@@ -263,7 +263,7 @@ Even though `CHARGE_RATE` references a cell on a different worksheet, because is
 
 However, a Named Range can be locally scoped so that it is only available when referenced from a specific worksheet, or it can be globally scoped. This means that you can use the same Named Range name with different values on different worksheets.
 
-Building further on our timesheet, perhaps we use a different worksheet for each client, and we use the same hourly rate when billing most of our clients; but for one particular client (perhaps doing work for a a friend) we use a lower rate.
+Building further on our timesheet, perhaps we use a different worksheet for each client, and we use the same hourly rate when billing most of our clients; but for one particular client (perhaps doing work for a friend) we use a lower rate.
 
 ```php
 $clients = [

src/PhpSpreadsheet/Calculation/Web/Service.php

@@ -29,7 +29,7 @@ public static function webService(string $url): string
             return ExcelError::VALUE(); // Invalid protocol
         }
 
-        // Get results from the the webservice
+        // Get results from the webservice
         $client = Settings::getHttpClient();
         $requestFactory = Settings::getRequestFactory();
         $request = $requestFactory->createRequest('GET', $url);

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -1643,7 +1643,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                         foreach ($xmlSheet->legacyDrawing as $drawing) {
                                             $drawingRelId = (string) self::getArrayItem(self::getAttributes($drawing, $xmlNamespaceBase), 'id');
                                             if (isset($vmlDrawingContents[$drawingRelId])) {
-                                                $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['legacyDrawing'] = $vmlDrawingContents[$drawingRelId];
+                                                if (self::onlyNoteVml($vmlDrawingContents[$drawingRelId]) === false) {
+                                                    $unparsedLoadedData['sheets'][$docSheet->getCodeName()]['legacyDrawing'] = $vmlDrawingContents[$drawingRelId];
+                                                }
                                             }
                                         }
                                     }
@@ -2391,4 +2393,36 @@ private static function storeFormulaAttributes(SimpleXMLElement $f, Worksheet $d
             $docSheet->getCell($r)->setFormulaAttributes($formulaAttributes);
         }
     }
+
+    private static function onlyNoteVml(string $data): bool
+    {
+        $data = str_replace('<br>', '<br/>', $data);
+
+        try {
+            $sxml = @simplexml_load_string($data);
+        } catch (Throwable) {
+            $sxml = false;
+        }
+
+        if ($sxml === false) {
+            return false;
+        }
+        $shapes = $sxml->children(Namespaces::URN_VML);
+        foreach ($shapes->shape as $shape) {
+            $clientData = $shape->children(Namespaces::URN_EXCEL);
+            if (!isset($clientData->ClientData)) {
+                return false;
+            }
+            $attrs = $clientData->ClientData->attributes();
+            if (!isset($attrs['ObjectType'])) {
+                return false;
+            }
+            $objectType = (string) $attrs['ObjectType'];
+            if ($objectType !== 'Note') {
+                return false;
+            }
+        }
+
+        return true;
+    }
 }

src/PhpSpreadsheet/Spreadsheet.php

@@ -1581,4 +1581,14 @@ public function getExcelCalendar(): int
     {
         return $this->excelCalendar;
     }
+
+    public function deleteLegacyDrawing(Worksheet $worksheet): void
+    {
+        unset($this->unparsedLoadedData['sheets'][$worksheet->getCodeName()]['legacyDrawing']);
+    }
+
+    public function getLegacyDrawing(Worksheet $worksheet): ?string
+    {
+        return $this->unparsedLoadedData['sheets'][$worksheet->getCodeName()]['legacyDrawing'] ?? null;
+    }
 }

tests/PhpSpreadsheetTests/Reader/Xlsx/VmlTest.php

@@ -0,0 +1,148 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PhpOffice\PhpSpreadsheet\Shared\File;
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
+use PHPUnit\Framework\TestCase;
+
+class VmlTest extends TestCase
+{
+    private string $outfile1 = '';
+
+    private string $outfile2 = '';
+
+    protected function tearDown(): void
+    {
+        if ($this->outfile1 !== '') {
+            unlink($this->outfile1);
+            $this->outfile1 = '';
+        }
+        if ($this->outfile2 !== '') {
+            unlink($this->outfile2);
+            $this->outfile2 = '';
+        }
+    }
+
+    public function testAddComments(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->getComment('A1')->getText()->createText('top left cell');
+        $writer = new XlsxWriter($spreadsheet);
+        $this->outfile1 = File::temporaryFileName();
+        $writer->save($this->outfile1);
+        $spreadsheet->disconnectWorksheets();
+
+        $reader = new XlsxReader();
+        $file = 'zip://' . $this->outfile1 . '#xl/worksheets/sheet1.xml';
+        $sheetContents = file_get_contents($file) ?: '';
+        self::assertStringContainsString('<legacyDrawing ', $sheetContents);
+        $file = 'zip://' . $this->outfile1 . '#xl/drawings/vmlDrawing1.vml';
+        $vmlContents = file_get_contents($file) ?: '';
+        $count = substr_count($vmlContents, '<v:shape ');
+        self::assertSame(1, $count);
+        $count = substr_count($vmlContents, '<x:ClientData ObjectType="Note">');
+        self::assertSame(1, $count);
+
+        $spreadsheet2 = $reader->load($this->outfile1);
+        $sheet2 = $spreadsheet2->getActiveSheet();
+        self::assertSame('top left cell', $sheet2->getComment('A1')->getText()->getPlainText());
+        self::assertNull($spreadsheet2->getLegacyDrawing($sheet2));
+        $sheet2->getComment('H1')->getText()->createText('Show me');
+        $sheet2->getComment('H2')->getText()->createText('Hide me');
+        $sheet2->getComment('H1')->setVisible(true);
+        $writer = new XlsxWriter($spreadsheet2);
+        $this->outfile2 = File::temporaryFileName();
+        $writer->save($this->outfile2);
+        $spreadsheet2->disconnectWorksheets();
+
+        $file = 'zip://' . $this->outfile2 . '#xl/worksheets/sheet1.xml';
+        $sheetContents = file_get_contents($file) ?: '';
+        self::assertStringContainsString('<legacyDrawing ', $sheetContents);
+        $file = 'zip://' . $this->outfile2 . '#xl/drawings/vmlDrawing1.vml';
+        $vmlContents = file_get_contents($file) ?: '';
+        $count = substr_count($vmlContents, '<v:shape ');
+        self::assertSame(3, $count);
+        $count = substr_count($vmlContents, '<x:ClientData ObjectType="Note">');
+        self::assertSame(3, $count);
+
+        $reader = new XlsxReader();
+        $spreadsheet3 = $reader->load($this->outfile2);
+        $sheet3 = $spreadsheet3->getActiveSheet();
+        self::assertSame('top left cell', $sheet3->getComment('A1')->getText()->getPlainText());
+        self::assertSame('Show me', $sheet3->getComment('H1')->getText()->getPlainText());
+        self::assertSame('Hide me', $sheet3->getComment('H2')->getText()->getPlainText());
+        self::assertNull($spreadsheet3->getLegacyDrawing($sheet3));
+        self::assertTrue($sheet3->getComment('H1')->getVisible());
+        self::assertFalse($sheet3->getComment('H2')->getVisible());
+        $spreadsheet3->disconnectWorksheets();
+    }
+
+    public function testDeleteNullLegacy(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet = $spreadsheet->getActiveSheet();
+        $sheet->getComment('A1')->getText()->createText('top left cell');
+        self::assertNull($spreadsheet->getLegacyDrawing($sheet));
+        $spreadsheet->deleteLegacyDrawing($sheet);
+        $writer = new XlsxWriter($spreadsheet);
+        $this->outfile1 = File::temporaryFileName();
+        $writer->save($this->outfile1);
+        $spreadsheet->disconnectWorksheets();
+
+        $reader = new XlsxReader();
+        $file = 'zip://' . $this->outfile1 . '#xl/worksheets/sheet1.xml';
+        $sheetContents = file_get_contents($file) ?: '';
+        self::assertStringContainsString('<legacyDrawing ', $sheetContents);
+        $file = 'zip://' . $this->outfile1 . '#xl/drawings/vmlDrawing1.vml';
+        $vmlContents = file_get_contents($file) ?: '';
+        $count = substr_count($vmlContents, '<v:shape ');
+        self::assertSame(1, $count);
+        $count = substr_count($vmlContents, '<x:ClientData ObjectType="Note">');
+        self::assertSame(1, $count);
+
+        $spreadsheet2 = $reader->load($this->outfile1);
+        $sheet2 = $spreadsheet2->getActiveSheet();
+        self::assertSame('top left cell', $sheet2->getComment('A1')->getText()->getPlainText());
+        $spreadsheet2->disconnectWorksheets();
+    }
+
+    public function testAddCommentDeleteFormControls(): void
+    {
+        $infile = 'samples/Reader2/sampleData/formscomments.xlsx';
+        $reader = new XlsxReader();
+        $reader->setLoadSheetsOnly('FormsComments');
+        $spreadsheet = $reader->load($infile);
+        self::assertTrue(true);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame('row1', $sheet->getCell('H1')->getValue());
+        self::assertStringContainsString('Hello', $sheet->getComment('F1')->getText()->getPlainText());
+        $vmlContents = $spreadsheet->getLegacyDrawing($sheet) ?? '';
+        $count = substr_count($vmlContents, '<v:shape ');
+        self::assertSame(4, $count);
+        $count = substr_count($vmlContents, '<x:ClientData ');
+        self::assertSame(4, $count);
+        $count = substr_count($vmlContents, '<x:ClientData ObjectType="Note"');
+        self::assertSame(1, $count);
+        $spreadsheet->deleteLegacyDrawing($sheet);
+        $sheet->getComment('F2')->getText()->createText('Goodbye');
+        $writer = new XlsxWriter($spreadsheet);
+        $this->outfile1 = File::temporaryFileName();
+        $writer->save($this->outfile1);
+        $spreadsheet->disconnectWorksheets();
+
+        $reader2 = new XlsxReader();
+        $spreadsheet2 = $reader2->load($this->outfile1);
+        $sheet2 = $spreadsheet2->getActiveSheet();
+        self::assertNull($spreadsheet2->getLegacyDrawing($sheet2));
+        self::assertSame('row1', $sheet2->getCell('H1')->getValue());
+        self::assertStringContainsString('Hello', $sheet2->getComment('F1')->getText()->getPlainText());
+        self::assertStringContainsString('Goodbye', $sheet2->getComment('F2')->getText()->getPlainText());
+        $spreadsheet2->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Reader/Xml/DataValidationsTest.php

@@ -92,7 +92,7 @@ private function validationAssertions(): array
             'Float between 2 and 5: 7' => [false, 'G1', 7],
             'Integer not between -5 and 5: 3' => [false, 'F2', 3],
             'Integer not between -5 and 5: -1' => [false, 'F2', -1],
-            'Integer not not between -5 and 5: 7' => [true, 'F2', 7],
+            'Integer not between -5 and 5: 7' => [true, 'F2', 7],
             'Any integer except 7: -1' => [true, 'F3', -1],
             'Any integer except 7: 7' => [false, 'F3', 7],
             'Only -3: -1' => [false, 'F4', -1],