Ods Reader Style Support (Last) - Column Styles (Partial)

Continuing the work of PR #4810 and PR #4813 and PR #4814. According to the spec for LibreOffice, the xml for both coluns and rows allow a `default-cell-style-name` attribute. However, my testing indicates that using that attribute with a row doesn't result in the results I would expect; furthermore, LibreOffice doesn't seem to use that attribute in association with a row (it does use it for columns). For rows, it fills the unpopulated rows in the cell with a repeated table cell with the appropriate style. These filler cells have been a source of problems for us, and I'm not willing to add them at this time (maybe in future). So, if a style is applied to a row in PhpSpreadsheet, it will be reflected in all cells in that row until `highestDataColumn`, but no further.

Since LibreOffice does support adding a style to a column both in the UI and the Xml, it should be a little easier. But its support is imcomplete as well - when reading a spreadsheet, the UI will apply the style only to the rows that are defined in the xml. Again, this requires filler rows for the rows beyond `highestDataRow`, and, again, I am not willing to add those. However, it is important to process the column style when reading because, unlike the case with rows, the cells which are defined in the column may omit style data altogether if the style matches the column style.

There has till now been no support for `ReadDataOnly` (default false) and `ReadEmptyCells` (default true) for Ods Reader. These are now added. ReadEmptyCells is especially important and it would be sensible to usually set it to false. The reason is that LibreOffice usually supplies a width for all the columns mentioned in its row fillers, and PhpSpreadsheet needs to create ColumnDimensions for each of those as a result, resulting in much larger xml than one would expect when the spreadsheet is saved. This arose when I was testing this change - it has always been the case, but nobody has raised an issue about it. We avoid this problem by ignoring the extra columns for ReadEmptyCells false. But this requires a little redundancy in the code. We need to define styles for an entire column before we define styles for the cells in the column, but we don't need to define column widths beforehand. So, if ReadEmptyCells is false, we need to process the columns for styles, then process the cell data (so that we know what `highDataColumn` is), then process the columns again but this time for widths (ignoring columns beyond highDataColumn). If ReadEmptyCells is true, we can process column styles and widths in a single pass.

src/PhpSpreadsheet/Reader/Ods.php

@@ -298,6 +298,8 @@ class Ods extends BaseReader
      */
     private array $allStyles;
 
+    private int $highestDataIndex;
+
     /**
      * Loads PhpSpreadsheet from file into PhpSpreadsheet instance.
      */
@@ -335,8 +337,7 @@ class Ods extends BaseReader
         $styleNs = (string) $dom->lookupNamespaceUri('style');
         $fontNs = (string) $dom->lookupNamespaceUri('fo');
 
-        $defaultStyleSet = false;
-        $automaticStyle0 = $dom->getElementsByTagNameNS($officeNs, 'styles')->item(0);
+        $automaticStyle0 = $this->readDataOnly ? null : $dom->getElementsByTagNameNS($officeNs, 'styles')->item(0);
         $automaticStyles = ($automaticStyle0 === null) ? [] : $automaticStyle0->getElementsByTagNameNS($styleNs, 'default-style');
         foreach ($automaticStyles as $automaticStyle) {
             $styleFamily = $automaticStyle->getAttributeNS($styleNs, 'family');
@@ -346,7 +347,6 @@ class Ods extends BaseReader
                     $fonts = $this->getFontStyles($textProperty, $styleNs, $fontNs);
                 }
                 if (!empty($fonts)) {
-                    $defaultStyleSet = true;
                     $spreadsheet->getDefaultStyle()
                         ->getFont()
                         ->applyFromArray($fonts);
@@ -368,7 +368,7 @@ class Ods extends BaseReader
                 if ($styleName !== '') {
                     if (!empty($fonts)) {
                         $this->allStyles[$styleName]['font'] = $fonts;
-                        if (!$defaultStyleSet && $styleName === 'Default') {
+                        if ($styleName === 'Default') {
                             $spreadsheet->getDefaultStyle()
                                 ->getFont()
                                 ->applyFromArray($fonts);
@@ -405,7 +405,7 @@ class Ods extends BaseReader
         $autoFilterReader = new AutoFilter($spreadsheet, $tableNs);
         $definedNameReader = new DefinedNames($spreadsheet, $tableNs);
         $columnWidths = [];
-        $automaticStyle0 = $dom->getElementsByTagNameNS($officeNs, 'automatic-styles')->item(0);
+        $automaticStyle0 = $this->readDataOnly ? null : $dom->getElementsByTagNameNS($officeNs, 'automatic-styles')->item(0);
         $automaticStyles = ($automaticStyle0 === null) ? [] : $automaticStyle0->getElementsByTagNameNS($styleNs, 'style');
         foreach ($automaticStyles as $automaticStyle) {
             $styleName = $automaticStyle->getAttributeNS($styleNs, 'name');
@@ -496,6 +496,7 @@ class Ods extends BaseReader
                 // Go through every child of table element
                 $rowID = 1;
                 $tableColumnIndex = 1;
+                $this->highestDataIndex = 16384;
                 foreach ($worksheetDataSet->childNodes as $childNode) {
                     /** @var DOMElement $childNode */
 
@@ -533,37 +534,6 @@ class Ods extends BaseReader
                                 $spreadsheet
                             );
 
-                            break;
-                        case 'table-header-columns':
-                        case 'table-columns':
-                            $this->processTableHeaderColumns(
-                                $childNode,
-                                $tableNs,
-                                $columnWidths,
-                                $tableColumnIndex,
-                                $spreadsheet
-                            );
-
-                            break;
-                        case 'table-column-group':
-                            $this->processTableColumnGroup(
-                                $childNode,
-                                $tableNs,
-                                $columnWidths,
-                                $tableColumnIndex,
-                                $spreadsheet
-                            );
-
-                            break;
-                        case 'table-column':
-                            $this->processTableColumn(
-                                $childNode,
-                                $tableNs,
-                                $columnWidths,
-                                $tableColumnIndex,
-                                $spreadsheet
-                            );
-
                             break;
                         case 'table-row':
                             $this->processTableRow(
@@ -577,6 +547,43 @@ class Ods extends BaseReader
                                 $spreadsheet
                             );
 
+                            break;
+                        case 'table-header-columns':
+                        case 'table-columns':
+                            $this->processTableColumnHeader(
+                                $childNode,
+                                $tableNs,
+                                $columnWidths,
+                                $tableColumnIndex,
+                                $spreadsheet,
+                                $this->readEmptyCells,
+                                true
+                            );
+
+                            break;
+                        case 'table-column-group':
+                            $this->processTableColumnGroup(
+                                $childNode,
+                                $tableNs,
+                                $columnWidths,
+                                $tableColumnIndex,
+                                $spreadsheet,
+                                $this->readEmptyCells,
+                                true
+                            );
+
+                            break;
+                        case 'table-column':
+                            $this->processTableColumn(
+                                $childNode,
+                                $tableNs,
+                                $columnWidths,
+                                $tableColumnIndex,
+                                $spreadsheet,
+                                $this->readEmptyCells,
+                                true
+                            );
+
                             break;
                     }
                 }
@@ -593,10 +600,94 @@ class Ods extends BaseReader
             if ($this->createBlankSheetIfNoneRead && !$sheetCreated) {
                 $spreadsheet->createSheet();
             }
+        }
+
+        foreach ($spreadsheets as $workbookData) {
+            /** @var DOMElement $workbookData */
+            $tables = $workbookData->getElementsByTagNameNS($tableNs, 'table');
+
+            $worksheetID = 0;
+            foreach ($tables as $worksheetDataSet) {
+                /** @var DOMElement $worksheetDataSet */
+                $worksheetName = $worksheetDataSet->getAttributeNS($tableNs, 'name');
+
+                // Check loadSheetsOnly
+                if (
+                    $this->loadSheetsOnly !== null
+                    && $worksheetName
+                    && !in_array($worksheetName, $this->loadSheetsOnly)
+                ) {
+                    continue;
+                }
+
+                // Create sheet
+                $spreadsheet->setActiveSheetIndex($worksheetID);
+                $highestDataColumn = $spreadsheet->getActiveSheet()->getHighestDataColumn();
+                $this->highestDataIndex = Coordinate::columnIndexFromString($highestDataColumn);
+
+                // Go through every child of table element processing column widths
+                $rowID = 1;
+                $tableColumnIndex = 1;
+                foreach ($worksheetDataSet->childNodes as $childNode) {
+                    /** @var DOMElement $childNode */
+                    if (empty($columnWidths) || $this->readEmptyCells) {
+                        break;
+                    }
+
+                    // Filter elements which are not under the "table" ns
+                    if ($childNode->namespaceURI != $tableNs) {
+                        continue;
+                    }
+
+                    $key = self::extractNodeName($childNode->nodeName);
+
+                    switch ($key) {
+                        case 'table-header-columns':
+                        case 'table-columns':
+                            $this->processTableColumnHeader(
+                                $childNode,
+                                $tableNs,
+                                $columnWidths,
+                                $tableColumnIndex,
+                                $spreadsheet,
+                                true,
+                                false
+                            );
+
+                            break;
+                        case 'table-column-group':
+                            $this->processTableColumnGroup(
+                                $childNode,
+                                $tableNs,
+                                $columnWidths,
+                                $tableColumnIndex,
+                                $spreadsheet,
+                                true,
+                                false
+                            );
+
+                            break;
+                        case 'table-column':
+                            $this->processTableColumn(
+                                $childNode,
+                                $tableNs,
+                                $columnWidths,
+                                $tableColumnIndex,
+                                $spreadsheet,
+                                true,
+                                false
+                            );
+
+                            break;
+                    }
+                }
+                ++$worksheetID;
+            }
 
             $autoFilterReader->read($workbookData);
             $definedNameReader->read($workbookData);
         }
+
         $spreadsheet->setActiveSheetIndex(0);
 
         if ($zip->locateName('settings.xml') !== false) {
@@ -1091,12 +1182,14 @@ class Ods extends BaseReader
     /**
      * @param string[] $columnWidths
      */
-    private function processTableHeaderColumns(
+    private function processTableColumnHeader(
         DOMElement $childNode,
         string $tableNs,
         array $columnWidths,
         int &$tableColumnIndex,
-        Spreadsheet $spreadsheet
+        Spreadsheet $spreadsheet,
+        bool $processWidths = true,
+        bool $processStyles = true
     ): void {
         foreach ($childNode->childNodes as $grandchildNode) {
             /** @var DOMElement $grandchildNode */
@@ -1108,7 +1201,9 @@ class Ods extends BaseReader
                         $tableNs,
                         $columnWidths,
                         $tableColumnIndex,
-                        $spreadsheet
+                        $spreadsheet,
+                        $processWidths,
+                        $processStyles
                     );
 
                     break;
@@ -1124,7 +1219,9 @@ class Ods extends BaseReader
         string $tableNs,
         array $columnWidths,
         int &$tableColumnIndex,
-        Spreadsheet $spreadsheet
+        Spreadsheet $spreadsheet,
+        bool $processWidths = true,
+        bool $processStyles = true
     ): void {
         foreach ($childNode->childNodes as $grandchildNode) {
             /** @var DOMElement $grandchildNode */
@@ -1136,18 +1233,22 @@ class Ods extends BaseReader
                         $tableNs,
                         $columnWidths,
                         $tableColumnIndex,
-                        $spreadsheet
+                        $spreadsheet,
+                        $processWidths,
+                        $processStyles
                     );
 
                     break;
                 case 'table-header-columns':
                 case 'table-columns':
-                    $this->processTableHeaderColumns(
+                    $this->processTableColumnHeader(
                         $grandchildNode,
                         $tableNs,
                         $columnWidths,
                         $tableColumnIndex,
-                        $spreadsheet
+                        $spreadsheet,
+                        $processWidths,
+                        $processStyles
                     );
 
                     break;
@@ -1157,7 +1258,9 @@ class Ods extends BaseReader
                         $tableNs,
                         $columnWidths,
                         $tableColumnIndex,
-                        $spreadsheet
+                        $spreadsheet,
+                        $processWidths,
+                        $processStyles
                     );
 
                     break;
@@ -1173,7 +1276,9 @@ class Ods extends BaseReader
         string $tableNs,
         array $columnWidths,
         int &$tableColumnIndex,
-        Spreadsheet $spreadsheet
+        Spreadsheet $spreadsheet,
+        bool $processWidths = true,
+        bool $processStyles = true
     ): void {
         if ($childNode->hasAttributeNS($tableNs, 'number-columns-repeated')) {
             $rowRepeats = (int) $childNode->getAttributeNS($tableNs, 'number-columns-repeated');
@@ -1181,15 +1286,41 @@ class Ods extends BaseReader
             $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) {
-                /** @var string $tableColumnString */
-                $spreadsheet->getActiveSheet()
-                    ->getColumnDimension($tableColumnString)
-                    ->setWidth($columnWidth->toUnit('cm'), 'cm');
-                StringHelper::stringIncrement($tableColumnString);
+        if ($processWidths) {
+            if (isset($columnWidths[$tableStyleName])) {
+                $columnWidth = new HelperDimension($columnWidths[$tableStyleName]);
+                $tableColumnIndex2 = $tableColumnIndex;
+                $tableColumnString = Coordinate::stringFromColumnIndex($tableColumnIndex2);
+                for ($rowRepeats2 = $rowRepeats; $rowRepeats2 > 0 && $tableColumnIndex2 <= 16384; --$rowRepeats2) {
+                    if (!$this->readEmptyCells && $tableColumnIndex2 > $this->highestDataIndex) {
+                        break;
+                    }
+                    $spreadsheet->getActiveSheet()
+                        ->getColumnDimension($tableColumnString)
+                        ->setWidth($columnWidth->toUnit('cm'), 'cm');
+                    StringHelper::stringIncrement(
+                        $tableColumnString
+                    );
+                    ++$tableColumnIndex2;
+                }
+            }
+        }
+        if ($processStyles) {
+            $defaultStyleName = $childNode->getAttributeNS($tableNs, 'default-cell-style-name');
+            if ($defaultStyleName !== 'Default' && isset($this->allStyles[$defaultStyleName])) {
+                $tableColumnIndex2 = $tableColumnIndex;
+                $tableColumnString = Coordinate::stringFromColumnIndex($tableColumnIndex2);
+                for ($rowRepeats2 = $rowRepeats; $rowRepeats2 > 0 && $tableColumnIndex2 <= 16384; --$rowRepeats2) {
+                    $spreadsheet->getActiveSheet()
+                        ->getStyle($tableColumnString)
+                        ->applyFromArray(
+                            $this->allStyles[$defaultStyleName]
+                        );
+                    StringHelper::stringIncrement(
+                        $tableColumnString
+                    );
+                    ++$tableColumnIndex2;
+                }
             }
         }
         $tableColumnIndex += $rowRepeats;

src/PhpSpreadsheet/Writer/Ods/Content.php

@@ -142,7 +142,6 @@ class Content extends WriterPart
                     'table:style-name',
                     sprintf('%s_%d_%d', Style::COLUMN_STYLE_PREFIX, $sheetIndex, $columnDimension->getColumnNumeric())
                 );
-                $objWriter->writeAttribute('table:default-cell-style-name', 'ce0');
                 $objWriter->endElement();
             }
             $this->writeRows($objWriter, $spreadsheet->getSheet($sheetIndex), $sheetIndex);

tests/PhpSpreadsheetTests/Reader/Ods/ColGroupTest.php

@@ -0,0 +1,69 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;
+
+use PhpOffice\PhpSpreadsheet\Helper\Dimension;
+use PhpOffice\PhpSpreadsheet\Reader\Ods as OdsReader;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @TODO There is some fuzziness in the conversions to and from column widths
+ */
+class ColGroupTest extends TestCase
+{
+    private const ODS_COLGROUP_FILE = 'tests/data/Reader/Ods/colgroup.ods';
+    private const ODS_COLHEADER_FILE = 'tests/data/Reader/Ods/colheader.ods';
+
+    public function testColGroup(): void
+    {
+        $reader = new OdsReader();
+        $reader->setReadEmptyCells(false);
+        $spreadsheet = $reader->load(self::ODS_COLGROUP_FILE);
+        self::assertSame('Arial', $spreadsheet->getDefaultStyle()->getFont()->getName());
+        $sheet = $spreadsheet->getActiveSheet();
+        $expected = [
+            'A' => 2.778,
+            'B' => 2.99,
+            'C' => 2.91,
+            'D' => 2.858,
+            'E' => 1.746,
+            'F' => 1.746,
+            'G' => 1.746,
+            'H' => 1.746,
+        ];
+        foreach ($expected as $col => $width) {
+            self::assertEqualsWithDelta(
+                $width,
+                $sheet->getColumnDimension($col)
+                    ->getWidth(Dimension::UOM_CENTIMETERS),
+                1.0E-3,
+                "mismatch for column $col"
+            );
+        }
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testColHeader(): void
+    {
+        $reader = new OdsReader();
+        $reader->setReadEmptyCells(false);
+        $spreadsheet = $reader->load(self::ODS_COLHEADER_FILE);
+        self::assertSame('Liberation Sans', $spreadsheet->getDefaultStyle()->getFont()->getName());
+        $sheet = $spreadsheet->getActiveSheet();
+        $expected = [
+            'A' => 2.858,
+        ];
+        foreach ($expected as $col => $width) {
+            self::assertEqualsWithDelta(
+                $width,
+                $sheet->getColumnDimension($col)
+                    ->getWidth(Dimension::UOM_CENTIMETERS),
+                1.0E-3,
+                "mismatch for column $col"
+            );
+        }
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Reader/Ods/CurrencyTest.php

@@ -7,9 +7,6 @@ namespace PhpOffice\PhpSpreadsheetTests\Reader\Ods;
 use PhpOffice\PhpSpreadsheet\Reader\Ods;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @TODO The class doesn't read the bold/italic/underline properties (rich text)
- */
 class CurrencyTest extends TestCase
 {
     private const ODS_CURRENCY_FILE = 'tests/data/Reader/Ods/currency4.ods';

tests/PhpSpreadsheetTests/Reader/Ods/FontStyleTest.php

@@ -121,4 +121,17 @@ class FontStyleTest extends AbstractFunctional
         self::assertSame('Times New Roman', $sheet->getStyle('A1')->getFont()->getName());
         $spreadsheet->disconnectWorksheets();
     }
+
+    public static function testDataOnly(): void
+    {
+        $reader = new OdsReader();
+        $reader->setReadDataOnly(true);
+        $infile = 'tests/data/Reader/Ods/odsstyles5.ods';
+        $spreadsheet = $reader->load($infile);
+        self::assertSame('Calibri', $spreadsheet->getDefaultStyle()->getFont()->getName(), 'ignore spreadsheet style, use PhpSpreadsheet default');
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame('Calibri', $sheet->getStyle('A1')->getFont()->getName());
+        self::assertSame(13, $sheet->getCell('A1')->getValue());
+        $spreadsheet->disconnectWorksheets();
+    }
 }

tests/PhpSpreadsheetTests/Reader/Ods/Issue3721Test.php

@@ -9,7 +9,7 @@ use PHPUnit\Framework\TestCase;
 
 class Issue3721Test extends TestCase
 {
-    public function testIssue2810(): void
+    public function xtestIssue2810ReadEmpty(): void
     {
         // Problems with getHighestDataColumn
         $filename = 'tests/data/Reader/Ods/issue.3721.ods';
@@ -29,4 +29,26 @@ class Issue3721Test extends TestCase
         self::assertSame('BL', $sheet->getHighestColumn());
         $spreadsheet->disconnectWorksheets();
     }
+
+    public function testIssue2810NoReadEmpty(): void
+    {
+        // Problems with getHighestDataColumn
+        $filename = 'tests/data/Reader/Ods/issue.3721.ods';
+        $reader = new Ods();
+        $reader->setReadEmptyCells(false);
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getSheetByNameOrThrow('sheet with data');
+        $origHigh = $sheet->getHighestDataColumn();
+        self::assertSame('C', $origHigh);
+        $cells = [];
+        foreach ($sheet->getRowIterator() as $row) {
+            foreach ($row->getCellIterator(iterateOnlyExistingCells: true) as $cell) {
+                $cells[] = $cell->getCoordinate();
+            }
+        }
+        self::assertSame(['A1', 'B1', 'C1', 'A2', 'B2', 'C2'], $cells);
+        self::assertSame('C', $sheet->getHighestDataColumn());
+        self::assertSame('C', $sheet->getHighestColumn());
+        $spreadsheet->disconnectWorksheets();
+    }
 }

tests/PhpSpreadsheetTests/Reader/Ods/NestedTableRowTest.php

@@ -13,6 +13,7 @@ class NestedTableRowTest extends TestCase
     {
         $infile = 'tests/data/Reader/Ods/issue.4528.ods';
         $reader = new OdsReader();
+        $reader->setReadDataOnly(true)->setReadEmptyCells(false);
         $spreadsheet = $reader->load($infile);
         $sheet = $spreadsheet->getActiveSheet();
         self::assertSame('Atterissage', $sheet->getCell('AS1')->getValue());
@@ -26,6 +27,7 @@ class NestedTableRowTest extends TestCase
     {
         $infile = 'tests/data/Reader/Ods/issue.2507.ods';
         $reader = new OdsReader();
+        $reader->setReadDataOnly(true)->setReadEmptyCells(false);
         $spreadsheet = $reader->load($infile);
         $sheet = $spreadsheet->getActiveSheet();
         $values = $sheet->rangeToArray('B3:C7', null, false, false);

tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php

@@ -8,9 +8,6 @@ use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException;
 use PhpOffice\PhpSpreadsheet\Reader\Ods;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @TODO The class doesn't read the bold/italic/underline properties (rich text)
- */
 class OdsInfoTest extends TestCase
 {
     public function testReadFileProperties(): void

tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php

@@ -12,9 +12,6 @@ use PhpOffice\PhpSpreadsheet\Spreadsheet;
 use PhpOffice\PhpSpreadsheet\Style\Font;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @TODO The class doesn't read the bold/italic/underline properties (rich text)
- */
 class OdsTest extends TestCase
 {
     private const ODS_TEST_FILE = 'samples/templates/OOCalcTest.ods';