From 570660f12ad740c251802575428485d5db34531a Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 13 Jun 2023 17:06:05 -0700 Subject: [PATCH] Column Widths, Especially for ODS (#3610) Fix #3609. Reporter created a spreadsheet with non-adjacent columns having non-default widths. Ods Writer needs to generate entries for the missing columns with default width. The fix was fairly simple. Testing it was not. Ods Reader basically ignores all styles; they are complicated, declared in (at least) 2 places (content.xml and styles.xml). This change deals only with the problem as reported, in which the missing declarations should be in content.xml. If someone reports a real-world example of this involving styles.xml, I can look at that then. In the meantime, this toehold might serve as a template for adding other style processing to Ods Reader. Looking at other formats, processing of column widths was also missing from Html Reader, and is now added. Note that this will work only with inline Css declarations on the `col` tags, which can be generated by Html Writer using `setUseInlineCss(true)`. This creates a much larger file than one created without inline CSS. A general problem became evident when studying the code. Worksheet `columnDimensions` is an unsorted array. This is not a problem per se, but it can easily lead to unexpected results from a `getColumnDimensions` call. The code is changed to sort the array before returning it in `getColumnDimensions`. One existing test failed as a result of this change. It errorneously tested `getHighestColumn` instead of `getHighestDataColumn`, which caused a problem because the final column declaration included a `number-columns-repeated` attribute. The new result for `getHighestColumn` is correct, and the test is changed to use `getHighestDataColumn` instead, which was certainly its intent. --- src/PhpSpreadsheet/Reader/Html.php | 28 +++++--- src/PhpSpreadsheet/Reader/Ods.php | 40 ++++++++++++ src/PhpSpreadsheet/Worksheet/Worksheet.php | 9 +++ src/PhpSpreadsheet/Writer/Ods/Content.php | 9 +++ .../Reader/Ods/OdsTest.php | 4 +- .../Worksheet/ColumnDimension2Test.php | 65 +++++++++++++++++++ 6 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Worksheet/ColumnDimension2Test.php diff --git a/src/PhpSpreadsheet/Reader/Html.php b/src/PhpSpreadsheet/Reader/Html.php index fa3db908e..bfb52401a 100644 --- a/src/PhpSpreadsheet/Reader/Html.php +++ b/src/PhpSpreadsheet/Reader/Html.php @@ -491,9 +491,12 @@ class Html extends BaseReader } } + private string $currentColumn = 'A'; + private function processDomElementTable(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child, array &$attributeArray): void { if ($child->nodeName === 'table') { + $this->currentColumn = 'A'; $this->flushCell($sheet, $column, $row, $cellContent, $attributeArray); $column = $this->setTableStartColumn($column); if ($this->tableLevel > 1 && $row > 1) { @@ -513,7 +516,10 @@ class Html extends BaseReader private function processDomElementTr(Worksheet $sheet, int &$row, string &$column, string &$cellContent, DOMElement $child, array &$attributeArray): void { - if ($child->nodeName === 'tr') { + if ($child->nodeName === 'col') { + $this->applyInlineStyle($sheet, -1, $this->currentColumn, $attributeArray); + ++$this->currentColumn; + } elseif ($child->nodeName === 'tr') { $column = $this->getTableStartColumn(); $cellContent = ''; $this->processDomElement($child, $sheet, $row, $column, $cellContent); @@ -877,7 +883,9 @@ class Html extends BaseReader return; } - if (isset($attributeArray['rowspan'], $attributeArray['colspan'])) { + if ($row <= 0 || $column === '') { + $cellStyle = new Style(); + } elseif (isset($attributeArray['rowspan'], $attributeArray['colspan'])) { $columnTo = $column; for ($i = 0; $i < (int) $attributeArray['colspan'] - 1; ++$i) { ++$columnTo; @@ -1009,16 +1017,20 @@ class Html extends BaseReader break; case 'width': - $sheet->getColumnDimension($column)->setWidth( - (new CssDimension($styleValue ?? ''))->width() - ); + if ($column !== '') { + $sheet->getColumnDimension($column)->setWidth( + (new CssDimension($styleValue ?? ''))->width() + ); + } break; case 'height': - $sheet->getRowDimension($row)->setRowHeight( - (new CssDimension($styleValue ?? ''))->height() - ); + if ($row > 0) { + $sheet->getRowDimension($row)->setRowHeight( + (new CssDimension($styleValue ?? ''))->height() + ); + } break; diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 250815400..9913f3325 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -8,6 +8,7 @@ use DOMElement; use DOMNode; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Cell\DataType; +use PhpOffice\PhpSpreadsheet\Helper\Dimension as HelperDimension; use PhpOffice\PhpSpreadsheet\Reader\Ods\AutoFilter; use PhpOffice\PhpSpreadsheet\Reader\Ods\DefinedNames; use PhpOffice\PhpSpreadsheet\Reader\Ods\FormulaTranslator; @@ -295,11 +296,29 @@ class Ods extends BaseReader $tableNs = $dom->lookupNamespaceUri('table'); $textNs = $dom->lookupNamespaceUri('text'); $xlinkNs = $dom->lookupNamespaceUri('xlink'); + $styleNs = $dom->lookupNamespaceUri('style'); $pageSettings->readStyleCrossReferences($dom); $autoFilterReader = new AutoFilter($spreadsheet, $tableNs); $definedNameReader = new DefinedNames($spreadsheet, $tableNs); + $columnWidths = []; + $automaticStyle0 = $dom->getElementsByTagNameNS($officeNs, 'automatic-styles')->item(0); + $automaticStyles = ($automaticStyle0 === null) ? [] : $automaticStyle0->getElementsByTagNameNS($styleNs, 'style'); + foreach ($automaticStyles as $automaticStyle) { + $styleName = $automaticStyle->getAttributeNS($styleNs, 'name'); + $styleFamily = $automaticStyle->getAttributeNS($styleNs, 'family'); + if ($styleFamily === 'table-column') { + $tcprops = $automaticStyle->getElementsByTagNameNS($styleNs, 'table-column-properties'); + if ($tcprops !== null) { + $tcprop = $tcprops->item(0); + if ($tcprop !== null) { + $columnWidth = $tcprop->getAttributeNs($styleNs, 'column-width'); + $columnWidths[$styleName] = $columnWidth; + } + } + } + } // Content $item0 = $dom->getElementsByTagNameNS($officeNs, 'body')->item(0); @@ -340,6 +359,7 @@ class Ods extends BaseReader // Go through every child of table element $rowID = 1; + $tableColumnIndex = 1; foreach ($worksheetDataSet->childNodes as $childNode) { /** @var DOMElement $childNode */ @@ -366,6 +386,26 @@ class Ods extends BaseReader // $rowData = $cellData; // break; // } + break; + case 'table-column': + if ($childNode->hasAttributeNS($tableNs, 'number-columns-repeated')) { + $rowRepeats = (int) $childNode->getAttributeNS($tableNs, 'number-columns-repeated'); + } else { + $rowRepeats = 1; + } + $tableStyleName = $childNode->getAttributeNS($tableNs, 'style-name'); + if (isset($columnWidths[$tableStyleName])) { + $columnWidth = new HelperDimension($columnWidths[$tableStyleName]); + $tableColumnString = Coordinate::stringFromColumnIndex($tableColumnIndex); + for ($rowRepeats2 = $rowRepeats; $rowRepeats2 > 0; --$rowRepeats2) { + $spreadsheet->getActiveSheet() + ->getColumnDimension($tableColumnString) + ->setWidth($columnWidth->toUnit('cm'), 'cm'); + ++$tableColumnString; + } + } + $tableColumnIndex += $rowRepeats; + break; case 'table-row': if ($childNode->hasAttributeNS($tableNs, 'number-rows-repeated')) { diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 06f86cd17..29221e991 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -543,9 +543,18 @@ class Worksheet implements IComparable */ public function getColumnDimensions() { + /** @var callable */ + $callable = [self::class, 'columnDimensionCompare']; + uasort($this->columnDimensions, $callable); + return $this->columnDimensions; } + private static function columnDimensionCompare(ColumnDimension $a, ColumnDimension $b): int + { + return $a->getColumnNumeric() - $b->getColumnNumeric(); + } + /** * Get default column dimension. * diff --git a/src/PhpSpreadsheet/Writer/Ods/Content.php b/src/PhpSpreadsheet/Writer/Ods/Content.php index e931421af..e0a729ab8 100644 --- a/src/PhpSpreadsheet/Writer/Ods/Content.php +++ b/src/PhpSpreadsheet/Writer/Ods/Content.php @@ -126,7 +126,16 @@ class Content extends WriterPart $objWriter->writeAttribute('table:name', $spreadsheet->getSheet($sheetIndex)->getTitle()); $objWriter->writeAttribute('table:style-name', Style::TABLE_STYLE_PREFIX . (string) ($sheetIndex + 1)); $objWriter->writeElement('office:forms'); + $lastColumn = 0; foreach ($spreadsheet->getSheet($sheetIndex)->getColumnDimensions() as $columnDimension) { + $thisColumn = $columnDimension->getColumnNumeric(); + $emptyColumns = $thisColumn - $lastColumn - 1; + if ($emptyColumns > 0) { + $objWriter->startElement('table:table-column'); + $objWriter->writeAttribute('table:number-columns-repeated', (string) $emptyColumns); + $objWriter->endElement(); + } + $lastColumn = $thisColumn; $objWriter->startElement('table:table-column'); $objWriter->writeAttribute( 'table:style-name', diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php index 47fc13d8e..f7c769cd1 100644 --- a/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php @@ -141,8 +141,8 @@ class OdsTest extends TestCase $firstSheet = $spreadsheet->getSheet(0); - self::assertEquals(29, $firstSheet->getHighestRow()); - self::assertEquals('N', $firstSheet->getHighestColumn()); + self::assertEquals(29, $firstSheet->getHighestDataRow()); + self::assertEquals('N', $firstSheet->getHighestDataColumn()); // Simple cell value self::assertEquals('Test String 1', $firstSheet->getCell('A1')->getValue()); diff --git a/tests/PhpSpreadsheetTests/Worksheet/ColumnDimension2Test.php b/tests/PhpSpreadsheetTests/Worksheet/ColumnDimension2Test.php new file mode 100644 index 000000000..f5878eef5 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/ColumnDimension2Test.php @@ -0,0 +1,65 @@ +getActiveSheet(); + $expectedCm = 0.45; + foreach ($columns as $column) { + $sheet->getColumnDimension($column)->setWidth($expectedCm, 'cm'); + } + if ($type === 'Html') { + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $type, null, [self::class, 'inlineCss']); + } else { + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $type); + } + $spreadsheet->disconnectWorksheets(); + $sheet = $reloadedSpreadsheet->getActiveSheet(); + for ($column = 'A'; $column !== 'Z'; ++$column) { + if (in_array($column, $columns, true)) { + self::assertEqualsWithDelta($expectedCm, $sheet->getColumnDimension($column)->getWidth(Dimension::UOM_CENTIMETERS), 1E-3, "column $column"); + } elseif ($type === 'Xls' && $column <= 'T') { + // Xls is a bit weird. Columns through max used + // actually set a width in the spreadsheet file. + // Columns above max used obviously do not. + self::assertEqualsWithDelta(9.140625, $sheet->getColumnDimension($column)->getWidth(), 1E-3, "column $column"); + } elseif ($type === 'Html' && $column <= 'T') { + // Html is a lot like Xls. Columns through max used + // actually set a width in the spreadsheet file. + // Columns above max used obviously do not. + self::assertEqualsWithDelta(7.998, $sheet->getColumnDimension($column)->getWidth(), 1E-3, "column $column"); + } else { + self::assertSame(-1.0, $sheet->getColumnDimension($column)->getWidth(), "column $column"); + } + } + $reloadedSpreadsheet->disconnectWorksheets(); + } + + public static function providerType(): array + { + return [ + ['Xlsx'], + ['Xls'], + ['Ods'], + ['Html'], + ]; + } + + public static function inlineCss(Html $writer): void + { + $writer->setUseInlineCss(true); + } +}