Skip to content

Commit ad5532e

Browse files
oleibmanoleibman
authored
Namespacing Phase 2 - Styles (#2471)
* WIP Namespacing Phase 2 - Styles This is part 2 of a several-phase process to permit PhpSpreadsheet to handle input Xlsx files which use unexpected namespacing. The first phase, introduced as part of release 1.19.0, essentially handled the reading of data. This phase handles the reading of styles. More phases are planned. It is my intention to leave this in draft status for at least a month. This will give time for additional testing, by me and, I hope, others who might be interested. This fixes the same problem addressed by PR #2458, if it reaches mergeable status before I am ready to take this out of draft status. I do not anticipate any difficult merge conflicts if the other change is merged first. This change is more difficult than I'd hoped. I can't get xpath to work properly with the namespaced style file, even though I don't have difficulties with others. Normally we expect: ```xml <stylesheet xmlns="http://whatever" ... ``` In the namespaced files, we typically see: ```xml <x:stylesheet xmlns:x="http://whatever" ... ``` Simplexml_load_file specifying a namespace handles the two situations the same, as expected. But, for some reason that I cannot figure out, there are significant differences when xpath processes the result. However, I can manipulate the xml if necessary; I'm not proud of doing that, and will gladly accept any suggestions. In the meantime, it seems to work. My major non-standard unit test file had disabled any style-related tests when phase 1 was installed. These are now all enabled. * Scrutinizer Its analysis is wrong, but the "errors" it pointed out are easy to fix. * Eliminate XML Source Manipulation Original solution required XML manipulation to overcome what appears to be an xpath problem. This version replaces xpath with iteration, eliminating the need to manipulate the XML. * Handle Some Edge Cases For example, Style file without a Fills section. * Restore RGB/ARGB Interchangeability Fix #2494. Apparently EPPlus outputs fill colors as `<fgColor rgb="BFBFBF">` while most output fill colors as `<fgColor rgb="FFBFBFBF">`. EPPlus actually makes more sense. Regardless, validating length of rgb/argb is a recent development for PhpSpreadsheet, under the assumption that an incorrect length is a user error. This development invalidates that assumption, so restore the previous behavior. In addition, a comment in Colors.php says that the supplied color is "the ARGB value for the colour, or named colour". However, although named colors are accepted, nothing sensible is done with them - they are passed unchanged to the ARGB value, where Excel treats them as black. The routine should either reject the named color, or convert it to the appropriate ARGB value. This change implements the latter.
1 parent 88da4e1 commit ad5532e

File tree

9 files changed

+321
-110
lines changed

9 files changed

+321
-110
lines changed

‎src/PhpSpreadsheet/Reader/Xlsx.php‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -539,16 +539,14 @@ public function load(string $filename, int $flags = 0): Spreadsheet
539539
if ($xpath === null) {
540540
$xmlStyles = self::testSimpleXml(null);
541541
} else {
542-
// I think Nonamespace is okay because I'm using xpath.
543-
$xmlStyles = $this->loadZipNonamespace("$dir/$xpath[Target]", $mainNS);
542+
$xmlStyles = $this->loadZip("$dir/$xpath[Target]", $mainNS);
544543
}
545544

546-
$xmlStyles->registerXPathNamespace('smm', Namespaces::MAIN);
547-
$fills = self::xpathNoFalse($xmlStyles, 'smm:fills/smm:fill');
548-
$fonts = self::xpathNoFalse($xmlStyles, 'smm:fonts/smm:font');
549-
$borders = self::xpathNoFalse($xmlStyles, 'smm:borders/smm:border');
550-
$xfTags = self::xpathNoFalse($xmlStyles, 'smm:cellXfs/smm:xf');
551-
$cellXfTags = self::xpathNoFalse($xmlStyles, 'smm:cellStyleXfs/smm:xf');
545+
$fills = self::extractStyles($xmlStyles, 'fills', 'fill');
546+
$fonts = self::extractStyles($xmlStyles, 'fonts', 'font');
547+
$borders = self::extractStyles($xmlStyles, 'borders', 'border');
548+
$xfTags = self::extractStyles($xmlStyles, 'cellXfs', 'xf');
549+
$cellXfTags = self::extractStyles($xmlStyles, 'cellStyleXfs', 'xf');
552550

553551
$styles = [];
554552
$cellStyles = [];
@@ -559,6 +557,7 @@ public function load(string $filename, int $flags = 0): Spreadsheet
559557
if (isset($numFmts) && ($numFmts !== null)) {
560558
$numFmts->registerXPathNamespace('sml', $mainNS);
561559
}
560+
$this->styleReader->setNamespace($mainNS);
562561
if (!$this->readDataOnly/* && $xmlStyles*/) {
563562
foreach ($xfTags as $xfTag) {
564563
$xf = self::getAttributes($xfTag);
@@ -643,6 +642,7 @@ public function load(string $filename, int $flags = 0): Spreadsheet
643642
}
644643
}
645644
$this->styleReader->setStyleXml($xmlStyles);
645+
$this->styleReader->setNamespace($mainNS);
646646
$this->styleReader->setStyleBaseData($theme, $styles, $cellStyles);
647647
$dxfs = $this->styleReader->dxfs($this->readDataOnly);
648648
$styles = $this->styleReader->styles();
@@ -2088,4 +2088,16 @@ private function readAutoFilterTablesInTablesFile(
20882088
}
20892089
}
20902090
}
2091+
2092+
private static function extractStyles(?SimpleXMLElement $sxml, string $node1, string $node2): array
2093+
{
2094+
$array = [];
2095+
if ($sxml && $sxml->{$node1}->{$node2}) {
2096+
foreach ($sxml->{$node1}->{$node2} as $node) {
2097+
$array[] = $node;
2098+
}
2099+
}
2100+
2101+
return $array;
2102+
}
20912103
}

‎src/PhpSpreadsheet/Reader/Xlsx/Styles.php‎

Lines changed: 158 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,37 @@ class Styles extends BaseParserClass
3333
/** @var SimpleXMLElement */
3434
private $styleXml;
3535

36+
/** @var string */
37+
private $namespace = '';
38+
39+
public function setNamespace(string $namespace): void
40+
{
41+
$this->namespace = $namespace;
42+
}
43+
44+
/**
45+
* Cast SimpleXMLElement to bool to overcome Scrutinizer problem.
46+
*
47+
* @param mixed $value
48+
*/
49+
private static function castBool($value): bool
50+
{
51+
return (bool) $value;
52+
}
53+
54+
private function getStyleAttributes(SimpleXMLElement $value): SimpleXMLElement
55+
{
56+
$attr = null;
57+
if (self::castBool($value)) {
58+
$attr = $value->attributes('');
59+
if ($attr === null || count($attr) === 0) {
60+
$attr = $value->attributes($this->namespace);
61+
}
62+
}
63+
64+
return Xlsx::testSimpleXml($attr);
65+
}
66+
3667
public function setStyleXml(SimpleXmlElement $styleXml): void
3768
{
3869
$this->styleXml = $styleXml;
@@ -52,48 +83,62 @@ public function setStyleBaseData(?Theme $theme = null, array $styles = [], array
5283

5384
public function readFontStyle(Font $fontStyle, SimpleXMLElement $fontStyleXml): void
5485
{
55-
if (isset($fontStyleXml->name, $fontStyleXml->name['val'])) {
56-
$fontStyle->setName((string) $fontStyleXml->name['val']);
86+
if (isset($fontStyleXml->name)) {
87+
$attr = $this->getStyleAttributes($fontStyleXml->name);
88+
if (isset($attr['val'])) {
89+
$fontStyle->setName((string) $attr['val']);
90+
}
5791
}
58-
if (isset($fontStyleXml->sz, $fontStyleXml->sz['val'])) {
59-
$fontStyle->setSize((float) $fontStyleXml->sz['val']);
92+
if (isset($fontStyleXml->sz)) {
93+
$attr = $this->getStyleAttributes($fontStyleXml->sz);
94+
if (isset($attr['val'])) {
95+
$fontStyle->setSize((float) $attr['val']);
96+
}
6097
}
6198
if (isset($fontStyleXml->b)) {
62-
$fontStyle->setBold(!isset($fontStyleXml->b['val']) || self::boolean((string) $fontStyleXml->b['val']));
99+
$attr = $this->getStyleAttributes($fontStyleXml->b);
100+
$fontStyle->setBold(!isset($attr['val']) || self::boolean((string) $attr['val']));
63101
}
64102
if (isset($fontStyleXml->i)) {
65-
$fontStyle->setItalic(!isset($fontStyleXml->i['val']) || self::boolean((string) $fontStyleXml->i['val']));
103+
$attr = $this->getStyleAttributes($fontStyleXml->i);
104+
$fontStyle->setItalic(!isset($attr['val']) || self::boolean((string) $attr['val']));
66105
}
67106
if (isset($fontStyleXml->strike)) {
68-
$fontStyle->setStrikethrough(
69-
!isset($fontStyleXml->strike['val']) || self::boolean((string) $fontStyleXml->strike['val'])
70-
);
107+
$attr = $this->getStyleAttributes($fontStyleXml->strike);
108+
$fontStyle->setStrikethrough(!isset($attr['val']) || self::boolean((string) $attr['val']));
71109
}
72110
$fontStyle->getColor()->setARGB($this->readColor($fontStyleXml->color));
73111

74-
if (isset($fontStyleXml->u) && !isset($fontStyleXml->u['val'])) {
75-
$fontStyle->setUnderline(Font::UNDERLINE_SINGLE);
76-
} elseif (isset($fontStyleXml->u, $fontStyleXml->u['val'])) {
77-
$fontStyle->setUnderline((string) $fontStyleXml->u['val']);
112+
if (isset($fontStyleXml->u)) {
113+
$attr = $this->getStyleAttributes($fontStyleXml->u);
114+
if (!isset($attr['val'])) {
115+
$fontStyle->setUnderline(Font::UNDERLINE_SINGLE);
116+
} else {
117+
$fontStyle->setUnderline((string) $attr['val']);
118+
}
78119
}
79-
80-
if (isset($fontStyleXml->vertAlign, $fontStyleXml->vertAlign['val'])) {
81-
$verticalAlign = strtolower((string) $fontStyleXml->vertAlign['val']);
82-
if ($verticalAlign === 'superscript') {
83-
$fontStyle->setSuperscript(true);
84-
} elseif ($verticalAlign === 'subscript') {
85-
$fontStyle->setSubscript(true);
120+
if (isset($fontStyleXml->vertAlign)) {
121+
$attr = $this->getStyleAttributes($fontStyleXml->vertAlign);
122+
if (!isset($attr['val'])) {
123+
$verticalAlign = strtolower((string) $attr['val']);
124+
if ($verticalAlign === 'superscript') {
125+
$fontStyle->setSuperscript(true);
126+
} elseif ($verticalAlign === 'subscript') {
127+
$fontStyle->setSubscript(true);
128+
}
86129
}
87130
}
88131
}
89132

90133
private function readNumberFormat(NumberFormat $numfmtStyle, SimpleXMLElement $numfmtStyleXml): void
91134
{
92-
if ($numfmtStyleXml->count() === 0) {
135+
if ((string) $numfmtStyleXml['formatCode'] !== '') {
136+
$numfmtStyle->setFormatCode(self::formatGeneral((string) $numfmtStyleXml['formatCode']));
137+
93138
return;
94139
}
95-
$numfmt = Xlsx::getAttributes($numfmtStyleXml);
96-
if ($numfmt->count() > 0 && isset($numfmt['formatCode'])) {
140+
$numfmt = $this->getStyleAttributes($numfmtStyleXml);
141+
if (isset($numfmt['formatCode'])) {
97142
$numfmtStyle->setFormatCode(self::formatGeneral((string) $numfmt['formatCode']));
98143
}
99144
}
@@ -103,10 +148,11 @@ public function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyleXml):
103148
if ($fillStyleXml->gradientFill) {
104149
/** @var SimpleXMLElement $gradientFill */
105150
$gradientFill = $fillStyleXml->gradientFill[0];
106-
if (!empty($gradientFill['type'])) {
107-
$fillStyle->setFillType((string) $gradientFill['type']);
151+
$attr = $this->getStyleAttributes($gradientFill);
152+
if (!empty($attr['type'])) {
153+
$fillStyle->setFillType((string) $attr['type']);
108154
}
109-
$fillStyle->setRotation((float) ($gradientFill['degree']));
155+
$fillStyle->setRotation((float) ($attr['degree']));
110156
$gradientFill->registerXPathNamespace('sml', Namespaces::MAIN);
111157
$fillStyle->getStartColor()->setARGB($this->readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=0]'))->color));
112158
$fillStyle->getEndColor()->setARGB($this->readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=1]'))->color));
@@ -121,18 +167,25 @@ public function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyleXml):
121167
$defaultFillStyle = Fill::FILL_SOLID;
122168
}
123169

124-
$patternType = (string) $fillStyleXml->patternFill['patternType'] != ''
125-
? (string) $fillStyleXml->patternFill['patternType']
126-
: $defaultFillStyle;
170+
$type = '';
171+
if ((string) $fillStyleXml->patternFill['patternType'] !== '') {
172+
$type = (string) $fillStyleXml->patternFill['patternType'];
173+
} else {
174+
$attr = $this->getStyleAttributes($fillStyleXml->patternFill);
175+
$type = (string) $attr['patternType'];
176+
}
177+
$patternType = ($type === '') ? $defaultFillStyle : $type;
127178

128179
$fillStyle->setFillType($patternType);
129180
}
130181
}
131182

132183
public function readBorderStyle(Borders $borderStyle, SimpleXMLElement $borderStyleXml): void
133184
{
134-
$diagonalUp = self::boolean((string) $borderStyleXml['diagonalUp']);
135-
$diagonalDown = self::boolean((string) $borderStyleXml['diagonalDown']);
185+
$diagonalUp = $this->getAttribute($borderStyleXml, 'diagonalUp');
186+
$diagonalUp = self::boolean($diagonalUp);
187+
$diagonalDown = $this->getAttribute($borderStyleXml, 'diagonalDown');
188+
$diagonalDown = self::boolean($diagonalDown);
136189
if (!$diagonalUp && !$diagonalDown) {
137190
$borderStyle->setDiagonalDirection(Borders::DIAGONAL_NONE);
138191
} elseif ($diagonalUp && !$diagonalDown) {
@@ -150,10 +203,26 @@ public function readBorderStyle(Borders $borderStyle, SimpleXMLElement $borderSt
150203
$this->readBorder($borderStyle->getDiagonal(), $borderStyleXml->diagonal);
151204
}
152205

206+
private function getAttribute(SimpleXMLElement $xml, string $attribute): string
207+
{
208+
$style = '';
209+
if ((string) $xml[$attribute] !== '') {
210+
$style = (string) $xml[$attribute];
211+
} else {
212+
$attr = $this->getStyleAttributes($xml);
213+
if (isset($attr[$attribute])) {
214+
$style = (string) $attr[$attribute];
215+
}
216+
}
217+
218+
return $style;
219+
}
220+
153221
private function readBorder(Border $border, SimpleXMLElement $borderXml): void
154222
{
155-
if (isset($borderXml['style'])) {
156-
$border->setBorderStyle((string) $borderXml['style']);
223+
$style = $this->getAttribute($borderXml, 'style');
224+
if ($style !== '') {
225+
$border->setBorderStyle((string) $style);
157226
}
158227
if (isset($borderXml->color)) {
159228
$border->getColor()->setARGB($this->readColor($borderXml->color));
@@ -162,25 +231,25 @@ private function readBorder(Border $border, SimpleXMLElement $borderXml): void
162231

163232
public function readAlignmentStyle(Alignment $alignment, SimpleXMLElement $alignmentXml): void
164233
{
165-
$alignment->setHorizontal((string) $alignmentXml['horizontal']);
166-
$alignment->setVertical((string) $alignmentXml['vertical']);
167-
168-
$textRotation = 0;
169-
if ((int) $alignmentXml['textRotation'] <= 90) {
170-
$textRotation = (int) $alignmentXml['textRotation'];
171-
} elseif ((int) $alignmentXml['textRotation'] > 90) {
172-
$textRotation = 90 - (int) $alignmentXml['textRotation'];
173-
}
174-
175-
$alignment->setTextRotation((int) $textRotation);
176-
$alignment->setWrapText(self::boolean((string) $alignmentXml['wrapText']));
177-
$alignment->setShrinkToFit(self::boolean((string) $alignmentXml['shrinkToFit']));
178-
$alignment->setIndent(
179-
(int) ((string) $alignmentXml['indent']) > 0 ? (int) ((string) $alignmentXml['indent']) : 0
180-
);
181-
$alignment->setReadOrder(
182-
(int) ((string) $alignmentXml['readingOrder']) > 0 ? (int) ((string) $alignmentXml['readingOrder']) : 0
183-
);
234+
$horizontal = $this->getAttribute($alignmentXml, 'horizontal');
235+
$alignment->setHorizontal($horizontal);
236+
$vertical = $this->getAttribute($alignmentXml, 'vertical');
237+
$alignment->setVertical((string) $vertical);
238+
239+
$textRotation = (int) $this->getAttribute($alignmentXml, 'textRotation');
240+
if ($textRotation > 90) {
241+
$textRotation = 90 - $textRotation;
242+
}
243+
$alignment->setTextRotation($textRotation);
244+
245+
$wrapText = $this->getAttribute($alignmentXml, 'wrapText');
246+
$alignment->setWrapText(self::boolean((string) $wrapText));
247+
$shrinkToFit = $this->getAttribute($alignmentXml, 'shrinkToFit');
248+
$alignment->setShrinkToFit(self::boolean((string) $shrinkToFit));
249+
$indent = (int) $this->getAttribute($alignmentXml, 'indent');
250+
$alignment->setIndent(max($indent, 0));
251+
$readingOrder = (int) $this->getAttribute($alignmentXml, 'readingOrder');
252+
$alignment->setReadOrder(max($readingOrder, 0));
184253
}
185254

186255
private static function formatGeneral(string $formatString): string
@@ -223,8 +292,8 @@ public function readStyle(Style $docStyle, $style): void
223292

224293
// protection
225294
if (isset($style->protection)) {
226-
$this->readProtectionLocked($docStyle, $style);
227-
$this->readProtectionHidden($docStyle, $style);
295+
$this->readProtectionLocked($docStyle, $style->protection);
296+
$this->readProtectionHidden($docStyle, $style->protection);
228297
}
229298

230299
// top-level style settings
@@ -235,13 +304,20 @@ public function readStyle(Style $docStyle, $style): void
235304

236305
/**
237306
* Read protection locked attribute.
238-
*
239-
* @param SimpleXMLElement|stdClass $style
240307
*/
241-
public function readProtectionLocked(Style $docStyle, $style): void
308+
public function readProtectionLocked(Style $docStyle, SimpleXMLElement $style): void
242309
{
243-
if (isset($style->protection['locked'])) {
244-
if (self::boolean((string) $style->protection['locked'])) {
310+
$locked = '';
311+
if ((string) $style['locked'] !== '') {
312+
$locked = (string) $style['locked'];
313+
} else {
314+
$attr = $this->getStyleAttributes($style);
315+
if (isset($attr['locked'])) {
316+
$locked = (string) $attr['locked'];
317+
}
318+
}
319+
if ($locked !== '') {
320+
if (self::boolean($locked)) {
245321
$docStyle->getProtection()->setLocked(Protection::PROTECTION_PROTECTED);
246322
} else {
247323
$docStyle->getProtection()->setLocked(Protection::PROTECTION_UNPROTECTED);
@@ -251,13 +327,20 @@ public function readProtectionLocked(Style $docStyle, $style): void
251327

252328
/**
253329
* Read protection hidden attribute.
254-
*
255-
* @param SimpleXMLElement|stdClass $style
256330
*/
257-
public function readProtectionHidden(Style $docStyle, $style): void
331+
public function readProtectionHidden(Style $docStyle, SimpleXMLElement $style): void
258332
{
259-
if (isset($style->protection['hidden'])) {
260-
if (self::boolean((string) $style->protection['hidden'])) {
333+
$hidden = '';
334+
if ((string) $style['hidden'] !== '') {
335+
$hidden = (string) $style['hidden'];
336+
} else {
337+
$attr = $this->getStyleAttributes($style);
338+
if (isset($attr['hidden'])) {
339+
$hidden = (string) $attr['hidden'];
340+
}
341+
}
342+
if ($hidden !== '') {
343+
if (self::boolean((string) $hidden)) {
261344
$docStyle->getProtection()->setHidden(Protection::PROTECTION_PROTECTED);
262345
} else {
263346
$docStyle->getProtection()->setHidden(Protection::PROTECTION_UNPROTECTED);
@@ -267,15 +350,18 @@ public function readProtectionHidden(Style $docStyle, $style): void
267350

268351
public function readColor(SimpleXMLElement $color, bool $background = false): string
269352
{
270-
if (isset($color['rgb'])) {
271-
return (string) $color['rgb'];
272-
} elseif (isset($color['indexed'])) {
273-
return Color::indexedColor((int) ($color['indexed'] - 7), $background)->getARGB() ?? '';
274-
} elseif (isset($color['theme'])) {
353+
$attr = $this->getStyleAttributes($color);
354+
if (isset($attr['rgb'])) {
355+
return (string) $attr['rgb'];
356+
}
357+
if (isset($attr['indexed'])) {
358+
return Color::indexedColor((int) ($attr['indexed'] - 7), $background)->getARGB() ?? '';
359+
}
360+
if (isset($attr['theme'])) {
275361
if ($this->theme !== null) {
276-
$returnColour = $this->theme->getColourByIndex((int) $color['theme']);
277-
if (isset($color['tint'])) {
278-
$tintAdjust = (float) $color['tint'];
362+
$returnColour = $this->theme->getColourByIndex((int) $attr['theme']);
363+
if (isset($attr['tint'])) {
364+
$tintAdjust = (float) $attr['tint'];
279365
$returnColour = Color::changeBrightness($returnColour ?? '', $tintAdjust);
280366
}
281367

0 commit comments

Comments
 (0)