Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fill Patterns/Colors When Xml Attributes are Missing #4250

Merged
merged 2 commits into from
Dec 6, 2024
Merged

Fill Patterns/Colors When Xml Attributes are Missing #4250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd69858
Fill Patterns/Colors When Xml Attributes are Missing
oleibman Nov 30, 2024
eeef6dc
Scrutinizer
oleibman Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/PhpSpreadsheet/Reader/Xlsx/Styles.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,22 @@ public function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyleXml):
$fillStyle->getStartColor()->setARGB($this->readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=0]'))->color)); //* @phpstan-ignore-line
$fillStyle->getEndColor()->setARGB($this->readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=1]'))->color)); //* @phpstan-ignore-line
} elseif ($fillStyleXml->patternFill) {
$defaultFillStyle = Fill::FILL_NONE;
$defaultFillStyle = ($fillStyle->getFillType() !== null) ? Fill::FILL_NONE : '';
$fgFound = false;
$bgFound = false;
if ($fillStyleXml->patternFill->fgColor) {
$fillStyle->getStartColor()->setARGB($this->readColor($fillStyleXml->patternFill->fgColor, true));
$defaultFillStyle = Fill::FILL_SOLID;
if ($fillStyle->getFillType() !== null) {
$defaultFillStyle = Fill::FILL_SOLID;
}
$fgFound = true;
}
if ($fillStyleXml->patternFill->bgColor) {
$fillStyle->getEndColor()->setARGB($this->readColor($fillStyleXml->patternFill->bgColor, true));
$defaultFillStyle = Fill::FILL_SOLID;
if ($fillStyle->getFillType() !== null) {
$defaultFillStyle = Fill::FILL_SOLID;
}
$bgFound = true;
}

$type = '';
Expand All @@ -169,6 +177,22 @@ public function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyleXml):
$patternType = ($type === '') ? $defaultFillStyle : $type;

$fillStyle->setFillType($patternType);
if (
!$fgFound // no foreground color specified
&& !in_array($patternType, [Fill::FILL_NONE, Fill::FILL_SOLID], true) // these patterns aren't relevant
&& $fillStyle->getStartColor()->getARGB() // not conditional
) {
$fillStyle->getStartColor()
->setARGB('', true);
}
if (
!$bgFound // no background color specified
&& !in_array($patternType, [Fill::FILL_NONE, Fill::FILL_SOLID], true) // these patterns aren't relevant
&& $fillStyle->getEndColor()->getARGB() // not conditional
) {
$fillStyle->getEndColor()
->setARGB('', true);
}
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/PhpSpreadsheet/Style/Color.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,14 @@ public function getARGB(): ?string
*
* @return $this
*/
public function setARGB(?string $colorValue = self::COLOR_BLACK): static
public function setARGB(?string $colorValue = self::COLOR_BLACK, bool $nullStringOkay = false): static
{
$this->hasChanged = true;
$colorValue = $this->validateColor($colorValue);
if ($colorValue === '') {
return $this;
if (!$nullStringOkay || $colorValue !== '') {
$colorValue = $this->validateColor($colorValue);
if ($colorValue === '') {
return $this;
}
}

if ($this->isSupervisor) {
Expand Down
13 changes: 10 additions & 3 deletions src/PhpSpreadsheet/Writer/Html.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,9 +1139,16 @@ private function createCSSStyleFill(Fill $fill): array

// Create CSS
if ($fill->getFillType() !== Fill::FILL_NONE) {
$value = $fill->getFillType() == Fill::FILL_NONE
? 'white' : '#' . $fill->getStartColor()->getRGB();
$css['background-color'] = $value;
if (
(in_array($fill->getFillType(), ['', Fill::FILL_SOLID], true) || !$fill->getEndColor()->getRGB())
&& $fill->getStartColor()->getRGB()
) {
$value = '#' . $fill->getStartColor()->getRGB();
$css['background-color'] = $value;
} elseif ($fill->getEndColor()->getRGB()) {
$value = '#' . $fill->getEndColor()->getRGB();
$css['background-color'] = $value;
}
}

return $css;
Expand Down
16 changes: 14 additions & 2 deletions src/PhpSpreadsheet/Writer/Xls/Workbook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,20 @@ public function addXfWriter(Style $style, bool $isStyleXf = false): int
$xfWriter->setFontIndex($fontIndex);

// Background colors, best to treat these after the font so black will come after white in custom palette
$xfWriter->setFgColor($this->addColor($style->getFill()->getStartColor()->getRGB()));
$xfWriter->setBgColor($this->addColor($style->getFill()->getEndColor()->getRGB()));
if ($style->getFill()->getStartColor()->getRGB()) {
$xfWriter->setFgColor(
$this->addColor(
$style->getFill()->getStartColor()->getRGB()
)
);
}
if ($style->getFill()->getEndColor()->getRGB()) {
$xfWriter->setBgColor(
$this->addColor(
$style->getFill()->getEndColor()->getRGB()
)
);
}
$xfWriter->setBottomColor($this->addColor($style->getBorders()->getBottom()->getColor()->getRGB()));
$xfWriter->setTopColor($this->addColor($style->getBorders()->getTop()->getColor()->getRGB()));
$xfWriter->setRightColor($this->addColor($style->getBorders()->getRight()->getColor()->getRGB()));
Expand Down
6 changes: 3 additions & 3 deletions src/PhpSpreadsheet/Writer/Xls/Worksheet.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2875,9 +2875,9 @@ private function writeCFRule(
$bFormatBorder = 0;
}
// Pattern
$bFillStyle = ($conditional->getStyle()->getFill()->getFillType() === null ? 0 : 1);
$bFillColor = ($conditional->getStyle()->getFill()->getStartColor()->getARGB() === null ? 0 : 1);
$bFillColorBg = ($conditional->getStyle()->getFill()->getEndColor()->getARGB() === null ? 0 : 1);
$bFillStyle = $conditional->getStyle()->getFill()->getFillType() ? 1 : 0;
$bFillColor = $conditional->getStyle()->getFill()->getStartColor()->getARGB() ? 1 : 0;
$bFillColorBg = $conditional->getStyle()->getFill()->getEndColor()->getARGB() ? 1 : 0;
if ($bFillStyle == 1 || $bFillColor == 1 || $bFillColorBg == 1) {
$bFormatFill = 1;
} else {
Expand Down
8 changes: 5 additions & 3 deletions src/PhpSpreadsheet/Writer/Xlsx/Style.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private function writeGradientFill(XMLWriter $objWriter, Fill $fill): void
$objWriter->writeAttribute('position', '0');

// color
if ($fill->getStartColor()->getARGB() !== null) {
if (!empty($fill->getStartColor()->getARGB())) {
$objWriter->startElement('color');
$objWriter->writeAttribute('rgb', $fill->getStartColor()->getARGB());
$objWriter->endElement();
Expand All @@ -212,7 +212,7 @@ private function writeGradientFill(XMLWriter $objWriter, Fill $fill): void
$objWriter->writeAttribute('position', '1');

// color
if ($fill->getEndColor()->getARGB() !== null) {
if (!empty($fill->getEndColor()->getARGB())) {
$objWriter->startElement('color');
$objWriter->writeAttribute('rgb', $fill->getEndColor()->getARGB());
$objWriter->endElement();
Expand Down Expand Up @@ -244,7 +244,9 @@ private function writePatternFill(XMLWriter $objWriter, Fill $fill): void

// patternFill
$objWriter->startElement('patternFill');
$objWriter->writeAttribute('patternType', (string) $fill->getFillType());
if ($fill->getFillType()) {
$objWriter->writeAttribute('patternType', (string) $fill->getFillType());
}

if (self::writePatternColors($fill)) {
// fgColor
Expand Down
2 changes: 1 addition & 1 deletion tests/PhpSpreadsheetTests/Reader/Xlsx/DefaultFillTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ public function testDefaultConditionalFill(): void
$spreadsheet = $reader->load($filename);

$style = $spreadsheet->getActiveSheet()->getConditionalStyles('A1')[0]->getStyle();
self::assertSame('solid', $style->getFill()->getFillType());
self::assertSame('', $style->getFill()->getFillType());
}
}
104 changes: 104 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?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\Html as HtmlWriter;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
use PHPUnit\Framework\TestCase;

class Issue4248Test extends TestCase
{
private string $outfile = '';

protected function tearDown(): void
{
if ($this->outfile !== '') {
unlink($this->outfile);
$this->outfile = '';
}
}

public function testStyles(): void
{
$file = 'tests/data/Reader/XLSX/issue.4248.xlsx';
$reader = new XlsxReader();
$spreadsheet = $reader->load($file);
$writer = new XlsxWriter($spreadsheet);
$this->outfile = File::temporaryFilename();
$writer->save($this->outfile);
$spreadsheet->disconnectWorksheets();

$file = 'zip://';
$file .= $this->outfile;
$file .= '#xl/styles.xml';
$data = file_get_contents($file) ?: '';
$expected = '<fill>'
. '<patternFill patternType="darkDown"/>'
. '</fill>';
self::assertStringContainsString($expected, $data, 'neither fgColor nor bgColor');
$expected = '<fill>'
. '<patternFill patternType="darkDown">'
. '<bgColor rgb="FFBDD7EE"/>'
. '</patternFill></fill>';
self::assertStringContainsString($expected, $data, 'bgColor but no fgColor');
$expected = '<dxfs count="15">'
. '<dxf>' // dxfId 1 - fill color for Oui
. '<fill>'
. '<patternFill><bgColor rgb="FF00B050"/></patternFill>'
. '</fill>'
. '<border/>'
. '</dxf>'
. '<dxf>' // dxfId 2 - fill color for Non
. '<font><color rgb="FF9C0006"/></font>'
. '<fill>'
. '<patternFill><bgColor rgb="FFFFC7CE"/></patternFill>'
. '</fill>'
. '<border/>'
. '</dxf>';
self::assertStringContainsString($expected, $data, 'conditional fill styles');

$file = 'zip://';
$file .= $this->outfile;
$file .= '#xl/worksheets/sheet1.xml';
$data = file_get_contents($file) ?: '';
$expected = '<conditionalFormatting sqref="C16:C38 E17:H18 I17:J37 D18 J23:J38 E38 I38">'
. '<cfRule type="containsText" dxfId="0" priority="1" operator="containsText" text="Oui">'
. '<formula>NOT(ISERROR(SEARCH(&quot;Oui&quot;,C16)))</formula>'
. '</cfRule>'
. '</conditionalFormatting>';
self::assertStringContainsString($expected, $data, 'first condition for D18');
$expected = '<conditionalFormatting sqref="C16:C38 I17:J37 E17:H18 D18 J23:J38 E38 I38">'
. '<cfRule type="containsText" dxfId="1" priority="2" operator="containsText" text="Non">'
. '<formula>NOT(ISERROR(SEARCH(&quot;Non&quot;,C16)))</formula>'
. '</cfRule>'
. '</conditionalFormatting>';
self::assertStringContainsString($expected, $data, 'second condition for D18');
}

public function testHtml(): void
{
$file = 'tests/data/Reader/XLSX/issue.4248.xlsx';
$reader = new XlsxReader();
$spreadsheet = $reader->load($file);
$writer = new HtmlWriter($spreadsheet);

$file = 'zip://';
$file .= $this->outfile;
$file .= '#xl/styles.xml';
$data = str_replace(["\r", "\n"], '', $writer->generateHtmlAll());
$expected = ' <tr class="row17">' // Cell D18
. ' <td class="column0 style0">&nbsp;</td>'
. ' <td class="column1 style28 null"></td>'
. ' <td class="column2 style35 s">Eligible </td>'
. ' <td class="column3 style70 f">Non</td>';
self::assertStringContainsString($expected, $data, 'Cell D18 style');
$expected = ' td.style70, th.style70 { vertical-align:middle; text-align:center; border-bottom:1px solid #000000 !important; border-top:2px solid #000000 !important; border-left:2px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:16pt; background-color:#BDD7EE }';
self::assertStringContainsString($expected, $data, 'background color');

$spreadsheet->disconnectWorksheets();
}
}
Binary file added tests/data/Reader/XLSX/issue.4248.xlsx
View file
Open in desktop
Binary file not shown.