From 0c93bbaaa31199f09b7cb79a9d3c821f90a916e2 Mon Sep 17 00:00:00 2001 From: Tiago Malheiro Date: Tue, 16 Mar 2021 16:25:25 +0000 Subject: [PATCH] Don't corrupt file when using chart with fill color When the fill color property of `DataSeries.plotLabel` using a DataSeriesValues on a line chart is set, the XLSX file written is corrupted, and MSExcel2016 removes the drawing1.xml if forced open. This problem was already documented on issue #589 along with a possible solution. So all credits go to @madrussa. I am only submitting the PR. Fixes #589 Closes #1930 --- src/PhpSpreadsheet/Writer/Xlsx/Chart.php | 11 +- .../Writer/Xlsx/Issue589Test.php | 162 ++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/Issue589Test.php diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Chart.php b/src/PhpSpreadsheet/Writer/Xlsx/Chart.php index db82f8bc9e..23b78a2f2f 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Chart.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Chart.php @@ -1058,7 +1058,7 @@ private function writePlotGroup(?DataSeries $plotGroup, $groupType, XMLWriter $o $objWriter->startElement('c:ser'); $plotLabel = $plotGroup->getPlotLabelByIndex($plotSeriesIdx); - if ($plotLabel) { + if ($plotLabel && $groupType !== DataSeries::TYPE_LINECHART) { $fillColor = $plotLabel->getFillColor(); if ($fillColor !== null && !is_array($fillColor)) { $objWriter->startElement('c:spPr'); @@ -1116,6 +1116,15 @@ private function writePlotGroup(?DataSeries $plotGroup, $groupType, XMLWriter $o if ($groupType == DataSeries::TYPE_STOCKCHART) { $objWriter->startElement('a:noFill'); $objWriter->endElement(); + } elseif ($plotLabel) { + $fillColor = $plotLabel->getFillColor(); + if (is_string($fillColor)) { + $objWriter->startElement('a:solidFill'); + $objWriter->startElement('a:srgbClr'); + $objWriter->writeAttribute('val', $fillColor); + $objWriter->endElement(); + $objWriter->endElement(); + } } $objWriter->endElement(); $objWriter->endElement(); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue589Test.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue589Test.php new file mode 100644 index 0000000000..82478fb93d --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue589Test.php @@ -0,0 +1,162 @@ +getActiveSheet(); + $worksheet->fromArray( + [ + [2010], + [12], + [56], + ] + ); + + $dataSeriesLabels = [ + new DataSeriesValues(DataSeriesValues::DATASERIES_TYPE_STRING, 'Worksheet!$A$1', null, 1), + ]; + $dataSeriesLabels[0]->setFillColor($color); + $dataSeriesValues = [ + new DataSeriesValues(DataSeriesValues::DATASERIES_TYPE_NUMBER, 'Worksheet!$A$2:$A$3', null, 2), + ]; + + // Build the dataseries + $series = new DataSeries( + DataSeries::TYPE_LINECHART, + DataSeries::GROUPING_STACKED, + range(0, count($dataSeriesValues) - 1), + $dataSeriesLabels, + [], + $dataSeriesValues + ); + + // Set the series in the plot area + $plotArea = new PlotArea(null, [$series]); + + // Create the chart + $chart = new Chart( + 'chart1', + null, + null, + $plotArea + ); + + // Add the chart to the worksheet + $worksheet->addChart($chart); + + return $spreadsheet; + } + + public function testLineChartFill(): void + { + $outputFilename = File::temporaryFilename(); + $spreadsheet = $this->buildChartSpreadsheet('98B954'); + $writer = new Writer($spreadsheet); + $writer->setIncludeCharts(true); + $writer->save($outputFilename); + + $zip = new ZipArchive(); + $zip->open($outputFilename); + $resultChart1Raw = $zip->getFromName('xl/charts/chart1.xml'); + $zip->close(); + unlink($outputFilename); + + $dom = new DOMDocument(); + if ($resultChart1Raw === false) { + self::fail('Unable to open the chart file'); + } else { + $loaded = $dom->loadXML($resultChart1Raw); + if (!$loaded) { + self::fail('Unable to load the chart xml'); + } else { + $series = $dom->getElementsByTagName('ser'); + $firstSeries = $series->item(0); + if ($firstSeries === null) { + self::fail('The chart XML does not contain a \'ser\' tag!'); + } else { + $spPrList = $firstSeries->getElementsByTagName('spPr'); + + // expect to see only one element with name 'c:spPr' + self::assertCount(1, $spPrList); + + /** @var DOMNode $node */ + $node = $spPrList->item(0); // Get the spPr element + $actualXml = $dom->saveXML($node); + if ($actualXml === false) { + self::fail('Failure saving the spPr element as xml string!'); + } else { + self::assertXmlStringEqualsXmlString('', $actualXml); + } + } + } + } + } + + public function testLineChartFillIgnoresColorArray(): void + { + $outputFilename = File::temporaryFilename(); + $spreadsheet = $this->buildChartSpreadsheet(['98B954']); + $writer = new Writer($spreadsheet); + $writer->setIncludeCharts(true); + $writer->save($outputFilename); + + $zip = new ZipArchive(); + $zip->open($outputFilename); + $resultChart1Raw = $zip->getFromName('xl/charts/chart1.xml'); + $zip->close(); + unlink($outputFilename); + + $dom = new DOMDocument(); + if ($resultChart1Raw === false) { + self::fail('Unable to open the chart file'); + } else { + $loaded = $dom->loadXML($resultChart1Raw); + if (!$loaded) { + self::fail('Unable to load the chart xml'); + } else { + $series = $dom->getElementsByTagName('ser'); + $firstSeries = $series->item(0); + if ($firstSeries === null) { + self::fail('The chart XML does not contain a \'ser\' tag!'); + } else { + $spPrList = $firstSeries->getElementsByTagName('spPr'); + + // expect to see only one element with name 'c:spPr' + self::assertCount(1, $spPrList); + + /** @var DOMNode $node */ + $node = $spPrList->item(0); // Get the spPr element + $actualXml = $dom->saveXML($node); + if ($actualXml === false) { + self::fail('Failure saving the spPr element as xml string!'); + } else { + self::assertXmlStringEqualsXmlString('', $actualXml); + } + } + } + } + } +}