Skip to content

Commit

Permalink
Column Widths, Especially for ODS (#3610)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
oleibman committed Jun 14, 2023
1 parent eba7271 commit 570660f
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 10 deletions.
28 changes: 20 additions & 8 deletions src/PhpSpreadsheet/Reader/Html.php
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,12 @@ private function processDomElementImg(Worksheet $sheet, int &$row, string &$colu
}
}

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) {
Expand All @@ -513,7 +516,10 @@ private function processDomElementTable(Worksheet $sheet, int &$row, string &$co

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);
Expand Down Expand Up @@ -877,7 +883,9 @@ private function applyInlineStyle(Worksheet &$sheet, $row, $column, $attributeAr
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;
Expand Down Expand Up @@ -1009,16 +1017,20 @@ private function applyInlineStyle(Worksheet &$sheet, $row, $column, $attributeAr
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;

Expand Down
40 changes: 40 additions & 0 deletions src/PhpSpreadsheet/Reader/Ods.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
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;
Expand Down Expand Up @@ -295,11 +296,29 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)
$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);
Expand Down Expand Up @@ -340,6 +359,7 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)

// Go through every child of table element
$rowID = 1;
$tableColumnIndex = 1;
foreach ($worksheetDataSet->childNodes as $childNode) {
/** @var DOMElement $childNode */

Expand All @@ -366,6 +386,26 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)
// $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')) {
Expand Down
9 changes: 9 additions & 0 deletions src/PhpSpreadsheet/Worksheet/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,18 @@ public function getDefaultRowDimension()
*/
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.
*
Expand Down
9 changes: 9 additions & 0 deletions src/PhpSpreadsheet/Writer/Ods/Content.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,16 @@ private function writeSheets(XMLWriter $objWriter): void
$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',
Expand Down
4 changes: 2 additions & 2 deletions tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ public function testReadValueAndComments(): void

$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());
Expand Down
65 changes: 65 additions & 0 deletions tests/PhpSpreadsheetTests/Worksheet/ColumnDimension2Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

namespace PhpOffice\PhpSpreadsheetTests\Worksheet;

use PhpOffice\PhpSpreadsheet\Helper\Dimension;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Writer\Html;
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;

class ColumnDimension2Test extends AbstractFunctional
{
/**
* @dataProvider providerType
*/
public function testSetsAndDefaults(string $type): void
{
$columns = ['J', 'A', 'F', 'M', 'N', 'T', 'S'];
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->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);
}
}

0 comments on commit 570660f

Please sign in to comment.