Xlsx Chart Reader and Writer Mishandle Explosion Value (#2928)

Fix #2506. Reader only tests if Explosion is set without capturing its value. Writer hard-codes value when it is set.

Author: oleibman

src/PhpSpreadsheet/Reader/Xlsx/Chart.php

@@ -228,7 +228,7 @@ public function readChart(SimpleXMLElement $chartElements, $chartName)
                                         case 'doughnutChart':
                                         case 'pieChart':
                                         case 'pie3DChart':
-                                            $explosion = isset($chartDetail->ser->explosion);
+                                            $explosion = self::getAttribute($chartDetail->ser->explosion, 'val', 'string');
                                             $plotSer = $this->chartDataSeries($chartDetail, $chartDetailKey);
                                             $plotSer->setPlotStyle("$explosion");
                                             $plotSeries[] = $plotSer;

src/PhpSpreadsheet/Writer/Xlsx/Chart.php

@@ -875,10 +875,6 @@ private function writePlotSeriesValuesElement(XMLWriter $objWriter, int $val, ?C
         $objWriter->writeAttribute('val', "$val");
         $objWriter->endElement(); // c:idx
 
-        $objWriter->startElement('c:bubble3D');
-        $objWriter->writeAttribute('val', '0');
-        $objWriter->endElement(); // c:bubble3D
-
         $objWriter->startElement('c:spPr');
         $this->writeColor($objWriter, $fillColor);
         $objWriter->endElement(); // c:spPr
@@ -1052,13 +1048,11 @@ private function writePlotGroup(?DataSeries $plotGroup, string $groupType, XMLWr
                 $catIsMultiLevelSeries = $catIsMultiLevelSeries || $plotSeriesCategory->isMultiLevelSeries();
 
                 if (($groupType == DataSeries::TYPE_PIECHART) || ($groupType == DataSeries::TYPE_PIECHART_3D) || ($groupType == DataSeries::TYPE_DONUTCHART)) {
-                    if ($plotGroup->getPlotStyle() !== null) {
-                        $plotStyle = $plotGroup->getPlotStyle();
-                        if ($plotStyle) {
-                            $objWriter->startElement('c:explosion');
-                            $objWriter->writeAttribute('val', '25');
-                            $objWriter->endElement();
-                        }
+                    $plotStyle = $plotGroup->getPlotStyle();
+                    if (is_numeric($plotStyle)) {
+                        $objWriter->startElement('c:explosion');
+                        $objWriter->writeAttribute('val', $plotStyle);
+                        $objWriter->endElement();
                     }
                 }
 

tests/PhpSpreadsheetTests/Chart/Issue2506Test.php

@@ -0,0 +1,73 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Chart;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
+use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+class Issue2506Test extends AbstractFunctional
+{
+    private const DIRECTORY = 'tests' . DIRECTORY_SEPARATOR . 'data' . DIRECTORY_SEPARATOR . 'Reader' . DIRECTORY_SEPARATOR . 'XLSX' . DIRECTORY_SEPARATOR;
+
+    public function readCharts(XlsxReader $reader): void
+    {
+        $reader->setIncludeCharts(true);
+    }
+
+    public function writeCharts(XlsxWriter $writer): void
+    {
+        $writer->setIncludeCharts(true);
+    }
+
+    public function testDataSeriesValues(): void
+    {
+        $reader = new XlsxReader();
+        self::readCharts($reader);
+        $spreadsheet = $reader->load(self::DIRECTORY . 'issue.2506.xlsx');
+        $worksheet = $spreadsheet->getActiveSheet();
+        $charts = $worksheet->getChartCollection();
+        self::assertCount(4, $charts);
+        $originalChart1 = $charts[0];
+        self::assertNotNull($originalChart1);
+        $originalPlotArea1 = $originalChart1->getPlotArea();
+        self::assertNotNull($originalPlotArea1);
+        $originalPlotSeries1 = $originalPlotArea1->getPlotGroup();
+        self::assertCount(1, $originalPlotSeries1);
+        self::assertSame('0', $originalPlotSeries1[0]->getPlotStyle());
+        $originalChart2 = $charts[1];
+        self::assertNotNull($originalChart2);
+        $originalPlotArea2 = $originalChart2->getPlotArea();
+        self::assertNotNull($originalPlotArea2);
+        $originalPlotSeries2 = $originalPlotArea2->getPlotGroup();
+        self::assertCount(1, $originalPlotSeries2);
+        self::assertSame('5', $originalPlotSeries2[0]->getPlotStyle());
+
+        /** @var callable */
+        $callableReader = [$this, 'readCharts'];
+        /** @var callable */
+        $callableWriter = [$this, 'writeCharts'];
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx', $callableReader, $callableWriter);
+        $spreadsheet->disconnectWorksheets();
+
+        $sheet = $reloadedSpreadsheet->getActiveSheet();
+        $charts2 = $sheet->getChartCollection();
+        self::assertCount(4, $charts2);
+        $chart1 = $charts[0];
+        self::assertNotNull($chart1);
+        $plotArea1 = $chart1->getPlotArea();
+        self::assertNotNull($plotArea1);
+        $plotSeries1 = $plotArea1->getPlotGroup();
+        self::assertCount(1, $plotSeries1);
+        self::assertSame('0', $plotSeries1[0]->getPlotStyle());
+        $chart2 = $charts[1];
+        self::assertNotNull($chart2);
+        $plotArea2 = $chart2->getPlotArea();
+        self::assertNotNull($plotArea2);
+        $plotSeries2 = $plotArea2->getPlotGroup();
+        self::assertCount(1, $plotSeries2);
+        self::assertSame('5', $plotSeries2[0]->getPlotStyle());
+
+        $reloadedSpreadsheet->disconnectWorksheets();
+    }
+}