Skip to content

Commit

Permalink
Fix Several Problems in a Very Complicated Spreadsheet
Browse files Browse the repository at this point in the history 
Fix PHPOffice#3679. That issue was opened for a problem which was already solved by PR PHPOffice#3659, however there were additional problems with the spreadsheet.

The main problem is that Data Validations and Conditional Styles can each be supplied in the Xml in either "external" or "internal" formats. The code for each to handle "external" assumes that each is the only "external" item on the worksheet in the Xml, but some of the worksheets in the sample spreadsheet provide both as "external" on some sheets. The code to fix this is verified against the supplied sample, however no formal test has been added for it. The sample is much too large and complicated to be added to the test suite - it takes several minutes to read, and even longer to write (`setPreCalculateFormulas(false)` is highly recommended). I will leave this ticket open for a few days to see if I can hand-craft a suitable test case, but I am not hopeful.

A second problem is that something in the Xlsx Reader `$xmlSheetNS->sheetData->row` loop breaks the selected cell for the worksheet. This is easily fixed and verified by eye (and with the supplied sample), but, again, no explicit test case is added.

A third problem is that drawings which are part of the supplied sample use `srcRect` tags in the Xml to effectively produce a cropped version of the image. This tag has hitherto been ignored. It is now supported in Xlsx Reader, Xlsx Writer, and Worksheet/BaseDrawing object. This is again verified with the supplied sample; unlike the other parts, it was easy to add a new formal test case for this part of the fix.
  • Loading branch information
@oleibman
oleibman committed Aug 20, 2023
1 parent 4971801 commit d8bafbf
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 23 deletions.
46 changes: 29 additions & 17 deletions src/PhpSpreadsheet/Reader/Xlsx.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
->load($this->getReadFilter(), $this->getReadDataOnly());
}

$holdSelectedCells = $docSheet->getSelectedCells();
if ($xmlSheetNS && $xmlSheetNS->sheetData && $xmlSheetNS->sheetData->row) {
$cIndex = 1; // Cell Start from 1
foreach ($xmlSheetNS->sheetData->row as $row) {
Expand Down Expand Up @@ -968,6 +969,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
++$cIndex;
}
}
$docSheet->setSelectedCells($holdSelectedCells);
if ($xmlSheetNS && $xmlSheetNS->ignoredErrors) {
foreach ($xmlSheetNS->ignoredErrors->ignoredError as $ignoredErrorx) {
$ignoredError = self::testSimpleXml($ignoredErrorx);
Expand Down Expand Up @@ -1007,23 +1009,30 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
$unparsedLoadedData = (new PageSetup($docSheet, $xmlSheet))->load($unparsedLoadedData);
}

if ($xmlSheet !== false && isset($xmlSheet->extLst, $xmlSheet->extLst->ext, $xmlSheet->extLst->ext['uri']) && ($xmlSheet->extLst->ext['uri'] == '{CCE6A557-97BC-4b89-ADB6-D9C93CAAB3DF}')) {
// Create dataValidations node if does not exists, maybe is better inside the foreach ?
if (!$xmlSheet->dataValidations) {
$xmlSheet->addChild('dataValidations');
}

foreach ($xmlSheet->extLst->ext->children(Namespaces::DATA_VALIDATIONS1)->dataValidations->dataValidation as $item) {
$item = self::testSimpleXml($item);
$node = self::testSimpleXml($xmlSheet->dataValidations)->addChild('dataValidation');
foreach ($item->attributes() ?? [] as $attr) {
$node->addAttribute($attr->getName(), $attr);
if ($xmlSheet !== false && isset($xmlSheet->extLst->ext)) {
foreach ($xmlSheet->extLst->ext as $extlst) {
$extAttrs = $extlst->/** @scrutinizer ignore-call */ attributes() ?? [];
$extUri = (string) ($extAttrs['uri'] ?? '');
if ($extUri !== '{CCE6A557-97BC-4b89-ADB6-D9C93CAAB3DF}') {
continue;
}
$node->addAttribute('sqref', $item->children(Namespaces::DATA_VALIDATIONS2)->sqref);
if (isset($item->formula1)) {
$childNode = $node->addChild('formula1');
if ($childNode !== null) { // null should never happen
$childNode[0] = (string) $item->formula1->children(Namespaces::DATA_VALIDATIONS2)->f; // @phpstan-ignore-line
// Create dataValidations node if does not exists, maybe is better inside the foreach ?
if (!$xmlSheet->dataValidations) {
$xmlSheet->addChild('dataValidations');
}

foreach ($extlst->children(Namespaces::DATA_VALIDATIONS1)->dataValidations->dataValidation as $item) {
$item = self::testSimpleXml($item);
$node = self::testSimpleXml($xmlSheet->dataValidations)->addChild('dataValidation');
foreach ($item->attributes() ?? [] as $attr) {
$node->addAttribute($attr->getName(), $attr);
}
$node->addAttribute('sqref', $item->children(Namespaces::DATA_VALIDATIONS2)->sqref);
if (isset($item->formula1)) {
$childNode = $node->addChild('formula1');
if ($childNode !== null) { // null should never happen
$childNode[0] = (string) $item->formula1->children(Namespaces::DATA_VALIDATIONS2)->f; // @phpstan-ignore-line
}
}
}
}
Expand Down Expand Up @@ -1475,10 +1484,13 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
foreach ($xmlDrawingChildren->twoCellAnchor as $twoCellAnchor) {
$twoCellAnchor = self::testSimpleXml($twoCellAnchor);
if ($twoCellAnchor->pic->blipFill) {
$objDrawing = new \PhpOffice\PhpSpreadsheet\Worksheet\Drawing();
$blip = $twoCellAnchor->pic->blipFill->children(Namespaces::DRAWINGML)->blip;
if (isset($twoCellAnchor->pic->blipFill->children(Namespaces::DRAWINGML)->srcRect)) {
$objDrawing->setSrcRect($twoCellAnchor->pic->blipFill->children(Namespaces::DRAWINGML)->srcRect->attributes());
}
$xfrm = $twoCellAnchor->pic->spPr->children(Namespaces::DRAWINGML)->xfrm;
$outerShdw = $twoCellAnchor->pic->spPr->children(Namespaces::DRAWINGML)->effectLst->outerShdw;
$objDrawing = new \PhpOffice\PhpSpreadsheet\Worksheet\Drawing();
/** @scrutinizer ignore-call */
$editAs = $twoCellAnchor->attributes();
if (isset($editAs, $editAs['editAs'])) {
Expand Down
12 changes: 10 additions & 2 deletions src/PhpSpreadsheet/Reader/Xlsx/ConditionalStyles.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,17 @@ private function setConditionalsFromExt(array $conditionals): void
private function readConditionalsFromExt(SimpleXMLElement $extLst, StyleReader $styleReader): array
{
$conditionals = [];
if (!isset($extLst->ext)) {
return $conditionals;
}

if (isset($extLst->ext['uri']) && (string) $extLst->ext['uri'] === '{78C0D931-6437-407d-A8EE-F0AAD7539E65}') {
$conditionalFormattingRuleXml = $extLst->ext->children($this->ns['x14']);
foreach ($extLst->ext as $extlstcond) {
$extAttrs = $extlstcond->/** @scrutinizer ignore-call */ attributes() ?? [];
$extUri = (string) ($extAttrs['uri'] ?? '');
if ($extUri !== '{78C0D931-6437-407d-A8EE-F0AAD7539E65}') {
continue;
}
$conditionalFormattingRuleXml = $extlstcond->children($this->ns['x14']);
if (!$conditionalFormattingRuleXml->conditionalFormattings) {
return [];
}
Expand Down
22 changes: 22 additions & 0 deletions src/PhpSpreadsheet/Worksheet/BaseDrawing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PhpOffice\PhpSpreadsheet\Cell\Hyperlink;
use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
use PhpOffice\PhpSpreadsheet\IComparable;
use SimpleXMLElement;

class BaseDrawing implements IComparable
{
Expand Down Expand Up @@ -164,6 +165,9 @@ class BaseDrawing implements IComparable
*/
protected $type = IMAGETYPE_UNKNOWN;

/** @var null|SimpleXMLElement|string[] */
protected $srcRect = [];

/**
* Create a new BaseDrawing.
*/
Expand Down Expand Up @@ -532,4 +536,22 @@ public function validEditAs(): bool
{
return in_array($this->editAs, self::VALID_EDIT_AS, true);
}

/**
* @return null|SimpleXMLElement|string[]
*/
public function getSrcRect()
{
return $this->srcRect;
}

/**
* @param null|SimpleXMLElement|string[] $srcRect
*/
public function setSrcRect($srcRect): self
{
$this->srcRect = $srcRect;

return $this;
}
}
19 changes: 15 additions & 4 deletions src/PhpSpreadsheet/Writer/Xlsx/Drawing.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,21 @@ public function writeDrawing(XMLWriter $objWriter, BaseDrawing $drawing, $relati
$objWriter->writeAttribute('r:embed', 'rId' . $relationId);
$objWriter->endElement();

// a:stretch
$objWriter->startElement('a:stretch');
$objWriter->writeElement('a:fillRect', null);
$objWriter->endElement();
$srcRect = $drawing->getSrcRect();
if (!empty($srcRect)) {
$objWriter->startElement('a:srcRect');
foreach ($srcRect as $key => $value) {
$objWriter->writeAttribute($key, (string) $value);
}
$objWriter->endElement(); // a:srcRect
$objWriter->startElement('a:stretch');
$objWriter->endElement(); // a:stretch
} else {
// a:stretch
$objWriter->startElement('a:stretch');
$objWriter->writeElement('a:fillRect', null);
$objWriter->endElement();
}

$objWriter->endElement();

Expand Down
31 changes: 31 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xlsx/Issue3679ImgTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;

use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;

class Issue3679ImgTest extends AbstractFunctional
{
public function testCroppedPicture(): void
{
$file = 'tests/data/Reader/XLSX/issue.3679.img.xlsx';
$reader = new Xlsx();
$spreadsheet = $reader->load($file);

$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx');
$spreadsheet->disconnectWorksheets();
$sheet = $reloadedSpreadsheet->getActiveSheet();
$drawings = $sheet->getDrawingCollection();
self::assertCount(1, $drawings);
if ($drawings[0] === null) {
self::fail('Unexpected null drawing');
} else {
$srcRect = $drawings[0]->getSrcRect();
self::assertSame('448', (string) ($srcRect['r'] ?? ''));
self::assertSame('65769', (string) ($srcRect['b'] ?? ''));
}

$reloadedSpreadsheet->disconnectWorksheets();
}
}
Binary file added tests/data/Reader/XLSX/issue.3679.img.xlsx
View file
Binary file not shown.

0 comments on commit d8bafbf

Please sign in to comment.