Address Some Chart Problems (#3771)

* Address Some Chart Problems

Fix #3767. The basic problem in that issue was user error, but it exposed a couple of problems in code which are being addressed.

When a spreadsheet with charts is loaded without the includeCharts option and then saved, a corrupt spreadsheet is created. I believe this has been the case for quite some time. Nothing in the test suite covers this scenario. It is, in fact, a difficult thing to test, since the problem is exposed only when the file is opened through Excel. The specific problem is that a rels file generated by the output writer continues to refer to the drawing file which described the chart, but that file is not included (and not needed) in the output spreadsheet. The resolution is kludgey. The information that the file will not be needed is not available when the rels file is written. But, when it comes time to write the drawing file, it is known whether the rels file included it. So, if nothing else has caused the file to be generated, it is written out as a basically empty xml file after all. This solves the problem, but I will continue to search for a less kludgey solution. This solution is, at least, testable; if a different solution is applied later on, the test being introduced here is likely to break so a new one will be needed.

When the provided spreadsheet is loaded with the includeCharts option and then saved, an error is exposed in processing the Chart Title caption when it doesn't exist. The change to Writer/Xlsx/Chart is simple and clearly justifiable. What is peculiar is that the error does not arise with release 1.29, but does arise with master. It is not at all clear to me what has changed since the release to expose the error - the code in question certainly hasn't changed. It is difficult to isolate changes because of the extensive number of changes following on the elimination of Php7.4 as a supported platform.

The provided spreadsheet is unusual in at least two senses. When opened in Excel, it will show a clearly default value for the chart title, namely 'Chart Title'. I cannot find anything in the xml corresponding to that text. Since I have no idea why Excel is using that title, I will not try to duplicate its behavior, so that loading and saving the provided spreadsheet will omit the chart title. I will continue to investigate.

The other sense in which it is unusual is that it includes some style files in the same directory as the chart. I doubt that PhpSpreadsheet looks at these. The styling after load and save seems to mostly match the original, although there is at least one color in the graph which does not match. I imagine it would be pretty complicated to formally support these files.

* Unused Assignment

Author: oleibman

src/PhpSpreadsheet/Writer/Xlsx.php

@@ -410,6 +410,9 @@ public function save($filename, int $flags = 0): void
                     }
                 }
             }
+            if (isset($unparsedLoadedData['sheets'][$sheetCodeName]['drawingOriginalIds']) && !isset($zipContent['xl/drawings/drawing' . ($i + 1) . '.xml'])) {
+                $zipContent['xl/drawings/drawing' . ($i + 1) . '.xml'] = '<xml></xml>';
+            }
 
             // Add comment relationship parts
             $legacy = $unparsedLoadedData['sheets'][$this->spreadSheet->getSheet($i)->getCodeName()]['legacyDrawing'] ?? null;

src/PhpSpreadsheet/Writer/Xlsx/Chart.php

@@ -169,8 +169,8 @@ private function writeTitle(XMLWriter $objWriter, ?Title $title = null): void
         $objWriter->endElement();
 
         $caption = $title->getCaption();
-        if ((is_array($caption)) && (count($caption) > 0)) {
-            $caption = $caption[0];
+        if (is_array($caption)) {
+            $caption = $caption[0] ?? '';
         }
         $this->getParentWriter()->getWriterPartstringtable()->writeRichTextForCharts($objWriter, $caption, 'a');
 

tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3767Test.php

@@ -0,0 +1,88 @@
+<?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\Writer\Xlsx as XlsxWriter;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+class Issue3767Test extends AbstractFunctional
+{
+    // some weirdness with this file, including the fact that it has a
+    // title ('Chart Title') which I cannot find anywhere in the xml.
+    private static string $testbook = 'tests/data/Reader/XLSX/issue.3767.xlsx';
+
+    private string $tempfile = '';
+
+    protected function tearDown(): void
+    {
+        if ($this->tempfile !== '') {
+            unlink($this->tempfile);
+            $this->tempfile = '';
+        }
+    }
+
+    public function readCharts(XlsxReader $reader): void
+    {
+        $reader->setIncludeCharts(true);
+    }
+
+    public function writeCharts(XlsxWriter $writer): void
+    {
+        $writer->setIncludeCharts(true);
+    }
+
+    public function testReadWithoutCharts(): void
+    {
+        $reader = new XlsxReader();
+        //$this->readCharts($reader); // Commented out - don't want to read charts.
+        $spreadsheet = $reader->load(self::$testbook);
+        $sheet = $spreadsheet->getActiveSheet();
+        $charts = $sheet->getChartCollection();
+        self::assertCount(0, $charts);
+        $this->tempfile = File::temporaryFileName();
+        $writer = new XlsxWriter($spreadsheet);
+        $this->writeCharts($writer);
+        $writer->save($this->tempfile);
+        $spreadsheet->disconnectWorksheets();
+        $file = 'zip://';
+        $file .= $this->tempfile;
+        $file .= '#xl/worksheets/_rels/sheet1.xml.rels';
+        $data = (string) file_get_contents($file);
+        // PhpSpreadsheet still generates this target even though charts aren't included
+        self::assertStringContainsString('Target="../drawings/drawing1.xml"', $data);
+        $file = 'zip://';
+        $file .= $this->tempfile;
+        $file .= '#xl/drawings/drawing1.xml';
+        $data = file_get_contents($file);
+        self::assertSame('<xml></xml>', $data); // fake file because rels needs it
+    }
+
+    public function testReadWithCharts(): void
+    {
+        $reader = new XlsxReader();
+        $this->readCharts($reader);
+        $spreadsheet = $reader->load(self::$testbook);
+        $xsheet = $spreadsheet->getActiveSheet();
+        $xcharts = $xsheet->getChartCollection();
+        self::assertCount(1, $xcharts);
+        /** @var callable */
+        $callableReader = [$this, 'readCharts'];
+        /** @var callable */
+        $callableWriter = [$this, 'writeCharts'];
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx', $callableReader, $callableWriter);
+        $spreadsheet->disconnectWorksheets();
+        $sheet = $reloadedSpreadsheet->getActiveSheet();
+        $charts = $xsheet->getChartCollection();
+        self::assertCount(1, $charts);
+        // In Excel, a default title ('Chart Title') is shown.
+        // I can't find that anywhere in the Xml.
+        self::assertSame('', $charts[0]?->getTitle()?->getCaptionText());
+        // Just test anything on the chart.
+        self::assertSame($sheet->getCell('B2')->getValue(), $charts[0]->getPlotArea()?->getPlotGroup()[0]->getPlotValues()[0]->getDataValues()[0]);
+        $reloadedSpreadsheet->disconnectWorksheets();
+    }
+}