From b1f901456706bec36d47872d7d2af4c330a8bead Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sun, 14 Dec 2025 01:21:28 -0800 Subject: [PATCH] Fix Ods ListWorksheetInfo It wasn't handling skipped rows and columns correctly. --- src/PhpSpreadsheet/Reader/Ods.php | 16 ++--- .../Reader/Ods/OdsInfoTest.php | 60 +++++++++++-------- 2 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index d243f4db0..ecbf4d5e9 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -187,28 +187,31 @@ class Ods extends BaseReader ]; // Loop through each child node of the table:table element reading - $currCells = 0; + $currRow = 0; do { $xml->read(); if ($xml->name == 'table:table-row' && $xml->nodeType == XMLReader::ELEMENT) { $rowspan = $xml->getAttribute('table:number-rows-repeated'); $rowspan = empty($rowspan) ? 1 : (int) $rowspan; - $tmpInfo['totalRows'] += $rowspan; - $tmpInfo['totalColumns'] = max($tmpInfo['totalColumns'], $currCells); - $currCells = 0; + $currRow += $rowspan; + $currCol = 0; // Step into the row $xml->read(); do { $doread = true; if ($xml->name == 'table:table-cell' && $xml->nodeType == XMLReader::ELEMENT) { + $mergeSize = $xml->getAttribute('table:number-columns-repeated'); + $mergeSize = empty($mergeSize) ? 1 : (int) $mergeSize; + $currCol += $mergeSize; if (!$xml->isEmptyElement) { - ++$currCells; + $tmpInfo['totalColumns'] = max($tmpInfo['totalColumns'], $currCol); + $tmpInfo['totalRows'] = $currRow; $xml->next(); $doread = false; } } elseif ($xml->name == 'table:covered-table-cell' && $xml->nodeType == XMLReader::ELEMENT) { $mergeSize = $xml->getAttribute('table:number-columns-repeated'); - $currCells += (int) $mergeSize; + $currCol += (int) $mergeSize; } if ($doread) { $xml->read(); @@ -217,7 +220,6 @@ class Ods extends BaseReader } } while ($xml->name != 'table:table'); - $tmpInfo['totalColumns'] = max($tmpInfo['totalColumns'], $currCells); $tmpInfo['lastColumnIndex'] = $tmpInfo['totalColumns'] - 1; $tmpInfo['lastColumnLetter'] = Coordinate::stringFromColumnIndex($tmpInfo['lastColumnIndex'] + 1); $worksheetInfo[] = $tmpInfo; diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php index 03e80356c..1ba0847d7 100644 --- a/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php @@ -22,7 +22,7 @@ class OdsInfoTest extends TestCase // Test "listWorksheetNames" method - self::assertEquals([ + self::assertSame([ 'Sheet1', 'Second Sheet', ], $reader->listWorksheetNames($filename)); @@ -47,7 +47,7 @@ class OdsInfoTest extends TestCase // Test "listWorksheetNames" method - self::assertEquals([ + self::assertSame([ 'Sheet1', 'Second Sheet', ], $reader->listWorksheetNames(__FILE__)); @@ -56,14 +56,9 @@ class OdsInfoTest extends TestCase public function testReadFileInfo(): void { $filename = 'tests/data/Reader/Ods/data.ods'; - - // Load into this instance $reader = new Ods(); - - // Test "listWorksheetNames" method - $wsinfo = $reader->listWorkSheetInfo($filename); - self::assertEquals([ + self::assertSame([ [ 'worksheetName' => 'Sheet1', 'lastColumnLetter' => 'C', @@ -74,10 +69,10 @@ class OdsInfoTest extends TestCase ], [ 'worksheetName' => 'Second Sheet', - 'lastColumnLetter' => 'A', - 'lastColumnIndex' => 0, + 'lastColumnLetter' => 'B', + 'lastColumnIndex' => 1, 'totalRows' => 2, - 'totalColumns' => 1, + 'totalColumns' => 2, 'sheetState' => 'visible', ], ], $wsinfo); @@ -87,27 +82,40 @@ class OdsInfoTest extends TestCase { $this->expectException(ReaderException::class); $filename = __FILE__; - - // Load into this instance $reader = new Ods(); - - // Test "listWorksheetNames" method - $wsinfo = $reader->listWorkSheetInfo($filename); - self::assertEquals([ + } + + public function testReadFileInfoWithEmpties(): void + { + $filename = 'tests/data/Reader/Ods/RepeatedCells.ods'; + $reader = new Ods(); + $wsinfo = $reader->listWorkSheetInfo($filename); + self::assertSame([ [ 'worksheetName' => 'Sheet1', - 'lastColumnLetter' => 'C', - 'lastColumnIndex' => 2, - 'totalRows' => 11, - 'totalColumns' => 3, + 'lastColumnLetter' => 'K', + 'lastColumnIndex' => 10, + 'totalRows' => 1, + 'totalColumns' => 11, + 'sheetState' => 'visible', ], + ], $wsinfo); + } + + public function testOneMoreWorksheetInfo(): void + { + $filename = 'tests/data/Reader/Ods/issue.4528.ods'; + $reader = new Ods(); + $wsinfo = $reader->listWorkSheetInfo($filename); + self::assertSame([ [ - 'worksheetName' => 'Second Sheet', - 'lastColumnLetter' => 'A', - 'lastColumnIndex' => 0, - 'totalRows' => 2, - 'totalColumns' => 1, + 'worksheetName' => 'Francais', + 'lastColumnLetter' => 'AZ', + 'lastColumnIndex' => 51, + 'totalRows' => 811, + 'totalColumns' => 52, + 'sheetState' => 'visible', ], ], $wsinfo); }