Skip to content

Commit a150917

Browse files
committed
Html Writer Conditional Formatting Inline Css
Fix #4539. Conditional Formatting was recently added to Html Writer. It works fine when not using inline Css. However, when using inline Css, the code inadvertently added 2 different `style` attributes (one for the unconditional style and one for the conditional style) to the same cell. This is not valid html, and results in losing the conditional styling. This PR combines the two `style` attributes into one, which will now come after the `class`, `colspan`, and `rowspan` attributes. Aside from the new tests, this PR changes an unusually large number of existing tests. While this might normally be considered a red flag, it is not a problem here. All of the changes involve merely changing the order of attributes within html tags; none of them affect how the generated html would appear in a browser.
1 parent 747ccd1 commit a150917

File tree

10 files changed

+78
-33
lines changed

10 files changed

+78
-33
lines changed

src/PhpSpreadsheet/Writer/Html.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,11 @@ private function generateRowStart(Worksheet $worksheet, int $sheetIndex, int $ro
13301330
$style = isset($this->cssStyles['table.sheet' . $sheetIndex . ' tr.row' . $row])
13311331
? $this->assembleCSS($this->cssStyles['table.sheet' . $sheetIndex . ' tr.row' . $row]) : '';
13321332

1333-
$html .= ' <tr style="' . $style . '">' . PHP_EOL;
1333+
if ($style === '') {
1334+
$html .= ' <tr>' . PHP_EOL;
1335+
} else {
1336+
$html .= ' <tr style="' . $style . '">' . PHP_EOL;
1337+
}
13341338
}
13351339

13361340
return $html;
@@ -1542,6 +1546,7 @@ private function generateRowWriteCell(
15421546
$html .= ' data-type="' . DataType::TYPE_STRING . '"';
15431547
}
15441548
}
1549+
$holdCss = '';
15451550
if (!$this->useInlineCss && !$this->isPdf && is_string($cssClass)) {
15461551
$html .= ' class="' . $cssClass . '"';
15471552
if ($htmlx) {
@@ -1587,7 +1592,7 @@ private function generateRowWriteCell(
15871592
$xcssClass['position'] = 'relative';
15881593
}
15891594
/** @var string[] $xcssClass */
1590-
$html .= ' style="' . $this->assembleCSS($xcssClass) . '"';
1595+
$holdCss = $this->assembleCSS($xcssClass);
15911596
if ($this->useInlineCss) {
15921597
$html .= ' class="gridlines gridlinesp"';
15931598
}
@@ -1638,13 +1643,17 @@ private function generateRowWriteCell(
16381643
}
16391644
if ($matched) {
16401645
$styles = $this->createCSSStyle($styleMerger->getStyle());
1641-
$html .= ' style="';
1646+
$html .= ' style="' . $holdCss . ' ';
1647+
$holdCss = '';
16421648
foreach ($styles as $key => $value) {
16431649
$html .= $key . ':' . $value . ';';
16441650
}
16451651
$html .= '"';
16461652
}
16471653
}
1654+
if ($holdCss !== '') {
1655+
$html .= ' style="' . $holdCss . '"';
1656+
}
16481657

16491658
$html .= '>';
16501659
$html .= $htmlx;

tests/PhpSpreadsheetTests/Writer/Dompdf/HideMergeTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public function testHideWithMerge(): void
9999
self::assertStringContainsString(
100100
'<tr class="row1">'
101101
. '<td class="column0 style0" style="width:42pt; height:17pt">&nbsp;</td>'
102-
. '<td class="column1 style1 s style1" style="width:84pt; height:17pt" colspan="2" rowspan="2">Hello World Headline</td>'
102+
. '<td class="column1 style1 s style1" colspan="2" rowspan="2" style="width:84pt; height:17pt">Hello World Headline</td>'
103103
. '</tr>',
104104
$html
105105
);
@@ -112,8 +112,8 @@ public function testHideWithMerge(): void
112112
self::assertStringContainsString(
113113
'<tr class="row3">'
114114
. '<td class="column0 style0" style="width:42pt; height:17pt">&nbsp;</td>'
115-
. '<td class="column1 style2 s style2" style="width:42pt; height:17pt" rowspan="2">Label 1</td>'
116-
. '<td class="column2 style3 s style3" style="width:42pt; height:17pt" rowspan="2">Text 1</td>'
115+
. '<td class="column1 style2 s style2" rowspan="2" style="width:42pt; height:17pt">Label 1</td>'
116+
. '<td class="column2 style3 s style3" rowspan="2" style="width:42pt; height:17pt">Text 1</td>'
117117
. '</tr>',
118118
$html
119119
);
@@ -126,8 +126,8 @@ public function testHideWithMerge(): void
126126
self::assertStringContainsString(
127127
'<tr class="row5">'
128128
. '<td class="column0 style0" style="width:42pt; height:17pt">&nbsp;</td>'
129-
. '<td class="column1 style2 s style2" style="width:42pt; height:17pt" rowspan="2">Label 2</td>'
130-
. '<td class="column2 style3 s style3" style="width:42pt; height:17pt" rowspan="2">Text 2</td>'
129+
. '<td class="column1 style2 s style2" rowspan="2" style="width:42pt; height:17pt">Label 2</td>'
130+
. '<td class="column2 style3 s style3" rowspan="2" style="width:42pt; height:17pt">Text 2</td>'
131131
. '</tr>',
132132
$html
133133
);

tests/PhpSpreadsheetTests/Writer/Html/BetterBooleanTest.php

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,15 @@ public function testInline(): void
153153
$writer->setUseInlineCss(true);
154154
$html = $writer->generateHtmlAll();
155155
$html = str_replace('vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; ', '', $html);
156-
$html = str_replace(' width:42pt" class="gridlines gridlinesp"', '"', $html);
156+
$html = str_replace(' width:42pt"', '"', $html);
157157
self::assertStringNotContainsString('TRUE', $html);
158-
self::assertStringContainsString('<td style="text-align:right;">1</td>', $html);
159-
self::assertStringContainsString('<td style="text-align:left;">Hello</td>', $html);
160-
self::assertStringContainsString('<td data-type="b" style="text-align:center;">VRAI</td>', $html);
161-
self::assertStringContainsString('<td data-type="b" style="text-align:center;">FAUX</td>', $html);
162-
self::assertStringContainsString('<td data-type="s" style="text-align:left;">1</td>', $html);
163-
self::assertStringContainsString('<td style="text-align:left;">AB</td>', $html);
164-
self::assertStringContainsString('<td style="text-align:right;">3</td>', $html);
158+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="text-align:right;">1</td>', $html);
159+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="text-align:left;">Hello</td>', $html);
160+
self::assertStringContainsString('<td data-type="b" class="gridlines gridlinesp" style="text-align:center;">VRAI</td>', $html);
161+
self::assertStringContainsString('<td data-type="b" class="gridlines gridlinesp" style="text-align:center;">FAUX</td>', $html);
162+
self::assertStringContainsString('<td data-type="s" class="gridlines gridlinesp" style="text-align:left;">1</td>', $html);
163+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="text-align:left;">AB</td>', $html);
164+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="text-align:right;">3</td>', $html);
165165

166166
$reloaded = $this->writeAndReload($spreadsheet, 'Html', null, $this->setBetter(...));
167167
$spreadsheet->disconnectWorksheets();

tests/PhpSpreadsheetTests/Writer/Html/Issue3678Test.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function testInlineAndNot(): void
3434
self::assertStringContainsString('.n { text-align:right }', $html);
3535
$writer->setUseInlineCss(true);
3636
$html = $writer->generateHtmlAll();
37-
self::assertStringContainsString('<td style="' . $style2 . '" class="gridlines gridlinesp">1</td>', $html);
37+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="' . $style2 . '">1</td>', $html);
3838
$spreadsheet->disconnectWorksheets();
3939
}
4040
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Html;
6+
7+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
8+
use PhpOffice\PhpSpreadsheet\Writer\Html;
9+
use PHPUnit\Framework\TestCase;
10+
11+
class Issue4539Test extends TestCase
12+
{
13+
public function testInlineAndNot(): void
14+
{
15+
$infile = 'tests/data/Reader/XLSX/issue.4539.xlsx';
16+
$reader = new XlsxReader();
17+
$spreadsheet = $reader->load($infile);
18+
$writer = new Html($spreadsheet);
19+
$writer->setConditionalFormatting(true);
20+
$writer->setUseInlineCss(true);
21+
$html = $writer->generateHtmlAll();
22+
$expected = '<td class="gridlines gridlinesp" style="vertical-align:bottom; color:#000000; font-family:\'Aptos Narrow\'; font-size:12pt; text-align:right; width:102pt vertical-align:bottom;border-bottom:none #000000;border-top:none #000000;border-left:none #000000;border-right:none #000000;color:#000000;font-family:\'Aptos Narrow\';font-size:12pt;background-color:#5A8AC6;">5</td>';
23+
self::assertStringContainsString($expected, $html, 'inline conditional style');
24+
$expected = '<td class="gridlines gridlinesp" style="vertical-align:bottom; font-weight:bold; color:#000000; font-family:\'Aptos Narrow\'; font-size:12pt; text-align:left; width:102pt">Column Heading</td>';
25+
self::assertStringContainsString($expected, $html, 'inline no conditional style');
26+
27+
$writer->setUseInlineCss(false);
28+
$html = $writer->generateHtmlAll();
29+
$expected = '<td class="column0 style2 n" style=" vertical-align:bottom;border-bottom:none #000000;border-top:none #000000;border-left:none #000000;border-right:none #000000;color:#000000;font-family:\'Aptos Narrow\';font-size:12pt;background-color:#5A8AC6;">5</td>';
30+
self::assertStringContainsString($expected, $html, 'notinline conditional style');
31+
$expected = '<td class="column0 style1 s">Column Heading</td>';
32+
self::assertStringContainsString($expected, $html, 'notinline no conditional style');
33+
34+
$spreadsheet->disconnectWorksheets();
35+
}
36+
}

tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public function testNoTitle(): void
2323
$writer->setUseInlineCss(true);
2424
$html = $writer->generateHTMLAll();
2525
self::assertStringContainsString('<title>Sheet1</title>', $html);
26-
self::assertStringContainsString('<td style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt" class="gridlines gridlinesp">C1</td>', $html);
26+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt">C1</td>', $html);
2727
$writer->setUseInlineCss(false);
2828
$html = $writer->generateHTMLAll();
2929
self::assertStringContainsString('<td class="column2 style0 s">C1</td>', $html);
@@ -55,8 +55,8 @@ public function testHideSomeGridlines(): void
5555
$writer = new Html($spreadsheet);
5656
$writer->setUseInlineCss(true);
5757
$html = $writer->generateHTMLAll();
58-
self::assertStringContainsString('<td style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:right; width:42pt" class="gridlines gridlinesp">7</td>', $html);
59-
self::assertStringContainsString('<td style="vertical-align:bottom; border-bottom:none #808080; border-top:none #808080; border-left:none #808080; border-right:none #808080; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:right; width:42pt" class="gridlines gridlinesp">19</td>', $html);
58+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:right; width:42pt">7</td>', $html);
59+
self::assertStringContainsString('<td class="gridlines gridlinesp" style="vertical-align:bottom; border-bottom:none #808080; border-top:none #808080; border-left:none #808080; border-right:none #808080; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:right; width:42pt">19</td>', $html);
6060
$spreadsheet->disconnectWorksheets();
6161
}
6262
}

tests/PhpSpreadsheetTests/Writer/Mpdf/HideMergeTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function testHideWithMerge(): void
9393
);
9494
self::assertStringContainsString(
9595
'<tr class="row1">'
96-
. '<td class="column1 style1 s style1" style="width:84pt; height:17pt" colspan="2" rowspan="2">Hello World Headline</td>'
96+
. '<td class="column1 style1 s style1" colspan="2" rowspan="2" style="width:84pt; height:17pt">Hello World Headline</td>'
9797
. '</tr>',
9898
$html
9999
);
@@ -104,8 +104,8 @@ public function testHideWithMerge(): void
104104
);
105105
self::assertStringContainsString(
106106
'<tr class="row3">'
107-
. '<td class="column1 style2 s style2" style="width:42pt; height:17pt" rowspan="2">Label 1</td>'
108-
. '<td class="column2 style3 s style3" style="width:42pt; height:17pt" rowspan="2">Text 1</td>'
107+
. '<td class="column1 style2 s style2" rowspan="2" style="width:42pt; height:17pt">Label 1</td>'
108+
. '<td class="column2 style3 s style3" rowspan="2" style="width:42pt; height:17pt">Text 1</td>'
109109
. '</tr>',
110110
$html
111111
);
@@ -116,8 +116,8 @@ public function testHideWithMerge(): void
116116
);
117117
self::assertStringContainsString(
118118
'<tr class="row5">'
119-
. '<td class="column1 style2 s style2" style="width:42pt; height:17pt" rowspan="2">Label 2</td>'
120-
. '<td class="column2 style3 s style3" style="width:42pt; height:17pt" rowspan="2">Text 2</td>'
119+
. '<td class="column1 style2 s style2" rowspan="2" style="width:42pt; height:17pt">Label 2</td>'
120+
. '<td class="column2 style3 s style3" rowspan="2" style="width:42pt; height:17pt">Text 2</td>'
121121
. '</tr>',
122122
$html
123123
);

tests/PhpSpreadsheetTests/Writer/Tcpdf/HideMergeTest.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,15 @@ public function testHideWithMerge(): void
8787
self::assertStringContainsString(
8888
'<tbody><tr style="height:17pt">'
8989
. '<td></td>'
90-
. '<td style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt" class="gridlines gridlinesp">B</td>'
91-
. '<td style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt" class="gridlines gridlinesp">C</td>'
90+
. '<td class="gridlines gridlinesp" style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt">B</td>'
91+
. '<td class="gridlines gridlinesp" style="vertical-align:bottom; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt">C</td>'
9292
. '</tr>',
9393
$html
9494
);
9595
self::assertStringContainsString(
9696
'<tr style="height:17pt">'
9797
. '<td></td>'
98-
. '<td style="vertical-align:bottom; text-align:center; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:11pt; width:84pt; height:17pt" class="gridlines gridlinesp" colspan="2" rowspan="2">Hello World Headline</td>'
98+
. '<td class="gridlines gridlinesp" colspan="2" rowspan="2" style="vertical-align:bottom; text-align:center; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:11pt; width:84pt; height:17pt">Hello World Headline</td>'
9999
. '</tr>',
100100
$html
101101
);
@@ -109,16 +109,16 @@ public function testHideWithMerge(): void
109109
self::assertStringContainsString(
110110
'<tr style="height:17pt">'
111111
. '<td></td>'
112-
. '<td style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt" class="gridlines gridlinesp" rowspan="2">Label 1</td>'
113-
. '<td style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt" class="gridlines gridlinesp" rowspan="2">Text 1</td>'
112+
. '<td class="gridlines gridlinesp" rowspan="2" style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt">Label 1</td>'
113+
. '<td class="gridlines gridlinesp" rowspan="2" style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt">Text 1</td>'
114114
. '</tr>',
115115
$html
116116
);
117117
self::assertStringContainsString(
118118
'<tr style="height:17pt">'
119119
. '<td></td>'
120-
. '<td style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt" class="gridlines gridlinesp" rowspan="2">Label 2</td>'
121-
. '<td style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt" class="gridlines gridlinesp" rowspan="2">Text 2</td>'
120+
. '<td class="gridlines gridlinesp" rowspan="2" style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt">Label 2</td>'
121+
. '<td class="gridlines gridlinesp" rowspan="2" style="vertical-align:middle; border-bottom:1px solid #000000 !important; border-top:1px solid #000000 !important; border-left:1px solid #000000 !important; border-right:1px solid #000000 !important; color:#000000; font-family:\'Calibri\'; font-size:11pt; text-align:left; width:42pt; height:17pt">Text 2</td>'
122122
. '</tr>',
123123
$html
124124
);

tests/PhpSpreadsheetTests/Writer/Tcpdf/MergedBorderTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public static function testMergedBorder(): void
3232
$sheet->setShowGridlines(false);
3333
$writer = new Tcpdf($spreadsheet);
3434
$html = $writer->generateHtmlAll();
35-
self::assertSame(1, preg_match('/border-bottom:1px solid #FF0000 !important; border-top:1px solid #FF0000 !important; border-left:1px solid #FF0000 !important; border-right:1px solid #FF0000 !important; color:#000000;[^>]+ colspan="2" rowspan="4"/', $html));
35+
self::assertSame(1, preg_match('/ colspan="2" rowspan="4" style="vertical-align:bottom; border-bottom:1px solid #FF0000 !important; border-top:1px solid #FF0000 !important; border-left:1px solid #FF0000 !important; border-right:1px solid #FF0000 !important; color:#000000;[^>]+/', $html));
3636
$spreadsheet->disconnectWorksheets();
3737
}
3838
}
8.9 KB
Binary file not shown.

0 commit comments

Comments
 (0)