From e40438916f5c29d13f8f98d175a0595b5b7b0422 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 25 Jun 2024 22:06:36 -0700 Subject: [PATCH 01/30] Change Style Without Affecting Current Cell/Sheet, and Invalid Formulas Fix #1310, which was closed as stale in 2020, but which I will now reopen. Supersedes PR #1311 (@jaiminmoslake7020), from which I will remove the stale label but leave closed. The issue and the PR were too limited - they detected that the use of two equal signs at the start of a string made for an invalid formula, but there are variations, trivial and otherwise, which might also be detected. Using `setValue` with a string which starts with an equal sign will now attempt to parse (not evaluate) the formula; for certain situations in which the parser throws an exception, the string will be treated as a string rather than a formula. An example where it will still be treated as a formula is a 3D range reference, where the problem is not that it can't be parsed, but rather that the formula isn't supported (see unit test Calculation/Engine/RangeTest::test3dRangeEvaluation). Allowing such a formula might cause problems later on, but that is already what happens. A string beginning with an equal sign but which isn't treated as a formula will automatically set the `quotePrefix` attribute to `true`; all other `setValue` attempts will set it to `false`. This avoids the problem of a lingering value causing problems later on. It has long been a matter of discontent that setting a style can change the selected cells. A new method is added to `Worksheet`: ```php applyStylesFromArray(string $coordinate, array $styleArray) ``` This will attempt to guarantee that the active sheet in the current spreadsheet, and the selected cells in the current worksheet, remain undisturbed after the call. The setting of `quotePrefix` above is the first use of the new method. --- .../Calculation/Calculation.php | 2 +- src/PhpSpreadsheet/Cell/Cell.php | 6 +++ .../Cell/DefaultValueBinder.php | 40 +++++++++++++++---- src/PhpSpreadsheet/Cell/StringValueBinder.php | 7 +--- src/PhpSpreadsheet/Worksheet/Worksheet.php | 15 +++++++ .../Calculation/Engine/RangeTest.php | 35 ++++++++++++++-- .../Cell/AdvancedValueBinderTest.php | 28 +++++++++++++ .../Cell/StringValueBinderTest.php | 8 +++- tests/data/Cell/DefaultValueBinder.php | 3 ++ 9 files changed, 126 insertions(+), 18 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 6028a9d72..f445717d0 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -4065,7 +4065,7 @@ class Calculation $opCharacter = $formula[$index]; // Get the first character of the value at the current index position // Check for two-character operators (e.g. >=, <=, <>) - if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && (isset(self::$comparisonOperators[$formula[$index + 1]]))) { + if ((isset(self::$comparisonOperators[$opCharacter])) && (strlen($formula) > $index) && isset($formula[$index + 1], self::$comparisonOperators[$formula[$index + 1]])) { $opCharacter .= $formula[++$index]; } // Find out if we're currently at the beginning of a number, variable, cell/row/column reference, diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php index 3dc6c2eab..bf0b681ac 100644 --- a/src/PhpSpreadsheet/Cell/Cell.php +++ b/src/PhpSpreadsheet/Cell/Cell.php @@ -248,6 +248,7 @@ class Cell implements Stringable public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE_STRING): self { $oldValue = $this->value; + $quotePrefix = false; // set the value according to data type switch ($dataType) { @@ -260,6 +261,10 @@ class Cell implements Stringable // no break case DataType::TYPE_STRING: // Synonym for string + if (is_string($value) && strlen($value) > 1 && $value[0] === '=') { + $quotePrefix = true; + } + // no break case DataType::TYPE_INLINE: // Rich text $this->value = DataType::checkString($value); @@ -299,6 +304,7 @@ class Cell implements Stringable $this->updateInCollection(); $cellCoordinate = $this->getCoordinate(); self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value); + $this->getWorksheet()->applyStylesFromArray($cellCoordinate, ['quotePrefix' => $quotePrefix]); return $this->getParent()?->get($cellCoordinate) ?? $this; } diff --git a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php index 2710ead43..da5138e7d 100644 --- a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php +++ b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php @@ -3,6 +3,8 @@ namespace PhpOffice\PhpSpreadsheet\Cell; use DateTimeInterface; +use PhpOffice\PhpSpreadsheet\Calculation\Calculation; +use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException; use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; use PhpOffice\PhpSpreadsheet\RichText\RichText; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; @@ -46,17 +48,40 @@ class DefaultValueBinder implements IValueBinder // Match the value against a few data types if ($value === null) { return DataType::TYPE_NULL; - } elseif (is_float($value) || is_int($value)) { + } + if (is_float($value) || is_int($value)) { return DataType::TYPE_NUMERIC; - } elseif (is_bool($value)) { + } + if (is_bool($value)) { return DataType::TYPE_BOOL; - } elseif ($value === '') { + } + if ($value === '') { return DataType::TYPE_STRING; - } elseif ($value instanceof RichText) { + } + if ($value instanceof RichText) { return DataType::TYPE_INLINE; - } elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=') { + } + if (is_string($value) && strlen($value) > 1 && $value[0] === '=') { + $calculation = new Calculation(); + $calculation->disableBranchPruning(); + + try { + if (empty($calculation->parseFormula($value))) { + return DataType::TYPE_STRING; + } + } catch (CalculationException $e) { + $message = $e->getMessage(); + if ( + $message === 'Formula Error: An unexpected error occurred' + || str_contains($message, 'has no operands') + ) { + return DataType::TYPE_STRING; + } + } + return DataType::TYPE_FORMULA; - } elseif (preg_match('/^[\+\-]?(\d+\\.?\d*|\d*\\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) { + } + if (preg_match('/^[\+\-]?(\d+\\.?\d*|\d*\\.?\d+)([Ee][\-\+]?[0-2]?\d{1,3})?$/', $value)) { $tValue = ltrim($value, '+-'); if (is_string($value) && strlen($tValue) > 1 && $tValue[0] === '0' && $tValue[1] !== '.') { return DataType::TYPE_STRING; @@ -67,7 +92,8 @@ class DefaultValueBinder implements IValueBinder } return DataType::TYPE_NUMERIC; - } elseif (is_string($value)) { + } + if (is_string($value)) { $errorCodes = DataType::getErrorCodes(); if (isset($errorCodes[$value])) { return DataType::TYPE_ERROR; diff --git a/src/PhpSpreadsheet/Cell/StringValueBinder.php b/src/PhpSpreadsheet/Cell/StringValueBinder.php index 6ff258d93..d86cdabd3 100644 --- a/src/PhpSpreadsheet/Cell/StringValueBinder.php +++ b/src/PhpSpreadsheet/Cell/StringValueBinder.php @@ -8,7 +8,7 @@ use PhpOffice\PhpSpreadsheet\RichText\RichText; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; use Stringable; -class StringValueBinder implements IValueBinder +class StringValueBinder extends DefaultValueBinder implements IValueBinder { protected bool $convertNull = true; @@ -87,12 +87,9 @@ class StringValueBinder implements IValueBinder $cell->setValueExplicit($value, DataType::TYPE_BOOL); } elseif ((is_int($value) || is_float($value)) && $this->convertNumeric === false) { $cell->setValueExplicit($value, DataType::TYPE_NUMERIC); - } elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=' && $this->convertFormula === false) { + } elseif (is_string($value) && strlen($value) > 1 && $value[0] === '=' && $this->convertFormula === false && parent::dataTypeForValue($value) === DataType::TYPE_FORMULA) { $cell->setValueExplicit($value, DataType::TYPE_FORMULA); } else { - if (is_string($value) && strlen($value) > 1 && $value[0] === '=') { - $cell->getStyle()->setQuotePrefix(true); - } $cell->setValueExplicit((string) $value, DataType::TYPE_STRING); } diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 8adada4c1..0bb64ba59 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -3673,4 +3673,19 @@ class Worksheet implements IComparable } } } + + public function applyStylesFromArray(string $coordinate, array $styleArray): bool + { + $spreadsheet = $this->parent; + if ($spreadsheet === null) { + return false; + } + $activeSheetIndex = $spreadsheet->getActiveSheetIndex(); + $originalSelected = $this->selectedCells; + $this->getStyle($coordinate)->applyFromArray($styleArray); + $this->selectedCells = $originalSelected; + $spreadsheet->setActiveSheetIndex($activeSheetIndex); + + return true; + } } diff --git a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php index fbac0ff91..aa7bc529d 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Engine/RangeTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace PhpOffice\PhpSpreadsheetTests\Calculation\Engine; +use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; use PhpOffice\PhpSpreadsheet\NamedRange; use PhpOffice\PhpSpreadsheet\Spreadsheet; @@ -13,13 +14,23 @@ class RangeTest extends TestCase { private string $incompleteMessage = 'Must be revisited'; - private Spreadsheet $spreadSheet; + private ?Spreadsheet $spreadSheet = null; - protected function setUp(): void + protected function getSpreadsheet(): Spreadsheet { - $this->spreadSheet = new Spreadsheet(); - $this->spreadSheet->getActiveSheet() + $spreadsheet = new Spreadsheet(); + $spreadsheet->getActiveSheet() ->fromArray(array_chunk(range(1, 240), 6), null, 'A1', true); + + return $spreadsheet; + } + + protected function tearDown(): void + { + if ($this->spreadSheet !== null) { + $this->spreadSheet->disconnectWorksheets(); + $this->spreadSheet = null; + } } /** @@ -27,6 +38,7 @@ class RangeTest extends TestCase */ public function testRangeEvaluation(string $formula, int|string $expectedResult): void { + $this->spreadSheet = $this->getSpreadsheet(); $workSheet = $this->spreadSheet->getActiveSheet(); $workSheet->setCellValue('H1', $formula); @@ -64,8 +76,20 @@ class RangeTest extends TestCase ]; } + public function test3dRangeParsing(): void + { + // This test shows that parsing throws exception. + // Next test shows that formula is still treated as a formula + // despite the parse failure. + $this->expectExceptionMessage('3D Range references are not yet supported'); + $calculation = new Calculation(); + $calculation->disableBranchPruning(); + $calculation->parseFormula('=SUM(Worksheet!A1:Worksheet2!B3'); + } + public function test3dRangeEvaluation(): void { + $this->spreadSheet = $this->getSpreadsheet(); $workSheet = $this->spreadSheet->getActiveSheet(); $workSheet->setCellValue('E1', '=SUM(Worksheet!A1:Worksheet2!B3)'); @@ -78,6 +102,7 @@ class RangeTest extends TestCase */ public function testNamedRangeEvaluation(array $ranges, string $formula, int $expectedResult): void { + $this->spreadSheet = $this->getSpreadsheet(); $workSheet = $this->spreadSheet->getActiveSheet(); foreach ($ranges as $id => $range) { $this->spreadSheet->addNamedRange(new NamedRange('GROUP' . ++$id, $workSheet, $range)); @@ -116,6 +141,7 @@ class RangeTest extends TestCase */ public function testUTF8NamedRangeEvaluation(array $names, array $ranges, string $formula, int $expectedResult): void { + $this->spreadSheet = $this->getSpreadsheet(); $workSheet = $this->spreadSheet->getActiveSheet(); foreach ($names as $index => $name) { $range = $ranges[$index]; @@ -144,6 +170,7 @@ class RangeTest extends TestCase if ($this->incompleteMessage !== '') { self::markTestIncomplete($this->incompleteMessage); } + $this->spreadSheet = $this->getSpreadsheet(); $workSheet = $this->spreadSheet->getActiveSheet(); $this->spreadSheet->addNamedRange(new NamedRange('COMPOSITE', $workSheet, $composite)); diff --git a/tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php b/tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php index d3f0ad664..d8de97a33 100644 --- a/tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php +++ b/tests/PhpSpreadsheetTests/Cell/AdvancedValueBinderTest.php @@ -6,6 +6,7 @@ namespace PhpOffice\PhpSpreadsheetTests\Cell; use PhpOffice\PhpSpreadsheet\Cell\AdvancedValueBinder; use PhpOffice\PhpSpreadsheet\Cell\Cell; +use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\Cell\IValueBinder; use PhpOffice\PhpSpreadsheet\Settings; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; @@ -232,4 +233,31 @@ class AdvancedValueBinderTest extends TestCase ["Hello\nWorld", true], ]; } + + /** + * @dataProvider formulaProvider + */ + public function testFormula(string $value, string $dataType): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + + $sheet->getCell('A1')->setValue($value); + self::assertSame($dataType, $sheet->getCell('A1')->getDataType()); + if ($dataType === DataType::TYPE_FORMULA) { + self::assertFalse($sheet->getStyle('A1')->getQuotePrefix()); + } else { + self::assertTrue($sheet->getStyle('A1')->getQuotePrefix()); + } + + $spreadsheet->disconnectWorksheets(); + } + + public static function formulaProvider(): array + { + return [ + 'normal formula' => ['=SUM(A1:C3)', DataType::TYPE_FORMULA], + 'issue 1310' => ['======', DataType::TYPE_STRING], + ]; + } } diff --git a/tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php b/tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php index 43ac2fbd1..71006fff8 100644 --- a/tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php +++ b/tests/PhpSpreadsheetTests/Cell/StringValueBinderTest.php @@ -211,13 +211,19 @@ class StringValueBinderTest extends TestCase $cell->setValue($value); self::assertSame($expectedValue, $cell->getValue()); self::assertSame($expectedDataType, $cell->getDataType()); + if ($expectedDataType === DataType::TYPE_FORMULA) { + self::assertFalse($sheet->getStyle('A1')->getQuotePrefix()); + } else { + self::assertTrue($sheet->getStyle('A1')->getQuotePrefix()); + } $spreadsheet->disconnectWorksheets(); } public static function providerDataValuesSuppressFormulaConversion(): array { return [ - ['=SUM(A1:C3)', '=SUM(A1:C3)', DataType::TYPE_FORMULA, false], + 'normal formula' => ['=SUM(A1:C3)', '=SUM(A1:C3)', DataType::TYPE_FORMULA], + 'issue 1310' => ['======', '======', DataType::TYPE_STRING], ]; } diff --git a/tests/data/Cell/DefaultValueBinder.php b/tests/data/Cell/DefaultValueBinder.php index b9f3d51e1..bd4295c0b 100644 --- a/tests/data/Cell/DefaultValueBinder.php +++ b/tests/data/Cell/DefaultValueBinder.php @@ -83,4 +83,7 @@ return [ 's', '1234567890123459012345689012345690', ], + 'Issue 1310 Multiple = at start' => ['s', '======'], + 'Issue 1310 Variant 1' => ['s', '= ====='], + 'Issue 1310 Variant 2' => ['s', '=2*3='], ]; From 352872048be062f5a0b78cb179df23928a0b4a9c Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Tue, 25 Jun 2024 23:16:10 -0700 Subject: [PATCH 02/30] Resolve Merge Conflicts --- .../Cell/DefaultValueBinder.php | 18 ++++++++++----- .../Cell/DefaultValueBinderTest.php | 23 +++++++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php index da5138e7d..e6f97317e 100644 --- a/src/PhpSpreadsheet/Cell/DefaultValueBinder.php +++ b/src/PhpSpreadsheet/Cell/DefaultValueBinder.php @@ -61,7 +61,15 @@ class DefaultValueBinder implements IValueBinder if ($value instanceof RichText) { return DataType::TYPE_INLINE; } - if (is_string($value) && strlen($value) > 1 && $value[0] === '=') { + if ($value instanceof Stringable) { + $value = (string) $value; + } + if (!is_string($value)) { + $gettype = is_object($value) ? get_class($value) : gettype($value); + + throw new SpreadsheetException("unusable type $gettype"); + } + if (strlen($value) > 1 && $value[0] === '=') { $calculation = new Calculation(); $calculation->disableBranchPruning(); @@ -93,11 +101,9 @@ class DefaultValueBinder implements IValueBinder return DataType::TYPE_NUMERIC; } - if (is_string($value)) { - $errorCodes = DataType::getErrorCodes(); - if (isset($errorCodes[$value])) { - return DataType::TYPE_ERROR; - } + $errorCodes = DataType::getErrorCodes(); + if (isset($errorCodes[$value])) { + return DataType::TYPE_ERROR; } return DataType::TYPE_STRING; diff --git a/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php b/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php index fb9b42dbe..8cb5d8876 100644 --- a/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php +++ b/tests/PhpSpreadsheetTests/Cell/DefaultValueBinderTest.php @@ -108,4 +108,27 @@ class DefaultValueBinderTest extends TestCase self::assertTrue($binder::$called); $spreadsheet->disconnectWorksheets(); } + + public function testDataTypeForValueExceptions(): void + { + try { + self::assertSame('s', DefaultValueBinder::dataTypeForValue(new SpreadsheetException())); + } catch (SpreadsheetException $e) { + self::fail('Should not have failed for stringable'); + } + + try { + DefaultValueBinder::dataTypeForValue([]); + self::fail('Should have failed for array'); + } catch (SpreadsheetException $e) { + self::assertStringContainsString('unusable type array', $e->getMessage()); + } + + try { + DefaultValueBinder::dataTypeForValue(new DateTime()); + self::fail('Should have failed for DateTime'); + } catch (SpreadsheetException $e) { + self::assertStringContainsString('unusable type DateTime', $e->getMessage()); + } + } } From 4b04cc1c8d6c539967effa523a907bf2daaf5cfb Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 29 Jun 2024 22:00:39 -0700 Subject: [PATCH 03/30] Propagate Errors in Text Functions Fix #2581 (not obvious - see next paragraph for explanation). This continues the work of PR #2902 (and also PR #3467) to have errors propagated through function calculations rather than treating them as strings. All text functions, and the concatenation operator, are addressed in this PR. In the original issue, the spreadsheet being loaded uses the result of an unimplemented function as an argument to another function. When `getCalculatedValue` is used on the cell in question, the result is returned as `#VALUE!`. If the cell had just contained a function call to the unimplemented function, getCalculatedValue would have recognized the situation and returned oldCalculatedValue as the result. Not perfect, but good enough most of the time. User would like oldCalculatedValue returned here as well, which seems like a reasonable request. PhpSpreadsheet always returns `#Not Yet Implemented` as the result for a function which it knows about but which is not yet implemented. That is the key to the `Cell` class being able to substitute oldCalculatedValue in the first place. However, in order to do that for the issue in question, that result has to be propagated to any functions for which the result is an argument. I don't want to add unimplemented to the list of known error codes, but I am willing to add a parameter to `ErrorValue::isError` to indicate whether that value should be considered an error (default is "no"). The first use of that new parameter would be by the text functions. They go through a common Helper routine, so it is pretty easily implemented. And, as it turns out, most of the text functions do not currently propagate errors, e.g. if A1 results in a value error, `=LEFT(A1,2)` will result in `#V` rather than `#VALUE!`. With this PR, they will now be handled correctly. --- .../Calculation/Calculation.php | 29 ++++-- src/PhpSpreadsheet/Calculation/Functions.php | 4 +- .../Calculation/Information/ErrorValue.php | 6 +- .../Calculation/TextData/CaseConvert.php | 19 +++- .../Calculation/TextData/CharacterConvert.php | 15 ++- .../Calculation/TextData/Concatenate.php | 6 +- .../Calculation/TextData/Extract.php | 22 +++- .../Calculation/TextData/Format.php | 15 ++- .../Calculation/TextData/Helpers.php | 14 ++- .../Calculation/TextData/Search.php | 8 +- .../Calculation/TextData/Text.php | 26 +++-- src/PhpSpreadsheet/Cell/Cell.php | 3 +- .../TextData/ErrorPropagationTest.php | 96 ++++++++++++++++++ .../Reader/Xlsx/Issue2581Test.php | 25 +++++ tests/data/Reader/XLSX/issue.2581.xlsx | Bin 0 -> 9189 bytes 15 files changed, 248 insertions(+), 40 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ErrorPropagationTest.php create mode 100644 tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2581Test.php create mode 100644 tests/data/Reader/XLSX/issue.2581.xlsx diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 6028a9d72..e60c12ed7 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -4792,13 +4792,20 @@ class Calculation for ($row = 0; $row < $rows; ++$row) { for ($column = 0; $column < $columns; ++$column) { - $operand1[$row][$column] - = Shared\StringHelper::substring( - self::boolToString($operand1[$row][$column]) - . self::boolToString($operand2[$row][$column]), - 0, - DataType::MAX_STRING_LENGTH - ); + $op1x = self::boolToString($operand1[$row][$column]); + $op2x = self::boolToString($operand2[$row][$column]); + if (Information\ErrorValue::isError($op1x)) { + // no need to do anything + } elseif (Information\ErrorValue::isError($op2x)) { + $operand1[$row][$column] = $op2x; + } else { + $operand1[$row][$column] + = Shared\StringHelper::substring( + $op1x . $op2x, + 0, + DataType::MAX_STRING_LENGTH + ); + } } } $result = $operand1; @@ -4808,7 +4815,13 @@ class Calculation // using the concatenation operator // with literals that fits in 32K, // so I don't think we can overflow here. - $result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE; + if (Information\ErrorValue::isError($operand1)) { + $result = $operand1; + } elseif (Information\ErrorValue::isError($operand2)) { + $result = $operand2; + } else { + $result = self::FORMULA_STRING_QUOTE . str_replace('""', self::FORMULA_STRING_QUOTE, self::unwrapResult($operand1) . self::unwrapResult($operand2)) . self::FORMULA_STRING_QUOTE; + } } $this->debugLog->writeDebugLog('Evaluation Result is %s', $this->showTypeDetails($result)); $stack->push('Value', $result); diff --git a/src/PhpSpreadsheet/Calculation/Functions.php b/src/PhpSpreadsheet/Calculation/Functions.php index 6b7450069..77f8317ae 100644 --- a/src/PhpSpreadsheet/Calculation/Functions.php +++ b/src/PhpSpreadsheet/Calculation/Functions.php @@ -26,6 +26,8 @@ class Functions const RETURNDATE_PHP_DATETIME_OBJECT = 'O'; const RETURNDATE_EXCEL = 'E'; + public const NOT_YET_IMPLEMENTED = '#Not Yet Implemented'; + /** * Compatibility mode to use for error checking and responses. */ @@ -123,7 +125,7 @@ class Functions */ public static function DUMMY(): string { - return '#Not Yet Implemented'; + return self::NOT_YET_IMPLEMENTED; } public static function isMatrixValue(mixed $idx): bool diff --git a/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php b/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php index dcef43990..f3a746273 100644 --- a/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php +++ b/src/PhpSpreadsheet/Calculation/Information/ErrorValue.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation\Information; use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled; +use PhpOffice\PhpSpreadsheet\Calculation\Functions; class ErrorValue { @@ -35,7 +36,7 @@ class ErrorValue * @return array|bool If an array of numbers is passed as an argument, then the returned result will also be an array * with the same dimensions */ - public static function isError(mixed $value = ''): array|bool + public static function isError(mixed $value = '', bool $tryNotImplemented = false): array|bool { if (is_array($value)) { return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $value); @@ -44,6 +45,9 @@ class ErrorValue if (!is_string($value)) { return false; } + if ($tryNotImplemented && $value === Functions::NOT_YET_IMPLEMENTED) { + return true; + } return in_array($value, ExcelError::ERROR_CODES, true); } diff --git a/src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php b/src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php index 83cc4ee1f..6667bac54 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php +++ b/src/PhpSpreadsheet/Calculation/TextData/CaseConvert.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation\TextData; use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled; +use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp; use PhpOffice\PhpSpreadsheet\Shared\StringHelper; class CaseConvert @@ -26,7 +27,11 @@ class CaseConvert return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $mixedCaseValue); } - $mixedCaseValue = Helpers::extractString($mixedCaseValue); + try { + $mixedCaseValue = Helpers::extractString($mixedCaseValue, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } return StringHelper::strToLower($mixedCaseValue); } @@ -48,7 +53,11 @@ class CaseConvert return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $mixedCaseValue); } - $mixedCaseValue = Helpers::extractString($mixedCaseValue); + try { + $mixedCaseValue = Helpers::extractString($mixedCaseValue, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } return StringHelper::strToUpper($mixedCaseValue); } @@ -70,7 +79,11 @@ class CaseConvert return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $mixedCaseValue); } - $mixedCaseValue = Helpers::extractString($mixedCaseValue); + try { + $mixedCaseValue = Helpers::extractString($mixedCaseValue, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } return StringHelper::strToTitle($mixedCaseValue); } diff --git a/src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php b/src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php index 8d90a17ba..06d0f9009 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php +++ b/src/PhpSpreadsheet/Calculation/TextData/CharacterConvert.php @@ -3,6 +3,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation\TextData; use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled; +use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp; use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; @@ -26,7 +27,12 @@ class CharacterConvert return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $character); } - $character = Helpers::validateInt($character); + try { + $character = Helpers::validateInt($character, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } + $min = Functions::getCompatibilityMode() === Functions::COMPATIBILITY_OPENOFFICE ? 0 : 1; if ($character < $min || $character > 255) { return ExcelError::VALUE(); @@ -52,7 +58,12 @@ class CharacterConvert return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $characters); } - $characters = Helpers::extractString($characters); + try { + $characters = Helpers::extractString($characters, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } + if ($characters === '') { return ExcelError::VALUE(); } diff --git a/src/PhpSpreadsheet/Calculation/TextData/Concatenate.php b/src/PhpSpreadsheet/Calculation/TextData/Concatenate.php index c2281d436..78940ed16 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Concatenate.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Concatenate.php @@ -27,7 +27,7 @@ class Concatenate foreach ($aArgs as $arg) { $value = Helpers::extractString($arg); - if (ErrorValue::isError($value)) { + if (ErrorValue::isError($value, true)) { $returnValue = $value; break; @@ -85,7 +85,7 @@ class Concatenate { foreach ($aArgs as $key => &$arg) { $value = Helpers::extractString($arg); - if (ErrorValue::isError($value)) { + if (ErrorValue::isError($value, true)) { return $value; } @@ -123,7 +123,7 @@ class Concatenate if (!is_numeric($repeatCount) || $repeatCount < 0) { $returnValue = ExcelError::VALUE(); - } elseif (ErrorValue::isError($stringValue)) { + } elseif (ErrorValue::isError($stringValue, true)) { $returnValue = $stringValue; } else { $returnValue = str_repeat($stringValue, (int) $repeatCount); diff --git a/src/PhpSpreadsheet/Calculation/TextData/Extract.php b/src/PhpSpreadsheet/Calculation/TextData/Extract.php index 32e5b967a..1dfb724cd 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Extract.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Extract.php @@ -31,7 +31,7 @@ class Extract } try { - $value = Helpers::extractString($value); + $value = Helpers::extractString($value, true); $chars = Helpers::extractInt($chars, 0, 1); } catch (CalcExp $e) { return $e->getMessage(); @@ -61,7 +61,7 @@ class Extract } try { - $value = Helpers::extractString($value); + $value = Helpers::extractString($value, true); $start = Helpers::extractInt($start, 1); $chars = Helpers::extractInt($chars, 0); } catch (CalcExp $e) { @@ -90,7 +90,7 @@ class Extract } try { - $value = Helpers::extractString($value); + $value = Helpers::extractString($value, true); $chars = Helpers::extractInt($chars, 0, 1); } catch (CalcExp $e) { return $e->getMessage(); @@ -132,7 +132,13 @@ class Extract return self::evaluateArrayArgumentsIgnore([self::class, __FUNCTION__], 1, $text, $delimiter, $instance, $matchMode, $matchEnd, $ifNotFound); } - $text = Helpers::extractString($text ?? ''); + try { + $text = Helpers::extractString($text ?? '', true); + Helpers::extractString(Functions::flattenSingleValue($delimiter ?? ''), true); + } catch (CalcExp $e) { + return $e->getMessage(); + } + $instance = (int) $instance; $matchMode = (int) $matchMode; $matchEnd = (int) $matchEnd; @@ -190,7 +196,13 @@ class Extract return self::evaluateArrayArgumentsIgnore([self::class, __FUNCTION__], 1, $text, $delimiter, $instance, $matchMode, $matchEnd, $ifNotFound); } - $text = Helpers::extractString($text ?? ''); + try { + $text = Helpers::extractString($text ?? '', true); + Helpers::extractString(Functions::flattenSingleValue($delimiter ?? ''), true); + } catch (CalcExp $e) { + return $e->getMessage(); + } + $instance = (int) $instance; $matchMode = (int) $matchMode; $matchEnd = (int) $matchEnd; diff --git a/src/PhpSpreadsheet/Calculation/TextData/Format.php b/src/PhpSpreadsheet/Calculation/TextData/Format.php index 40335ced7..0560b376b 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Format.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Format.php @@ -8,6 +8,7 @@ use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\DateTimeExcel; use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp; use PhpOffice\PhpSpreadsheet\Calculation\Functions; +use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue; use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; use PhpOffice\PhpSpreadsheet\Calculation\MathTrig; use PhpOffice\PhpSpreadsheet\RichText\RichText; @@ -123,8 +124,13 @@ class Format return self::evaluateArrayArguments([self::class, __FUNCTION__], $value, $format); } - $value = Helpers::extractString($value); - $format = Helpers::extractString($format); + try { + $value = Helpers::extractString($value, true); + $format = Helpers::extractString($format, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } + $format = (string) NumberFormat::convertSystemFormats($format); if (!is_numeric($value) && Date::isDateTimeFormatCode($format)) { @@ -152,6 +158,9 @@ class Format } if (is_string($value)) { $value = trim($value); + if (ErrorValue::isError($value, true)) { + throw new CalcExp($value); + } if ($spacesMeanZero && $value === '') { $value = 0; } @@ -220,7 +229,7 @@ class Format } /** - * TEXT. + * VALUETOTEXT. * * @param mixed $value The value to format * Or can be an array of values diff --git a/src/PhpSpreadsheet/Calculation/TextData/Helpers.php b/src/PhpSpreadsheet/Calculation/TextData/Helpers.php index 15b046704..719de04a8 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Helpers.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Helpers.php @@ -27,7 +27,7 @@ class Helpers if (is_bool($value)) { return self::convertBooleanValue($value); } - if ($throwIfError && is_string($value) && ErrorValue::isError($value)) { + if ($throwIfError && is_string($value) && ErrorValue::isError($value, true)) { throw new CalcExp($value); } @@ -63,18 +63,28 @@ class Helpers $value = (float) $value; } if (!is_numeric($value)) { + if (is_string($value) && ErrorValue::isError($value, true)) { + throw new CalcExp($value); + } + throw new CalcExp(ExcelError::VALUE()); } return (float) $value; } - public static function validateInt(mixed $value): int + public static function validateInt(mixed $value, bool $throwIfError = false): int { if ($value === null) { $value = 0; } elseif (is_bool($value)) { $value = (int) $value; + } elseif ($throwIfError && is_string($value) && !is_numeric($value)) { + if (!ErrorValue::isError($value, true)) { + $value = ExcelError::VALUE(); + } + + throw new CalcExp($value); } return (int) $value; diff --git a/src/PhpSpreadsheet/Calculation/TextData/Search.php b/src/PhpSpreadsheet/Calculation/TextData/Search.php index ad83f1a37..663d49fc2 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Search.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Search.php @@ -32,8 +32,8 @@ class Search } try { - $needle = Helpers::extractString($needle); - $haystack = Helpers::extractString($haystack); + $needle = Helpers::extractString($needle, true); + $haystack = Helpers::extractString($haystack, true); $offset = Helpers::extractInt($offset, 1, 0, true); } catch (CalcExp $e) { return $e->getMessage(); @@ -74,8 +74,8 @@ class Search } try { - $needle = Helpers::extractString($needle); - $haystack = Helpers::extractString($haystack); + $needle = Helpers::extractString($needle, true); + $haystack = Helpers::extractString($haystack, true); $offset = Helpers::extractInt($offset, 1, 0, true); } catch (CalcExp $e) { return $e->getMessage(); diff --git a/src/PhpSpreadsheet/Calculation/TextData/Text.php b/src/PhpSpreadsheet/Calculation/TextData/Text.php index 44e0cd402..81c0ca7a1 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Text.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Text.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Calculation\TextData; use PhpOffice\PhpSpreadsheet\Calculation\ArrayEnabled; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; +use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalcExp; use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Information\ErrorValue; @@ -20,13 +21,17 @@ class Text * @return array|int If an array of values is passed for the argument, then the returned result * will also be an array with matching dimensions */ - public static function length(mixed $value = ''): array|int + public static function length(mixed $value = ''): array|int|string { if (is_array($value)) { return self::evaluateSingleArgumentArray([self::class, __FUNCTION__], $value); } - $value = Helpers::extractString($value); + try { + $value = Helpers::extractString($value, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } return mb_strlen($value, 'UTF-8'); } @@ -44,14 +49,18 @@ class Text * @return array|bool If an array of values is passed for either of the arguments, then the returned result * will also be an array with matching dimensions */ - public static function exact(mixed $value1, mixed $value2): array|bool + public static function exact(mixed $value1, mixed $value2): array|bool|string { if (is_array($value1) || is_array($value2)) { return self::evaluateArrayArguments([self::class, __FUNCTION__], $value1, $value2); } - $value1 = Helpers::extractString($value1); - $value2 = Helpers::extractString($value2); + try { + $value1 = Helpers::extractString($value1, true); + $value2 = Helpers::extractString($value2, true); + } catch (CalcExp $e) { + return $e->getMessage(); + } return $value2 === $value1; } @@ -97,11 +106,14 @@ class Text * @param mixed $padding The value with which to pad the result. * The default is #N/A. * - * @return array the array built from the text, split by the row and column delimiters + * @return array|string the array built from the text, split by the row and column delimiters, or an error string */ - public static function split(mixed $text, $columnDelimiter = null, $rowDelimiter = null, bool $ignoreEmpty = false, bool $matchMode = true, mixed $padding = '#N/A'): array + public static function split(mixed $text, $columnDelimiter = null, $rowDelimiter = null, bool $ignoreEmpty = false, bool $matchMode = true, mixed $padding = '#N/A'): array|string { $text = Functions::flattenSingleValue($text); + if (ErrorValue::isError($text, true)) { + return $text; + } $flags = self::matchFlags($matchMode); diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php index 3dc6c2eab..45b58d2a3 100644 --- a/src/PhpSpreadsheet/Cell/Cell.php +++ b/src/PhpSpreadsheet/Cell/Cell.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Cell; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException; +use PhpOffice\PhpSpreadsheet\Calculation\Functions; use PhpOffice\PhpSpreadsheet\Calculation\Information\ExcelError; use PhpOffice\PhpSpreadsheet\Collection\Cells; use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; @@ -392,7 +393,7 @@ class Cell implements Stringable ); } - if ($result === '#Not Yet Implemented') { + if ($result === Functions::NOT_YET_IMPLEMENTED) { return $this->calculatedValue; // Fallback if calculation engine does not support the formula. } diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ErrorPropagationTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ErrorPropagationTest.php new file mode 100644 index 000000000..9ab19d94c --- /dev/null +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/TextData/ErrorPropagationTest.php @@ -0,0 +1,96 @@ +getSheet(); + $sheet->getCell('A1')->setValue('=ABS("X")'); + self::assertSame('#VALUE!', $sheet->getCell('A1')->getCalculatedValue()); + $sheet->getCell('A2')->setValue('=SQRT(-1)'); + self::assertSame('#NUM!', $sheet->getCell('A2')->getCalculatedValue()); + $sheet->getCell('A3')->setValue('=3/0'); + self::assertSame('#DIV/0!', $sheet->getCell('A3')->getCalculatedValue()); + $sheet->getCell('A4')->setValue('=XXXX()'); + self::assertSame('#NAME?', $sheet->getCell('A4')->getCalculatedValue()); + $sheet->getCell('A5')->setValue('=ABS("X")'); + self::assertSame('#VALUE!', $sheet->getCell('A5')->getCalculatedValue()); + + $sheet->getCell('B1')->setValue('=UPPER(A1)'); + self::assertSame('#VALUE!', $sheet->getCell('A1')->getCalculatedValue()); + $sheet->getCell('B2')->setValue('=LOWER(A2)'); + self::assertSame('#NUM!', $sheet->getCell('A2')->getCalculatedValue()); + $sheet->getCell('B3')->setValue('=PROPER(A3)'); + self::assertSame('#DIV/0!', $sheet->getCell('A3')->getCalculatedValue()); + + $sheet->getCell('C2')->setValue('=CHAR(A2)'); + self::assertSame('#NUM!', $sheet->getCell('C2')->getCalculatedValue()); + $sheet->getCell('C3')->setValue('=CODE(A3)'); + self::assertSame('#DIV/0!', $sheet->getCell('C3')->getCalculatedValue()); + + $sheet->getCell('D1')->setValue('=CONCATENATE(A1,A1)'); + self::assertSame('#VALUE!', $sheet->getCell('D1')->getCalculatedValue()); + $sheet->getCell('D2')->setValue('=TEXTJOIN(",",TRUE,A2,A3)'); + self::assertSame('#NUM!', $sheet->getCell('D2')->getCalculatedValue()); + $sheet->getCell('D3')->setValue('=REPT(A3,3)'); + self::assertSame('#DIV/0!', $sheet->getCell('D3')->getCalculatedValue()); + $sheet->getCell('D4')->setValue('=CONCAT(A4,A4)'); + self::assertSame('#NAME?', $sheet->getCell('D4')->getCalculatedValue()); + $sheet->getCell('D5')->setValue('="X"&A4'); + self::assertSame('#NAME?', $sheet->getCell('D5')->getCalculatedValue()); + $sheet->getCell('D6')->setValue('=A2&"X"'); + self::assertSame('#NUM!', $sheet->getCell('D6')->getCalculatedValue()); + + $sheet->getCell('E1')->setValue('=LEFT(A1)'); + self::assertSame('#VALUE!', $sheet->getCell('E1')->getCalculatedValue()); + $sheet->getCell('E2')->setValue('=RIGHT(A2)'); + self::assertSame('#NUM!', $sheet->getCell('E2')->getCalculatedValue()); + $sheet->getCell('E3')->setValue('=MID(A3,2,2)'); + self::assertSame('#DIV/0!', $sheet->getCell('E3')->getCalculatedValue()); + $sheet->getCell('E4')->setValue('=TEXTBEFORE(A4,"M")'); + self::assertSame('#NAME?', $sheet->getCell('E4')->getCalculatedValue()); + $sheet->getCell('E5')->setValue('=TEXTAFTER(A5,"U")'); + self::assertSame('#VALUE!', $sheet->getCell('E5')->getCalculatedValue()); + + $sheet->getCell('F1')->setValue('=VALUETOTEXT(A1)'); + self::assertSame('#VALUE!', $sheet->getCell('F1')->getCalculatedValue()); + $sheet->getCell('F2')->setValue('=DOLLAR(A2)'); + self::assertSame('#NUM!', $sheet->getCell('F2')->getCalculatedValue()); + $sheet->getCell('F3')->setValue('=FIXED(A3)'); + self::assertSame('#DIV/0!', $sheet->getCell('E3')->getCalculatedValue()); + $sheet->getCell('F4')->setValue('=TEXT(A4,"M")'); + self::assertSame('#NAME?', $sheet->getCell('F4')->getCalculatedValue()); + $sheet->getCell('F5')->setValue('=VALUE(A2)'); + self::assertSame('#NUM!', $sheet->getCell('F5')->getCalculatedValue()); + $sheet->getCell('F6')->setValue('=NUMBERVALUE(A3)'); + self::assertSame('#DIV/0!', $sheet->getCell('F6')->getCalculatedValue()); + + $sheet->getCell('G1')->setValue('=REPLACE("oldtext",2,2,A1)'); + self::assertSame('#VALUE!', $sheet->getCell('G1')->getCalculatedValue()); + $sheet->getCell('G2')->setValue('=SUBSTITUTE(A2,"U","V")'); + self::assertSame('#NUM!', $sheet->getCell('G2')->getCalculatedValue()); + + $sheet->getCell('H1')->setValue('=FIND(A1, "U")'); + self::assertSame('#VALUE!', $sheet->getCell('H1')->getCalculatedValue()); + $sheet->getCell('H2')->setValue('=SEARCH(A2,"U")'); + self::assertSame('#NUM!', $sheet->getCell('H2')->getCalculatedValue()); + + $sheet->getCell('I1')->setValue('=LEN(A1)'); + self::assertSame('#VALUE!', $sheet->getCell('I1')->getCalculatedValue()); + $sheet->getCell('I2')->setValue('=EXACT(A2,A2)'); + self::assertSame('#NUM!', $sheet->getCell('I2')->getCalculatedValue()); + $sheet->getCell('I3')->setValue('=T(A3)'); + self::assertSame('#DIV/0!', $sheet->getCell('I3')->getCalculatedValue()); + $sheet->getCell('I4')->setValue('=TEXTSPLIT(A4,"M")'); + self::assertSame('#NAME?', $sheet->getCell('I4')->getCalculatedValue()); + + $sheet->getCell('J1')->setValue('=TRIM(A1)'); + self::assertSame('#VALUE!', $sheet->getCell('J1')->getCalculatedValue()); + $sheet->getCell('J2')->setValue('=CLEAN(A2)'); + self::assertSame('#NUM!', $sheet->getCell('J2')->getCalculatedValue()); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2581Test.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2581Test.php new file mode 100644 index 000000000..6ba87bd53 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/Issue2581Test.php @@ -0,0 +1,25 @@ +load($filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame('=CONCATENATE("Prefix ",MID(CELL("filename"),FIND("]",CELL("filename"))+1,255), " Suffix")', $sheet->getCell('B1')->getValue()); + self::assertSame('Prefix SomeName Suffix', $sheet->getCell('B1')->getCalculatedValue()); + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Reader/XLSX/issue.2581.xlsx b/tests/data/Reader/XLSX/issue.2581.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..82af1c627c90cf6ae4e78b3953624b3b483d6686 GIT binary patch literal 9189 zcmeHNgo0U5f7Zjc7ap+OJ?B&AzQ29S^rK|oSEBt<0e z=sD-}9Q30S>XalB2>`$b0RV)6 zS)@nOPSo@L#6L=$`Yx**|g9c)g87Fuj1~n1&|{ zGDI`EsKPI=?uOU)z!C-Cq2h&nK#rK{{Wj%M7wu14eykH7&paphv@uv7LPh6sS%qGt z-Wh9kCwKI2Es78sZuv{x%(gLqYiusk`3|X4N;5oizZMPddP7t9rYq}WsX~(H?;~^y zeGXU2dudp2X$4_;8F0D8a0PJdAgit%q_Xnl-@&isB68yPGau_E>oI(o!=%X-^idGJ z4RD+6%Md#v81Xk4XTC>BXGduSEjD>0GA!n-(2PNGIWWBjT5YbH0pxNbh&((b#z9u*;TSrK=mJnS*yu;`wQZUBEs&l0E0-6oy=^VIoQ8nX>#8EYmonE z3^6Kx2w{QPUAuD4(#vO?_HyXic}J1_sKT%#0kb4c&V* z_j%RoAvJ|DhjK?Fvt%WYvG=64M`@N8-V0+o3oD{-vKR`8_UxSPV~>}K>Bf_1g;kM} zWQ~dEw~cuzRfw?mx1I9hIMv=kEJqSZ3(YpA^i`saCo|ic*#w=U`v`M7d{XEhZz}vBy0MXcW}(*P}SdweNa}i5koX z&`gw#m%0}kNdy`=dRF>&X*v7I)NO!eh#z3GIzQoHAdP&U-}!ZYDgPjQjG|k$&oDi= zOl{SwF>bLQjfg4J_%RFm*#TK6WZmRoey+8cEGTxa*>(LyF?hKrWJEQLOcyTb7pY4i zDVb@}GiiFVwlEmtqITX0dOtrL8KG;SgoP7d0Kdt8Jsje8`+7kX}%}t^GPc z2iDgOGPXA*8>ZDDeU*xrJ z>ks>4z(y7pX~sZr>n$73H-M!9*-hw_<6fhFz1Mb!Dr~rBZw*eBewGX4N-xmJAi764 z&$^^qnR}E@1AT$^rY98!E{W2zMXmW@{-ZEXM=^6L+uWM2;5hRi}p~r zaD6^ga@$5_Uvvm;%0#?3*vk1*B<^!f3puKq*@^ndM^}6p#FUs$fBTt5oTI_jrcH&m z(sE7_wW#9VG}a)r{q=SFtoY_fb|*LyP{bntss?r;qN#C&8n96S0GvP6;OydQYv%mj zkP|gViZeLzeL0S(rRKa1h`p>L_ti16Lgv&zv!|IJ~$A zGlZPEZ>eYlw{v`k2Qf46Z3}ADxkcAKU*IU{Xu``tO{|MsC?W897;rBMI!}c3K()JC zy(hdK1Rn~&C}73I8RNZix{KvKsdj4)RLrNPaG7VBQ#{W<%4Zr zxiaoCX7ROLJgX(S&9HEc#wGx!T zsZwJQuW~6MX8-s}o%_ehwx1nJfmj4+o+g^J<0({F=8|ouvn4w^eebGfdz$_H zN%0_fek&*0n(>UP3#h~eeKab+$$#r3X+>1~k|E}$Sef`{W5EQum(+Zs}&CFb!|8(s?Zn*yA*n?x?5$&8rvR{Fh(jMLk6Cv2r8Ul0; znrlGg(`nF53hXPv$t_$g&BsWyPNNG}{}=O2N&K zL>O$MFvu8t>ocITvi#F-n9fbC$`CdB3z81M&Z){6j3Su$>J<2?Cui@sNb=#5Vr#pD z$7nSi%C)pE(w!9J-V)i?TljN5b_%V0x?{kleQ&B+#|F^qxK4CrX)AmvKo;_2x z9792)5BB$-5VotKX9w58>$`VqtR5))#%w*#eEjZhY_)MLE!hD9qb?G%uTu1WY4A|& zmx0^isfo};hFAhWuJ)Y9Ru(*>x=LxqKn!vDb@k0PPpU24=Mkh|)?eq|5lm{pwxc@p zFD4Dk7$}f+%n~sZrNB7cqXt>oS?jB{IIoZ0AQ&$0yfm>mMc14n`HX}0)L*ZC4CSDh zqWpr}Bb-boH?2i4_wrR_%CZMN3pVKV9v*?}iFr-0!O86>VbexfbF%Iw6R z=fyE5vN7d7A95Mx3ye8E{R%e`-uzdhZR9gF1R^M?LH@mZ{o(FiEY0l9IDTAzuxnda z1vL`jbCWMxkqp(uo>CIy~`6`k=g;5as#QQKAx+gwY^Pg_Jewhvm&z7d6UvZpyWl-i7 z-YHH!CTQvH9(tibP8E;YZPM?dI6apkw>v#?wrlVZ1Ag5eUbI|CFq@#Ce(OAI4P#$=o}e1G2jZsI-WoFXw-t+5@% zVE%m6bt7=!Wnhm8&TOH#HTg)}&FK`>HqKo(x#j9zn~{lRG>@iK$}0Qp%jNyjgb`N% z?N8i0_O)uIO()0~Ul`IJJy$LmkvFT9!RZm7NZBKQZ|>_o04ImN6-eIuGU!%6%I>n;wDo`8J^C*@RJt{Qxu$G3xNWBa-X#mGlz$YHu6Sd#J(@qiQMyL zKLrd+gx-vfd&XoM=o`Mo9&%nS7fsy^dv*a2WKUrCWpW-Zqf_poyYPfMGBDkCY`xim zuVI0T8{meqk%>vUP%30@)`066?XE4c&kQuonsqm&3^vZU#8@MTECap@T2Qjf<2Xnk zvgmjSMg}>p8f$e@8u{pSHAR|q*S6>)Yw}>pcOTEf1dTuQP~4?}r82)gD{qi;w>{g_k6^dV4$_93sn{Bl_v8&+SF|RO+Mmah3e)(W( zg4NW0;mkV}NpTcQ;Wu&o2wznw_Q3T|Wc5{8_FN=L9<*@9RoM>MA)VgJ@B>P>u(+O4 zr9WU-XBKA5^(1sQl=-}8CD5s+9!fbZ$ZC8Krh%>&b^MwoXxC9d`bgvisr&A%Zwf*8 zYyokg2(N>T9!hv-8pe(S^SJUwwB@<`+f=o}E`HG5%};gpIFB@R<+n4L;jbL7;O$L8 z7W?7>I*;c(=HAb`yH0vPpBltY8J+N0 zA4?J=#Bt&_@5AW-#$!@bhLMppb()mOTzDbeZ>?9z6&&wgt0-e&f(!AbQr21;XqMowWK!ep5fqnH`a4e3EX0f;!R(3|F z$Pu*}8@>rLssqQ-V@8LPkZxwAa)sFw$gQ?nSa~qcnMT5t>9_XuQbjHS3~L7s$WH6a z3Jo*@+V{B0**G{nNuhv9KBl`@dcb}!HOvokPjK!`2+)vR^4)2RDYNM{mGaSaaf8Ur z47TRyB@PkY2y*ZVzHiHj%h~WajAflCpTM3;QFfTaAt}GqF8||F2BX0~a^J$XpgA~v z%;}W|Z=WfafM-UZ(H!PdfPG#S`O08<1o#`4dLBvF6_4hTAMzyXmdOqhwY?MSec{J5 zRKzUHsv+^PQ(vU$pdw0qHU~)?cF!QjI zGS_?tS^RDbKicKSU^gQV^U4HdY}oX8NS1X+7a_Y^xK&@xT z<#*bJ>h&H~Ci5EFn6?KHr|^Zsn`z}2M$tCzRa~hp@|-zJK?|k*0JS&a^&lnec2Le~ zyqzeF9p>hM5}iO<1AhtD9m_9GsMD*{ee6(>ekca9Wt-AH_ft+q< zo^Z3xD}q-uX%K}nPDyqNcKN(#@$*>9JzH3(Y*@6QTQCG17#J;m zg9u2!hAeS(!UzULb&`yzb^l9d@FQ+TX(+``BN}tsn@=iMQCJ3uks6tMBSK_lr}JJ_ z*{O)7no5-@E%mthA5XexNNCo2uhYheL01R*$weB)v>8NlMx`dWPGZoow;r13h*ARNW%MG;!sY?L4q?24rla%& zR|nTzn1z9=W@Esb(riQ|nOErRQ)=9_HM+M+iZhrtV+qWxud`2GX~}TFi5Ts+(uxqO zj`pniZwriSDc@eTC0v>eVbAYYbdDR{M+Q_!@8tPZ#*lM+@{eCIxy|w&ztjm8f+`&9 zW{T$s5{1f1G1W*GvECspEg{g-H@bQ6tB{O2<}|7@$!WF#4rAM>7P+o(6HUsAICq20 z=+*eAfVF<}YDd9LwPmVWU99vIzFYMixi&^`aa8CSRH8Z=p|cJm!cUa@4OR%_IcgW$ zb0)fvAvo@)FNmg@oqBcOv8A%!vrCCwJ7e<&deFUon+9KZtR`dKrKSSGA_;5^X|W${ z4Y=|7?{c1!leQi{fSYnta2YivuX=|<-IRR&;lK$9rA*eQj{= zE!@^dQ?t=BRz^)wZF_hx)gEd(rd#ST3R|AkkW3*g(ikEI_T}+T0>MU}?v~S`V21T2 z*RO1$_gQiqIA2mtJ&{L#r+M2LJt%$8TsZgU_7*ptFRH3@+}juYx&>8dXR-{xr`B*( z1(pwjbODHy=#Om3*%IPpW~$-hWMyyhqxiz^lf-Q2#FrRpS)gp+w#&h~VKx1nX&8t- z@Bt+r+=HD{FVHviX=tn7^e~&fs4DDmxeXu6M?WXloEbZY6UFS*rk^HyE@xyA6Q(OO z;Gruj^WLpqt4yoaUUU2RP3UD-Bp@ic8E;YM3GMgdexiX+8w z0;*hzu2L{zcgYiV=t0@9%T^Ob5Dl|Dc+km#E-zX_6A=M}j z@gx^<;v-H(Kg9%MYa(q4v9kYOzlaY0pi{`=A9T_qri*|1Vgcg_AT7)vV5-h7d30>P zaqDD~G4(nQ{6R1HYg9z(#?j(eBMLA`&RCb14{ z@I+nY#1hafG26DYrCEO9!)eK@2MtlkywW)y z&Mq69b0e%yVSKxnK_Me;Ke}!9K5HUidS_l?rc`S!DAr-{LU1kA$4^!B2xFpGkC<6O zohV?n7Ure>BIOHRW_FKfKI$uL98}>z>BEClmbK7>>GpRMppujYM5Ptur17HVYpC(| z2^Enu*kPT<-UiVhd(hMoy6;{GR2)6Jb$3sZbI1WJYCuAU>E!}&)aOGn5IrUv8FgAD zgMsw&*sJ2*h0ySw+XeBLmQn>8AGsdw8B%;iV?r|-;|23<7zdw|`Wc9exoesf%@{aJ zn6V44+)6FCd!6Vr^ddqlzM3L7*zM^}XFt;8mL!_bv2U7FycXQ}yadCUQF48wtEF>}w$}h1RBt*^O3oFe}uRZCnmpsFA#oUK3x!mua4fg;g3bme4tPmsC?^pM> zva?8>jG&$nBF^Lgoq7-lhku#(opXO283~fm8BXkwUEnb__AL2pxGGUViLsLQaz!mt zyqkq`zDahKX?RK8(WFY)ugX^80~SN8PYE+r5y6j1@X*sFc-38NSNQ zS)@(ejUWyLXaa5zZ#moPqR< z6pl7iv|?H|;pZGn0gz=%NB(R$n&l{FOz^HIhqzPpe(tBJ)sAm{!!-+drw~`~SIgr? zSrc!DHJWhT+XPE5dio*jLXC>`y}-I7k`JE2TYSgRg>9KW`&GMs|QzcLr? z6?qt9-3_l$|Kc(dG8-a^`2A}Ff9dY8@87&MpsMgsfPZdh{w4VRI}KqVf7{%=F8I&A zp}z};AR0gaeW&O;&h?#vpGZT9HzBU=6I>U*-p~FitcCeQ_-A+fy6E*@=uc5i?En1l z|JE734tTw__!CeY(Q5y5B7Xw@Y&2d+x!yVZiPDPu1Lb-j?K;5qI_4*U!8=uo}Zx9WIsUvkpx|r y{%6+lQx*W2LrDLUgIpK?XXN<1xGv@2#Q%&VstTwGI{*N15uaj&DKXG|zxy9LHDi7N literal 0 HcmV?d00001 From 4860e8e4b89866a6884ffd16a6fc9183f32ccac4 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 29 Jun 2024 22:23:50 -0700 Subject: [PATCH 04/30] Correct 2 Doc-block statements --- src/PhpSpreadsheet/Calculation/TextData/Text.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/TextData/Text.php b/src/PhpSpreadsheet/Calculation/TextData/Text.php index 81c0ca7a1..f988a6c19 100644 --- a/src/PhpSpreadsheet/Calculation/TextData/Text.php +++ b/src/PhpSpreadsheet/Calculation/TextData/Text.php @@ -18,7 +18,7 @@ class Text * @param mixed $value String Value * Or can be an array of values * - * @return array|int If an array of values is passed for the argument, then the returned result + * @return array|int|string If an array of values is passed for the argument, then the returned result * will also be an array with matching dimensions */ public static function length(mixed $value = ''): array|int|string @@ -46,7 +46,7 @@ class Text * @param mixed $value2 String Value * Or can be an array of values * - * @return array|bool If an array of values is passed for either of the arguments, then the returned result + * @return array|bool|string If an array of values is passed for either of the arguments, then the returned result * will also be an array with matching dimensions */ public static function exact(mixed $value1, mixed $value2): array|bool|string From 8557ccb72a482fefc08b2ee82948e5d5989887a6 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 1 Jul 2024 14:52:23 -0700 Subject: [PATCH 05/30] Ods Comments With Newlines Fix #4081. Ods Reader was not reading entire contents of comment. On further inspection, Ods Writer also was not handling comments completely correctly. Ods comments are recorded as `text:p` children of `office:annotation` elements. A newline is inserted between successive `text:p` elements. The `text:p` element itself can have as descendants (at least): - raw text - `text:span` elements - `text:line-break` elements, which also causes the insertion of a newline Ods Writer is changed to use a single `text:p` with multiple span/linebreak elements. Ods Reader is changed to process in their entirety either that form, or multiple `text:p` elements. Styling of the individual elements of the comment is permitted in Ods. That has not been supported till now by PhpSpreadsheet, and this PR will not address that situation - Ods Reader hast little style support, and this would hardly be the most urgent case where it is missing. --- src/PhpSpreadsheet/Reader/Ods.php | 27 +++++++++--- .../Writer/Ods/Cell/Comment.php | 17 +++++++- .../Reader/Ods/MultiLineCommentTest.php | 41 ++++++++++++++++++ tests/data/Reader/Ods/issue.4081.ods | Bin 0 -> 3146 bytes 4 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Ods/MultiLineCommentTest.php create mode 100644 tests/data/Reader/Ods/issue.4081.ods diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index ceb345dc3..346d97c54 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -436,14 +436,25 @@ class Ods extends BaseReader if ($annotation->length > 0 && $annotation->item(0) !== null) { $textNode = $annotation->item(0)->getElementsByTagNameNS($textNs, 'p'); + $textNodeLength = $textNode->length; + $newLineOwed = false; + for ($textNodeIndex = 0; $textNodeIndex < $textNodeLength; ++$textNodeIndex) { + $textNodeItem = $textNode->item($textNodeIndex); + if ($textNodeItem !== null) { + $text = $this->scanElementForText($textNodeItem); + if ($newLineOwed) { + $spreadsheet->getActiveSheet() + ->getComment($columnID . $rowID) + ->getText() + ->createText("\n"); + } + $newLineOwed = true; - if ($textNode->length > 0 && $textNode->item(0) !== null) { - $text = $this->scanElementForText($textNode->item(0)); - - $spreadsheet->getActiveSheet() - ->getComment($columnID . $rowID) - ->setText($this->parseRichText($text)); -// ->setAuthor( $author ) + $spreadsheet->getActiveSheet() + ->getComment($columnID . $rowID) + ->getText() + ->createText($this->parseRichText($text)); + } } } @@ -731,6 +742,8 @@ class Ods extends BaseReader /** @var DOMNode $child */ if ($child->nodeType == XML_TEXT_NODE) { $str .= $child->nodeValue; + } elseif ($child->nodeType == XML_ELEMENT_NODE && $child->nodeName == 'text:line-break') { + $str .= "\n"; } elseif ($child->nodeType == XML_ELEMENT_NODE && $child->nodeName == 'text:s') { // It's a space diff --git a/src/PhpSpreadsheet/Writer/Ods/Cell/Comment.php b/src/PhpSpreadsheet/Writer/Ods/Cell/Comment.php index b0829bf1d..f0b7d5704 100644 --- a/src/PhpSpreadsheet/Writer/Ods/Cell/Comment.php +++ b/src/PhpSpreadsheet/Writer/Ods/Cell/Comment.php @@ -24,7 +24,22 @@ class Comment $objWriter->writeAttribute('svg:x', $comment->getMarginLeft()); $objWriter->writeAttribute('svg:y', $comment->getMarginTop()); $objWriter->writeElement('dc:creator', $comment->getAuthor()); - $objWriter->writeElement('text:p', $comment->getText()->getPlainText()); + + $objWriter->startElement('text:p'); + $text = $comment->getText()->getPlainText(); + $textElements = explode("\n", $text); + $newLineOwed = false; + foreach ($textElements as $textSegment) { + if ($newLineOwed) { + $objWriter->writeElement('text:line-break'); + } + $newLineOwed = true; + if ($textSegment !== '') { + $objWriter->writeElement('text:span', $textSegment); + } + } + $objWriter->endElement(); // text:p + $objWriter->endElement(); } } diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/MultiLineCommentTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/MultiLineCommentTest.php new file mode 100644 index 000000000..f6fd5657a --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/MultiLineCommentTest.php @@ -0,0 +1,41 @@ +load($filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame("First line.\n\nSecond line.", $sheet->getComment('A1')->getText()->getPlainText()); + $spreadsheet->disconnectWorksheets(); + } + + public function testOneParagraphMultipleSpans(): void + { + $spreadsheetOld = new Spreadsheet(); + $sheetOld = $spreadsheetOld->getActiveSheet(); + $sheetOld->getCell('A1')->setValue('Hello'); + $text = $sheetOld->getComment('A1')->getText(); + $text->createText('First'); + $text->createText(' line.'); + $text->createText("\n"); + $text->createText("\n"); + $text->createText("Second line.\nThird line."); + $spreadsheet = $this->writeAndReload($spreadsheetOld, 'Ods'); + $spreadsheetOld->disconnectWorksheets(); + + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame("First line.\n\nSecond line.\nThird line.", $sheet->getComment('A1')->getText()->getPlainText()); + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Reader/Ods/issue.4081.ods b/tests/data/Reader/Ods/issue.4081.ods new file mode 100644 index 0000000000000000000000000000000000000000..6d690bd4e14c5429fea6048e336a8fdea8103a90 GIT binary patch literal 3146 zcmZ`+2{@GP8Xn8=+bLUQUrI7)kg_FFmSM&^c0)t9!HnN7vZU;55y?&xjWz4f5F*5A zY-Qi~?6S-mo$H*==|AWFuJ`)h?|r}f`L6H1p8L7)7phG`NelRA699y}+=v8`9?tq7 zr0(kIiuCk#MhHMdh z+T+%*<|&mjX%)$xSd%yPV`{u26K50TUX5{&X=2n^UZoKiVRDzYs=G4j*RCh)NAypg z*QVWKF>J8fqaN?(wHD)K=WqNYBGpH|PGB8Q?mv;=MLaOCm2bhb$2I|wQ?DHa9dPln zMGZ(EFc(z_((3ADlf7cxm>0I?`t9$XF+AMH*CQKcP{OkeQ;MbVhy&YN9wu4zU;rVy zml_CGyFk_Yp-h#V!C9(L+JT2&LXnG^>GtK^^3kdJS_3qz#BhZ6<{KD%kxX1_=PR}U z27Vy^!sJHv`gzlvjY@x2mT`37_;iEb#fo}_uKVNni&XXXfig!FHG#F(^ zQK(e+K$`xSGN0dy(`7(8N5^^!MXazWm5via|HVn8xp4z>D+pGAB8*>k{b54;V#$ad zPz5o(JOc0!F_-dV?&M&Uz9M3%tipEDQBN>^EKd8yn_;!*eMZ4oFcqaRT8mRA$OI19 zA@XXnyp6762-L%7G1TfWa786%jJYy}{n<(cdU@t<+#fyP~wpIL4be(Aym}fdTju<=r@|X?)AQCA6|IMCkpOMSs zlmNg4BLF~p0bu8b@;sv(i9X3DW2hN9=0M5MLCl6(Zk4O;wQy&DSQdOIdpK}-hvGqE z`4ecKmVUywvW-nqZETX$?qplWltphwT0+e$y_im`G&zfpmQ7|?x1%+4E_PCXX8jcs0R82ULuzcnacOQ$|@3SHRd=~by+Oc zrnI;KE@7-bkYb6k@k52Nng@i(`2x*)?0`j0Ntwm-qfoZyE%QwE$Y`IY-umg%d=tVLmtaVwzz892$PSXk>a|E#1o zQP|vy#VlCB-1!U73iBN3xfCujF+zG)Hkgm!!_%FdGL)_r8lbo8r}qx8PDZ|E%s}M$ zJX)!SZgsroYalngJZP!0fQd5vrISzeP0h5uTZd=}nO`oVPePcD zSqHlt&F7q2X@FOL&;NtJu4ohHQ(OUhZ)t@k6?iSZ+J)7TSC+kot{E#ypsfVK{oc;7t3IxhF?X0 z)qz|cO%_!(5MT7+F8cxmUphVuUVdmDmWamF(uZCT<2<)2Tk{SM%e~C1`rtkj<};*X zv&h37lQ>NaCYOwW<^<4}n9t>7%^HK%aw!YlxxLin(Z^}gVrA={k760;JD+xLUB?-| z=5M^M;2OTN#5aUf?caX4tV4I4S785Xy@pIR1h_=G${><}Adoqf3zI8ziZV1G`Z>1p z%g{UP3z5UsDiDL7?Sfycr=$7Hnv6@`Y+mIJgqg=8lsJ-YAU#$0Dumi{SQJ#V+8dKX z^xH%@-_M$QXeKmMi&F92y7b_%FIBy0!uvUxS*lY>Pq6!<Q|$o)LRV|4~!PqS>1<(6Xa z=NsIYgYyju&lRFd{xrYOUt1`#S~)k(a;{d98$^+xJ@hPUZVOSttD<=`x(4gss5^aU zer|f{VWS^s=3VHpo0u5QCjLqzkpNCR`EZckf;Nw)G_N7+V(0e=fw`N%gQP!!8crKcz; zf{JhB_<(ziyc`0t9Og)4pgwOVIi$BAw_mfk%c$cZE`Sw)%n+#%zM4xn9#DuJ-A>U;wj|1eewCSy(Z z$-8VyPUw!ZU}SY1w<#go#c$bqFvI~nw4icJO=RirDYz=DIrN_Y(bu<}lB6H~yCS^M z#OQ000RT@(Tk+GQAw3aiauBWGrxz;4+LV5v5%@Jf9Y{_#I!!JMHFu zr(HV9_G9z(ju}C{Fc9N5oUW`YSrsYAGud#m6DXzX$yH1h0)4rkw5t-xIr>+hx zURa~vucN;Zo7Hm7&E@23t!(ebV~}Ck$19q-6MYe2>hO)U>9@8`ZYO_URp}+HxEM{- zVPO_dOCko+HOyDNcor9FX8l`t?$m}Qhu~K>KiC*Dy;Ih_ni_FayFyTR$U|#SxX@Qs z^ShkyF_4kbgi~E>yTRcT{+u3LEV8HUkQO&JsPgaqCL@>l=~YP+fWN!qv)NzmUmh8% zeTMq~zV{cxkTm?|f`7;TuK2%ji6j^OANv10@OO^<0;ZAj Date: Wed, 3 Jul 2024 19:56:38 -0700 Subject: [PATCH 06/30] Ods Xml Reader and Whitespace Text Nodes Fix #804, opened in Dec. 2018, and closed as stale in Feb. 2019, and which I have re-opened to be closed properly by this PR. Better late than never, I suppose. A third party generated an ODS spreadsheet which PhpSpreadsheet could not read. By way of explanation, the xml in the file contained lots of whitespace between tags, which is wonderful for those humans among us who have to analyze it; but PhpSpreadsheet was not prepared for it. It is now. --- src/PhpSpreadsheet/Reader/Ods.php | 6 ++- .../Reader/Ods/Issue804Test.php | 38 ++++++++++++++++++ tests/data/Reader/Ods/issue.804.ods | Bin 0 -> 4979 bytes 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Ods/Issue804Test.php create mode 100644 tests/data/Reader/Ods/issue.804.ods diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index ceb345dc3..746210ab3 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -6,6 +6,7 @@ use DOMAttr; use DOMDocument; use DOMElement; use DOMNode; +use DOMText; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\Helper\Dimension as HelperDimension; @@ -403,8 +404,11 @@ class Ods extends BaseReader } $columnID = 'A'; - /** @var DOMElement $cellData */ + /** @var DOMElement|DOMText $cellData */ foreach ($childNode->childNodes as $cellData) { + if ($cellData instanceof DOMText) { + continue; // should just be whitespace + } if ($this->getReadFilter() !== null) { if (!$this->getReadFilter()->readCell($columnID, $rowID, $worksheetName)) { if ($cellData->hasAttributeNS($tableNs, 'number-columns-repeated')) { diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/Issue804Test.php b/tests/PhpSpreadsheetTests/Reader/Ods/Issue804Test.php new file mode 100644 index 000000000..afb33dabe --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/Issue804Test.php @@ -0,0 +1,38 @@ + + + Name', $data); + } + } + + public function testIssue2810(): void + { + // Whitespace between Xml nodes + $filename = 'tests/data/Reader/Ods/issue.804.ods'; + $reader = new Ods(); + $spreadsheet = $reader->load($filename); + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame('Straße', $sheet->getCell('G1')->getValue()); + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Reader/Ods/issue.804.ods b/tests/data/Reader/Ods/issue.804.ods new file mode 100644 index 0000000000000000000000000000000000000000..b42de4cf635d248d117abde3ef3f084c93e3b9d6 GIT binary patch literal 4979 zcmb7|2{@Ep8^_1KWs8t#CTl3Wvb@QJu|&pJgc#E#8Z*X>U6!$A4_UKiEZH-*M9TVx zB0@+ZLfMHB@r}OEt9PS&tLrQ+Tvl>^$r$zfaihip9i=^W8EjtDIF|IdiU z!EjFhL$}{n;?QX1ui0pS9tX%q54`{x05DAj030FB^Pl5j4@24BKwxp=uJ*|C7%-{< z#G<~U5YRMO*_L+!Qxeq8Rz{OBLiF<|lTu|LdM0nc_PC9ZTOR>#@BpU6N|nMZ6s-kD zEyPmC?&WmY4jnyPHu7Vp+>5n(#!dz=bMA2dBlXzV3RN+Mnw=ud*EJQmqS$YZQB<{0 z$r9kpHI{AJGkH~b4>++s9FtchJ$DYQdFmzA_gRSC-kb*6}0zTqQpJE!{{^MJpO$mZGoC(_5BT z!Y=4zj7)eyM$ekiU-!{lmvQ<{bj%w@?)lC#UQX~T;P1YbY{cb>mo9^zh3WP{_uLsv z={h4=W~&)2A@42(HI3Kv&O6YsvZ-DwFApB~L->`+)qd=(%TmN)dom z2suS6fjH^?vlO42`@>yRgwu`j?+(G5Mp;u zRbf`qu7f0=D5HJBnagFKP??;#^;JkhEc;vg@v6sx&_#g|g|3_&={`*_g|vEpiA7LG z)fsJ7b-SB#RULK{<3R{4@9EXjPpWrSvgA05k>ykmeH6oQF;>``Sb52OU>p>9uGs6# zGAa)lRVF-R)w-W;{iH}=-6Uz;GDZ)rueL;1L%*3wRS`PsH!ou|C;Yto6PcNA0eRyo zjg69fGP>~_`RK29`2YHJx zmw-Npqoo~N0Jf6@FPk7&%Nqi<>sZwt4i`OPx6m2nfz%nlr@J%O zN|Bs*`dq&`(MF=oHEm9zHn*zBwe^U{+aB3i7tt%8(od(2v|_l55m)q+Q{IKNU4{??&7trSMq{ULjGesL@g#Yfid-V4n~cbkV*_kqi_h+&#(F!^EStUGc3<0MC2#4&Ov2G z6o_&}nM7O9kbz5HiZuujON=BL856+NAi^!`naU6W{YF^CYZcgXnWsU3%_1hUuW8n+ zdU)30>UnRXRxkQuCAu0&p)cgwvbCBi!InV0WN~+t6@=fqwkUW zxtq)EXOGBKyGvcgD8f~&q%uJ2)bWYn^mfbf+(INAG`N8t}QDfbpB5Wil42RPEYq;R&^bUEa?3OlA?nDf>tH9x+F z!+C%jvXCph_oIwY_dPO8KVzbmjX$P3H2t=Fz>~w;apskwVTxXf_Lm&k-I^7!W!UtY zF^H4U`B70+WuvAmT~eC@Iq-*q!Jx6nO{WkG?4n%6a(_~UURprc=YkFY+;W2fQ^S=3 z#z>RHoE-k6Ro}ZlzfYtB;JLE6ylApBMPosmyj;;%;+gpkQntJi&51{=3@D*qP;Gh$ zjeSTY&xtR!{x?0}mempIIv?1t`j>)%$d`I^@|XF`Oh!( zTgMYXw^|-B>X*w3{U`yUM<@p(OITU5CpqE9!WbbFz8864UWttZ>_L3Kv+|cIWed&u z9#}pMb8f(EnpWBPOu2}S*FS#xoIqL7{46=cb)W;VimB^a3zNg?lnlI>wak!*kUGrb98#YtaT3SF*p)imVehUq!$8lgL7}!pjDqqlGxSz3u1HE3 zN{?sq13cpFV=u$dk)Jn>)Nj}EO>+lpkvhtv*`&@ScW~DrsMFXesORbEwRJo9<)ul} z!d`J;S~d49_tD3vms4<9p{&h_B`;pj*J3`c-YfY4yG@|y3yc24-)KW;^_;tbvh_v8 zge6jkAN~#~ODH1B&#-b>s0Usz`IgW1FrZedXXvHE%FD&KkNouJ{|+k#(&F6JGOk&% zI0w~ck~-W}U8IgvP_6BGmP#@DX}xM2IvA+;6F6OR9Ws6iVJzOYIXZSl3NKG3%E<kr@Q7(Hm!b*_w1&SB}u%u5_}(5tm1kEVMd_ z7}Gi{aWjD<$HYPzN2U&&`wr)qR+=v?JLxM9(U%0>@EmnCdn0RX6!po_lJZw z%%pr^I-V^Sj2bAimS}TImS2=AfX8>e7JngBie9{QDP1FzdrIl+r%t~j=~6b}tk$}Q zgrX>AvOLBmY5~4l{9cU8bdABB8*Upo9QMI2@cUYP=%NKpvvi5QRpqc|=gs0Unc326 zx|sgzvvZcW3lm-(QnLW|YhqYKJj7Z?*_zbHzO+YqOuYkkM~#_J7JclpmUL<#v2VB3 z`EY~B*W?syUWl7g16ujqx_5rhu^6cz%8i3BEA29kqo{&%I?#?28FUJ&$a8inC0#vn zMNyl#Mj!dYPdM|uDUW%}m3s>luCLcyZ#SAyq+SjYvIu?of*Yuz^T|BoI5W;Gd5Jb{ zU9uSNTr~D6!=YX8)@}2)a!DnF3O#z8DZ8vf-@}(T&(Ww{ZjK+&@m3IjvIa9vmDO>6 zdyLNVx{m>SQhcGMX!jbTFxR0j7MUbB;_)=WU+;8@ z$>(8=GKTET@$%FouE+>Go}Sf+p1!2AV+>6$l+~7t@hdaEWMhbR2~JDpH6ZjN896(} z!BkE9QT-WP;O)ylQ1vI-Hf8U`7Fd|n{BPzyIO#TZ@5C0^nAF^>{*A#8HrhtVo!A0L zkeYjqcA5NOmu)KFi7jv=skztXA4Wge<7DSQ~OSAfh9;!wtJ`YE5je` zvrX|ku?4o-&*wL$KiKK_8Kv*z^ef{Z?DPBn^zY-d%ltbo`&0kUx+;8`$$vSnzy1fX zdvy0F++B+sB<9`W-My;4Ft)2&pd_jOgJyeGdx2|LrAmU7U-4_Naxcv6Do;@ViZ*+7 id!LYZb(AEAgzr8tLoZU1i~s-@(u Date: Thu, 4 Jul 2024 20:06:17 -0700 Subject: [PATCH 07/30] Documentation Updates Mostly in response to issue #3961, which noted some discrepancies, both positive and negative, between documentation and reality concerning ODS support. --- docs/index.md | 3 ++ docs/references/features-cross-reference.md | 35 +++++++++++---------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/docs/index.md b/docs/index.md index 9505181b7..d577d8982 100644 --- a/docs/index.md +++ b/docs/index.md @@ -20,6 +20,9 @@ allow you to read and write various spreadsheet file formats such as Excel and L |CSV | ✓ | ✓ | |PDF (using either the TCPDF, Dompdf or mPDF libraries, which need to be installed separately)| | ✓ | +Note - reading or writing certain aspects of a spreadsheet may not be supported in all formats. For more details, please consult +[Features Cross-reference](./references/features-cross-reference.md). + # Getting started ## Software requirements diff --git a/docs/references/features-cross-reference.md b/docs/references/features-cross-reference.md index 37668ba82..f116be511 100644 --- a/docs/references/features-cross-reference.md +++ b/docs/references/features-cross-reference.md @@ -28,7 +28,7 @@ ✔ ✔ ● - ● + ● 6 ✔ N/A N/A @@ -389,7 +389,7 @@ Conditional Formatting - ● + ✔ ✔ ✖ ✖ @@ -414,7 +414,7 @@ ✔ ✔ ✔ - ✖ + ✔ ✔ N/A N/A @@ -502,7 +502,7 @@ ✔ ✔ ✔ - ✔ + ● ✔ N/A ✔ @@ -797,13 +797,13 @@ Alignment ✖ 3 - ✖ + ✔ ✖ ✖ ✖ N/A ✖ - ✖ + ✔ Background Image @@ -929,7 +929,7 @@ Macros ✖ - ✔ + ● 5 ✖ ✖ ✖ @@ -940,7 +940,7 @@ Form Controls ✖ - ✖ + ● 4 ✖ ✖ ✖ @@ -1001,6 +1001,9 @@ 1. Only text contents 2. Only BIFF8 files support Rich Text. Prior to that, comments could only be plain text 3. Only BIFF8 files support alignment and rotation. Prior to that, comments could only be unformatted text +4. Xlsx forms and controls can be read and written but not otherwise manipulated +5. Xlsx macros can be read and written; their values can be retrieved and changed, but only in a binary form which is unlikely to be useful +6. There is very limited support for reading styles from an Ods spreadsheet. Writing styles has better support, although Number Format is incomplete. ## Writers @@ -1184,7 +1187,7 @@ Row Height/Column Width ✔ ✔ - ✖ + ✔ N/A ✔ ✔ @@ -1256,7 +1259,7 @@ Number Format Mask ✔ ✔ - ✔ + ● N/A ✔ ✔ @@ -1472,10 +1475,10 @@ Merged Cells ✔ ✔ - ✖ + ✔ N/A ✔ - ✖ + ✔ Cell Comments @@ -1606,7 +1609,7 @@ Macros ✖ - ✔ + ● 5 ✖ N/A ✖ @@ -1615,7 +1618,7 @@ Form Controls ✖ - ✖ + ● 4 ✖ N/A ✖ @@ -1803,8 +1806,8 @@ Macros - $spreadsheet->getMacrosCode(); - $spreadsheet->setMacrosCode(); + $spreadsheet->getMacrosCode();5 + $spreadsheet->setMacrosCode();5 Security From 2897c4de33d7107b2038cd572f27ac96a2a29b9e Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 4 Jul 2024 20:18:25 -0700 Subject: [PATCH 08/30] Missed One Doc Change --- docs/references/features-cross-reference.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/references/features-cross-reference.md b/docs/references/features-cross-reference.md index f116be511..23dfbb333 100644 --- a/docs/references/features-cross-reference.md +++ b/docs/references/features-cross-reference.md @@ -1501,10 +1501,10 @@ Alignment ✖ - ✖ + ✔ ✖ N/A - ✖ + ✔ N/A From 070ceef5d0bb0557dd91fd795eb34457117d189b Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 5 Jul 2024 20:57:43 -0700 Subject: [PATCH 09/30] Changes to INDEX Function Fix #64 (really!), closed as stale in December 2017, another in our "better late than never" series. Excel's INDEX function doesn't really behave quite as described. If a single row is used as an argument, either in literal form `{item1, item2, item3}` or expressed as a range `A1:A6`, INDEX is happy to evaluate the array as if each entry were a row rather than a single item. PhpSpreadsheet is changed to do likewise. INDEX also returned `#REF!` when it would normally return an array (which would often be reduced to its leftmost topmost entry later). This code is deleted, invalidating one existing test, and INDEX will now operate like other functions which can return arrays. --- .../Calculation/LookupRef/Matrix.php | 15 ++++++--- .../LookupRef/IndexOnSpreadsheetTest.php | 33 +++++++++++++++++++ .../LookupRef/INDEXonSpreadsheet.php | 2 +- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/PhpSpreadsheet/Calculation/LookupRef/Matrix.php b/src/PhpSpreadsheet/Calculation/LookupRef/Matrix.php index d578854de..228b46448 100644 --- a/src/PhpSpreadsheet/Calculation/LookupRef/Matrix.php +++ b/src/PhpSpreadsheet/Calculation/LookupRef/Matrix.php @@ -81,7 +81,6 @@ class Matrix } $rowNum = $rowNum ?? 0; - $originalColumnNum = $columnNum; $columnNum = $columnNum ?? 0; try { @@ -91,6 +90,17 @@ class Matrix return $e->getMessage(); } + if (is_array($matrix) && count($matrix) === 1 && $rowNum > 1) { + $matrixKey = array_keys($matrix)[0]; + if (is_array($matrix[$matrixKey])) { + $tempMatrix = []; + foreach ($matrix[$matrixKey] as $key => $value) { + $tempMatrix[$key] = [$value]; + } + $matrix = $tempMatrix; + } + } + if (!is_array($matrix) || ($rowNum > count($matrix))) { return ExcelError::REF(); } @@ -101,9 +111,6 @@ class Matrix if ($columnNum > count($columnKeys)) { return ExcelError::REF(); } - if ($originalColumnNum === null && 1 < count($columnKeys)) { - return ExcelError::REF(); - } if ($columnNum === 0) { return self::extractRowValue($matrix, $rowKeys, $rowNum); diff --git a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndexOnSpreadsheetTest.php b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndexOnSpreadsheetTest.php index e893fc7f3..3f508405a 100644 --- a/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndexOnSpreadsheetTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/IndexOnSpreadsheetTest.php @@ -34,4 +34,37 @@ class IndexOnSpreadsheetTest extends AllSetupTeardown { return require 'tests/data/Calculation/LookupRef/INDEXonSpreadsheet.php'; } + + /** + * @dataProvider providerIndexLiteralArrays + */ + public function testLiteralArrays(mixed $expectedResult, string $indexArgs): void + { + $sheet = $this->getSheet(); + $sheet->getCell('A10')->setValue(10); + $sheet->getCell('B10')->setValue(11); + $sheet->getCell('C10')->setValue(12); + $sheet->getCell('D10')->setValue(13); + $sheet->getCell('X10')->setValue(10); + $sheet->getCell('X11')->setValue(11); + $sheet->getCell('X12')->setValue(12); + $sheet->getCell('X13')->setValue(13); + $sheet->getCell('A1')->setValue("=INDEX($indexArgs)"); + $result = $sheet->getCell('A1')->getCalculatedValue(); + self::assertEquals($expectedResult, $result); + } + + public static function providerIndexLiteralArrays(): array + { + return [ + 'issue 64' => ['Fourth', '{"First","Second","Third","Fourth","Fifth","Sixth","Seventh"}, 4'], + 'issue 64 selecting first "row"' => ['First', '{"First","Second","Third","Fourth","Fifth","Sixth","Seventh"}, 1'], + 'array result condensed to single value' => [40, '{10,11;20,21;30,31;40,41;50,51;60,61},4'], + 'both row and column' => [41, '{10,11;20,21;30,31;40,41;50,51;60,61},4,2'], + '1*1 array' => ['first', '{"first"},1'], + 'array expressed in rows' => [20, '{10;20;30;40},2'], + 'spreadsheet single row' => [11, 'A10:D10,2'], + 'spreadsheet single column' => [13, 'X10:X13,4'], + ]; + } } diff --git a/tests/data/Calculation/LookupRef/INDEXonSpreadsheet.php b/tests/data/Calculation/LookupRef/INDEXonSpreadsheet.php index 76f6ddd46..b2202704d 100644 --- a/tests/data/Calculation/LookupRef/INDEXonSpreadsheet.php +++ b/tests/data/Calculation/LookupRef/INDEXonSpreadsheet.php @@ -82,7 +82,7 @@ return [ 2, ], 'Column number omitted from 2-column matrix' => [ - '#REF!', // Expected + 'abc', // Expected [ ['abc', 'def'], ['xyz', 'tuv'], From b8715a5d8a175a52d24d67c1c33fc661afb10cd4 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 5 Jul 2024 22:20:57 -0700 Subject: [PATCH 10/30] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 229a32022..bffebad01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Added - Xlsx Reader Optionally Ignore Rows With No Cells. [Issue #3982](https://github.com/PHPOffice/PhpSpreadsheet/issues/3982) [PR #4035](https://github.com/PHPOffice/PhpSpreadsheet/pull/4035) +- Means to change style without affecting current cell/sheet. [PR #4073](https://github.com/PHPOffice/PhpSpreadsheet/pull/4073) ### Changed @@ -36,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Csv Reader allow use of html mimetype. [Issue #4036](https://github.com/PHPOffice/PhpSpreadsheet/issues/4036) [PR #4049](https://github.com/PHPOffice/PhpSpreadsheet/pull/4040) - More RTL in Xlsx/Html Comments [Issue #4004](https://github.com/PHPOffice/PhpSpreadsheet/issues/4004) [PR #4065](https://github.com/PHPOffice/PhpSpreadsheet/pull/4065) - Empty String in sharedStrings. [Issue #4063](https://github.com/PHPOffice/PhpSpreadsheet/issues/4063) [PR #4064](https://github.com/PHPOffice/PhpSpreadsheet/pull/4064) +- Treat invalid formulas as strings. [Issue #1310](https://github.com/PHPOffice/PhpSpreadsheet/issues/1310) [PR #4073](https://github.com/PHPOffice/PhpSpreadsheet/pull/4073) ## 2024-05-11 - 2.1.0 From 61b8aff3599936df6bd58efddc59e702714c4473 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 5 Jul 2024 22:46:55 -0700 Subject: [PATCH 11/30] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ecca9678..d0f147133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Xls Conditional Format Improvements. [PR #4030](https://github.com/PHPOffice/PhpSpreadsheet/pull/4030) [PR #4033](https://github.com/PHPOffice/PhpSpreadsheet/pull/4033) - Conditional Range Unions and Intersections [Issue #4039](https://github.com/PHPOffice/PhpSpreadsheet/issues/4039) [PR #4042](https://github.com/PHPOffice/PhpSpreadsheet/pull/4042) - Csv Reader allow use of html mimetype. [Issue #4036](https://github.com/PHPOffice/PhpSpreadsheet/issues/4036) [PR #4049](https://github.com/PHPOffice/PhpSpreadsheet/pull/4040) +- Propagate errors in Text functions. [Issue #2581](https://github.com/PHPOffice/PhpSpreadsheet/issues/2581) [PR #4080](https://github.com/PHPOffice/PhpSpreadsheet/pull/4080) ## 2024-05-11 - 2.1.0 From 10123c441b3d963ba18ffc5d0f42b20b912ba61e Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 6 Jul 2024 19:58:19 -0700 Subject: [PATCH 12/30] Html Writer Minor Fixes While researching issue #1551, I came across some minor problems. When a spreadsheet does not have a title, which is often the case for spreadsheets created with Excel (note that this is not the case for spreadsheets created with PhpSpreadsheet), if you try to save it as Html, it throws an exception. It will now use the sheet title of the active sheet as a title in this case. When writing an Html spreadsheet using `useInlineCss(true)`, gridlines are not handled properly. This is addressed by adding `class=gridlines gridlinesp` to the cell's `td` tag, and by suppressing any border attributes which would be styled as `none #000000`. It would be unusual to turn off gridlines for specific cells, but that can still be accomplished by using `Border::BORDER_NONE` in conjunction with any color other than `#000000` - see new test `testHideSomeGridlines`. --- src/PhpSpreadsheet/Writer/Html.php | 16 ++++- .../Writer/Html/Issue3678Test.php | 5 +- .../Writer/Html/NoTitleTest.php | 61 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 457af6612..37a9c8994 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -355,7 +355,11 @@ class Html extends BaseWriter $html .= ' ' . PHP_EOL; $html .= ' ' . PHP_EOL; $html .= ' ' . PHP_EOL; - $html .= ' ' . htmlspecialchars($properties->getTitle(), Settings::htmlEntityFlags()) . '' . PHP_EOL; + $title = $properties->getTitle(); + if ($title === '') { + $title = $this->spreadsheet->getActiveSheet()->getTitle(); + } + $html .= ' ' . htmlspecialchars($title, Settings::htmlEntityFlags()) . '' . PHP_EOL; $html .= self::generateMeta($properties->getCreator(), 'author'); $html .= self::generateMeta($properties->getTitle(), 'title'); $html .= self::generateMeta($properties->getDescription(), 'description'); @@ -1462,11 +1466,21 @@ class Html extends BaseWriter $xcssClass['height'] = $height; } //** end of redundant code ** + if ($this->useInlineCss) { + foreach (['border-top', 'border-bottom', 'border-right', 'border-left'] as $borderType) { + if (($xcssClass[$borderType] ?? '') === 'none #000000') { + unset($xcssClass[$borderType]); + } + } + } if ($htmlx) { $xcssClass['position'] = 'relative'; } $html .= ' style="' . $this->assembleCSS($xcssClass) . '"'; + if ($this->useInlineCss) { + $html .= ' class="gridlines gridlinesp"'; + } } $html = $this->generateRowSpans($html, $rowSpan, $colSpan); diff --git a/tests/PhpSpreadsheetTests/Writer/Html/Issue3678Test.php b/tests/PhpSpreadsheetTests/Writer/Html/Issue3678Test.php index cabcaca8f..e4f292c2d 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/Issue3678Test.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/Issue3678Test.php @@ -24,7 +24,8 @@ class Issue3678Test extends TestCase ]; $sheet->getStyle('A1')->applyFromArray($styleArray); $style1 = "vertical-align:bottom; border-bottom:none #000000; border-top:none #000000; border-left:none #000000; border-right:none #000000; color:#000000; font-family:'Calibri'; font-size:11pt; background-color:#FFFF00"; - $style2 = $style1 . '; text-align:right; width:42pt'; + $style2 = "vertical-align:bottom; color:#000000; font-family:'Calibri'; font-size:11pt; background-color:#FFFF00"; + $style2 .= '; text-align:right; width:42pt'; $writer = new Html($spreadsheet); $html = $writer->generateHtmlAll(); self::assertStringContainsString('td.style1, th.style1 { ' . $style1 . ' }', $html); @@ -33,7 +34,7 @@ class Issue3678Test extends TestCase self::assertStringContainsString('.n { text-align:right }', $html); $writer->setUseInlineCss(true); $html = $writer->generateHtmlAll(); - self::assertStringContainsString('1', $html); + self::assertStringContainsString('1', $html); $spreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php b/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php new file mode 100644 index 000000000..264e88b4e --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php @@ -0,0 +1,61 @@ +load($file); + self::assertSame('', $spreadsheet->getProperties()->getTitle()); + + $writer = new Html($spreadsheet); + $writer->setUseInlineCss(true); + $html = $writer->generateHTMLAll(); + self::assertStringContainsString('Sheet1', $html); + self::assertStringContainsString('C1', $html); + $writer->setUseInlineCss(false); + $html = $writer->generateHTMLAll(); + self::assertStringContainsString('C1', $html); + $spreadsheet->disconnectWorksheets(); + } + + public function testHideSomeGridlines(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->fromArray( + [ + [1, 2, 3, 4, 5, 6], + [7, 8, 9, 10, 11, 12], + [17, 18, 19, 20, 21, 22], + [27, 28, 29, 30, 31, 32], + [37, 38, 39, 40, 41, 42], + ]); + $sheet->getStyle('B2:D4')->getBorders()->applyFromArray( + [ + 'allBorders' => [ + 'borderStyle' => Border::BORDER_NONE, + 'color' => ['rgb' => '808080'], + ], + ], + ); + + $writer = new Html($spreadsheet); + $writer->setUseInlineCss(true); + $html = $writer->generateHTMLAll(); + self::assertStringContainsString('7', $html); + self::assertStringContainsString('19', $html); + $spreadsheet->disconnectWorksheets(); + } +} From 3fee2c02e319140d199c8d4e5d3f7ab834602b5f Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 6 Jul 2024 20:15:19 -0700 Subject: [PATCH 13/30] Wrong Case in File Name --- tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php b/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php index 264e88b4e..cedfe8dd2 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php @@ -14,7 +14,7 @@ class NoTitleTest extends TestCase { public function testNoTitle(): void { - $file = 'tests/data/Reader/Xlsx/blankcell.xlsx'; + $file = 'tests/data/Reader/XLSX/blankcell.xlsx'; $reader = new XlsxReader(); $spreadsheet = $reader->load($file); self::assertSame('', $spreadsheet->getProperties()->getTitle()); From 7e3afabba833fcea44af37d7dc5a0ab15e6169ba Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 6 Jul 2024 20:24:43 -0700 Subject: [PATCH 14/30] Formatting errors --- tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php b/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php index cedfe8dd2..7faf8a6ad 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/NoTitleTest.php @@ -41,7 +41,8 @@ class NoTitleTest extends TestCase [17, 18, 19, 20, 21, 22], [27, 28, 29, 30, 31, 32], [37, 38, 39, 40, 41, 42], - ]); + ] + ); $sheet->getStyle('B2:D4')->getBorders()->applyFromArray( [ 'allBorders' => [ From f6823c79e825518afbc6aa883ff5e22630865444 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 6 Jul 2024 20:26:15 -0700 Subject: [PATCH 15/30] More Doc Updates --- docs/references/features-cross-reference.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/references/features-cross-reference.md b/docs/references/features-cross-reference.md index 23dfbb333..746357894 100644 --- a/docs/references/features-cross-reference.md +++ b/docs/references/features-cross-reference.md @@ -495,7 +495,7 @@ ✔ N/A ● - ● + ● 7 Number Format Mask @@ -517,7 +517,7 @@ ✔ N/A ✖ - ● + ● 7 Horizontal @@ -583,7 +583,7 @@ ✔ N/A ✔ - ✔ + ● 7 Patterned @@ -605,7 +605,7 @@ ✔ N/A ✔ - ✔ + ● 7 Font Face @@ -704,7 +704,7 @@ ✔ N/A ● - ✔ + ● 7 Line Style @@ -1004,6 +1004,7 @@ 4. Xlsx forms and controls can be read and written but not otherwise manipulated 5. Xlsx macros can be read and written; their values can be retrieved and changed, but only in a binary form which is unlikely to be useful 6. There is very limited support for reading styles from an Ods spreadsheet. Writing styles has better support, although Number Format is incomplete. +7. In most cases, Html reader processes only inline styles; styles provided by Css classes may be ignored. ## Writers From 81964f991ac9b73058ebc402a674d067540ff3ad Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sun, 7 Jul 2024 07:26:18 -0700 Subject: [PATCH 16/30] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ecca9678..96cc24630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Xls Conditional Format Improvements. [PR #4030](https://github.com/PHPOffice/PhpSpreadsheet/pull/4030) [PR #4033](https://github.com/PHPOffice/PhpSpreadsheet/pull/4033) - Conditional Range Unions and Intersections [Issue #4039](https://github.com/PHPOffice/PhpSpreadsheet/issues/4039) [PR #4042](https://github.com/PHPOffice/PhpSpreadsheet/pull/4042) - Csv Reader allow use of html mimetype. [Issue #4036](https://github.com/PHPOffice/PhpSpreadsheet/issues/4036) [PR #4049](https://github.com/PHPOffice/PhpSpreadsheet/pull/4040) +- Ods comments with newlines. [Issue #4081](https://github.com/PHPOffice/PhpSpreadsheet/issues/4081) [PR #4086](https://github.com/PHPOffice/PhpSpreadsheet/pull/4086) ## 2024-05-11 - 2.1.0 From f6f155263c7cfb0feafcb71d1a7af3526548703d Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:13:50 -0700 Subject: [PATCH 17/30] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b595933..760df33ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Problem rendering line chart with missing plot label. [PR #4074](https://github.com/PHPOffice/PhpSpreadsheet/pull/4074) - More RTL in Xlsx/Html Comments [Issue #4004](https://github.com/PHPOffice/PhpSpreadsheet/issues/4004) [PR #4065](https://github.com/PHPOffice/PhpSpreadsheet/pull/4065) - Empty String in sharedStrings. [Issue #4063](https://github.com/PHPOffice/PhpSpreadsheet/issues/4063) [PR #4064](https://github.com/PHPOffice/PhpSpreadsheet/pull/4064) +- Ods Xml Reader and Whitespace Text Nodes. [Issue #804](https://github.com/PHPOffice/PhpSpreadsheet/issues/804) [PR #4087](https://github.com/PHPOffice/PhpSpreadsheet/pull/4087) ## 2024-05-11 - 2.1.0 From 18e3c00e405d84e09438f180150f9759f82df693 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 11 Jul 2024 19:48:12 -0700 Subject: [PATCH 18/30] Performance Improvements for Csv Reader Fix #460. Another in the "better late than never" series, closed as stale in June 2018. Ods Writer and Ods Reader handle booleans differently; what is worse, neither of them do it correctly. They will now match the behavior of LibreOffice. Reporter said that part of the xml would vary depending on locale; I believe that part is never actually used, but I do emulate that behavior. --- src/PhpSpreadsheet/Reader/Ods.php | 2 +- src/PhpSpreadsheet/Writer/Ods/Content.php | 5 +- .../CalculationFunctionListTest.php | 6 -- .../Calculation/CalculationTest.php | 6 -- .../Reader/Ods/BooleanDataTest.php | 83 +++++++++++++++++++ tests/data/Writer/Ods/content-with-data.xml | 8 +- 6 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Ods/BooleanDataTest.php diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index ceb345dc3..e3d86e624 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -492,7 +492,7 @@ class Ods extends BaseReader break; case 'boolean': $type = DataType::TYPE_BOOL; - $dataValue = ($allCellDataText == 'TRUE') ? true : false; + $dataValue = ($cellData->getAttributeNS($officeNs, 'boolean-value') === 'true') ? true : false; break; case 'percentage': diff --git a/src/PhpSpreadsheet/Writer/Ods/Content.php b/src/PhpSpreadsheet/Writer/Ods/Content.php index 7b052bbcc..7ffcd46b6 100644 --- a/src/PhpSpreadsheet/Writer/Ods/Content.php +++ b/src/PhpSpreadsheet/Writer/Ods/Content.php @@ -2,6 +2,7 @@ namespace PhpOffice\PhpSpreadsheet\Writer\Ods; +use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Calculation\Exception as CalculationException; use PhpOffice\PhpSpreadsheet\Cell\Cell; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; @@ -209,8 +210,8 @@ class Content extends WriterPart switch ($cell->getDataType()) { case DataType::TYPE_BOOL: $objWriter->writeAttribute('office:value-type', 'boolean'); - $objWriter->writeAttribute('office:value', $cell->getValueString()); - $objWriter->writeElement('text:p', $cell->getValueString()); + $objWriter->writeAttribute('office:boolean-value', $cell->getValue() ? 'true' : 'false'); + $objWriter->writeElement('text:p', Calculation::getInstance()->getLocaleBoolean($cell->getValue() ? 'TRUE' : 'FALSE')); break; case DataType::TYPE_ERROR: diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php index f961c7ac7..978162667 100644 --- a/tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationFunctionListTest.php @@ -13,21 +13,15 @@ class CalculationFunctionListTest extends TestCase { private string $compatibilityMode; - private string $locale; - protected function setUp(): void { $this->compatibilityMode = Functions::getCompatibilityMode(); - $calculation = Calculation::getInstance(); - $this->locale = $calculation->getLocale(); Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); } protected function tearDown(): void { Functions::setCompatibilityMode($this->compatibilityMode); - $calculation = Calculation::getInstance(); - $calculation->setLocale($this->locale); } /** diff --git a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php index 79685d267..797b0bfd7 100644 --- a/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php +++ b/tests/PhpSpreadsheetTests/Calculation/CalculationTest.php @@ -15,21 +15,15 @@ class CalculationTest extends TestCase { private string $compatibilityMode; - private string $locale; - protected function setUp(): void { $this->compatibilityMode = Functions::getCompatibilityMode(); - $calculation = Calculation::getInstance(); - $this->locale = $calculation->getLocale(); Functions::setCompatibilityMode(Functions::COMPATIBILITY_EXCEL); } protected function tearDown(): void { Functions::setCompatibilityMode($this->compatibilityMode); - $calculation = Calculation::getInstance(); - $calculation->setLocale($this->locale); } /** diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/BooleanDataTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/BooleanDataTest.php new file mode 100644 index 000000000..28efec197 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/BooleanDataTest.php @@ -0,0 +1,83 @@ +locale = $calculation->getLocale(); + } + + protected function tearDown(): void + { + $calculation = Calculation::getInstance(); + $calculation->setLocale($this->locale); + if ($this->tempfile !== '') { + unlink($this->tempfile); + $this->tempfile = ''; + } + } + + public function testBooleanData(): void + { + $spreadsheetOld = new Spreadsheet(); + $sheetOld = $spreadsheetOld->getActiveSheet(); + $sheetOld->getCell('A1')->setValue(true); + $sheetOld->getCell('A2')->setValue(false); + $writer = new OdsWriter($spreadsheetOld); + $this->tempfile = File::temporaryFileName(); + $writer->save($this->tempfile); + $spreadsheetOld->disconnectWorksheets(); + $reader = new OdsReader(); + $spreadsheet = $reader->load($this->tempfile); + $sheet = $spreadsheet->getActiveSheet(); + self::assertTrue($sheet->getCell('A1')->getValue()); + self::assertFalse($sheet->getCell('A2')->getValue()); + $spreadsheet->disconnectWorksheets(); + $zipFile = 'zip://' . $this->tempfile . '#content.xml'; + $contents = (string) file_get_contents($zipFile); + self::assertStringContainsString('TRUE', $contents); + self::assertStringContainsString('FALSE', $contents); + } + + public function testBooleanDataGerman(): void + { + $calculation = Calculation::getInstance(); + $calculation->setLocale('de'); + $spreadsheetOld = new Spreadsheet(); + $sheetOld = $spreadsheetOld->getActiveSheet(); + $sheetOld->getCell('A1')->setValue(true); + $sheetOld->getCell('A2')->setValue(false); + $writer = new OdsWriter($spreadsheetOld); + $this->tempfile = File::temporaryFileName(); + $writer->save($this->tempfile); + $spreadsheetOld->disconnectWorksheets(); + $reader = new OdsReader(); + $spreadsheet = $reader->load($this->tempfile); + $sheet = $spreadsheet->getActiveSheet(); + self::assertTrue($sheet->getCell('A1')->getValue()); + self::assertFalse($sheet->getCell('A2')->getValue()); + $spreadsheet->disconnectWorksheets(); + $zipFile = 'zip://' . $this->tempfile . '#content.xml'; + $contents = (string) file_get_contents($zipFile); + self::assertStringContainsString('WAHR', $contents); + self::assertStringContainsString('FALSCH', $contents); + self::assertStringNotContainsString('TRUE', $contents); + self::assertStringNotContainsString('FALSE', $contents); + } +} diff --git a/tests/data/Writer/Ods/content-with-data.xml b/tests/data/Writer/Ods/content-with-data.xml index 12140fa92..bb115583c 100644 --- a/tests/data/Writer/Ods/content-with-data.xml +++ b/tests/data/Writer/Ods/content-with-data.xml @@ -95,11 +95,11 @@ - - 1 + + TRUE - - + + FALSE 1 1 From 6ab27d2931c3a996e481cb7a226b56a72c670301 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 11 Jul 2024 20:05:16 -0700 Subject: [PATCH 19/30] Xlsx Writer Rich Text and TYPE_STRING Fix #476. Another in the "better late than never" series, closed as stale in June 2018. Xlsx Writer expects cells containing RichText to have DataType `TYPE_INLINE`; but the spreadsheet associated with the issue has the cell defined as `TYPE_STRING`. Change Writer to handle RichText TYPE_STRING appropriately. --- src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php | 2 +- .../Writer/Xlsx/Issue476Test.php | 30 ++++++++++++++++++ tests/data/Writer/XLSX/issue.476.xlsx | Bin 0 -> 9028 bytes 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/Issue476Test.php create mode 100644 tests/data/Writer/XLSX/issue.476.xlsx diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php index a1940f329..bd6eec367 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Worksheet.php @@ -1528,7 +1528,7 @@ class Worksheet extends WriterPart break; case 's': // String - $this->writeCellString($objWriter, $mappedType, $cellValueString, $flippedStringTable); + $this->writeCellString($objWriter, $mappedType, ($cellValue instanceof RichText) ? $cellValue : $cellValueString, $flippedStringTable); break; case 'f': // Formula diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue476Test.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue476Test.php new file mode 100644 index 000000000..513fe7940 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue476Test.php @@ -0,0 +1,30 @@ +load('tests/data/Writer/XLSX/issue.476.xlsx'); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + + $sheet = $reloadedSpreadsheet->getActiveSheet(); + $richText = $sheet->getCell('A1')->getValue(); + self::assertInstanceOf(RichText::class, $richText); + $plainText = $richText->getPlainText(); + self::assertSame("Art. 1A of the Geneva Refugee Convention and Protocol or other international or national instruments.\n", $plainText); + + $reloadedSpreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Writer/XLSX/issue.476.xlsx b/tests/data/Writer/XLSX/issue.476.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..f76f2d227f50eb47cd663917d5c6b6b226b040cc GIT binary patch literal 9028 zcmeHNgWbFzQ5q!HP5rxGtcbbyJxTU{@z@=8Yrlw0CWH*003YFSnXxmn=t> zR1xChVdLUqq3`Qz1AWZv19oP}Lq%qP13*UH|G)8HyaMHkgW8?^gv#e?H;U^#>I*e; zm=AWtdWkt7$+UO9?kTl4&316O%Z|P#RLCWB6s;y6UGNt?9Ja4<0XKw2^)v%p}d?CkS2BM6eDAK}!pF8)qmV@6YT1==fjE z!9Trvd4iTsCqFK1U+pHc?|f=8o=`!|b zom{wyCmSWMx$`-VtPg5sH(MFd=gfF|c@Ly#FYfTpD%)Lx$=BkkbNjJUMjOeG03Pkk zJ|L4Y@rB^1YA@@X(_oW2;lsf!-!4(C&a%FP{;*j3;w6NK`a4NNWfD`45i8P!Ajv%d zCX$ac-|sx}hPXRfK_E^)z1P1vgM@Hl2r2*Fqa3KE(Zx^Lf^!?m@15aAMl|ce$Fu|5 zCcx~k-~jGICvXD4$^- zeY~;N@*Q#N$iKqipmSNcFY0Or9E*{4b$%#i^7nwQoL+J}26Tn})x%^z_Op6uJM z_xDaMBX;`lDl@qyFvy7l08n5600f9T5GwO$WhnW3%+&pi{IKa7?30YMDXfbYvy2t7UFG3 z$-I&#c~|qglo=i7NdMhj0Jsekn5dU0g*l87@s35smz_W9TYRb4knHh2m7h7vibp`s zLiFP_iHYhm$~2Lu=!s0yV*eVgDrgNdmdc9Ic={MLET6(80-Ha%S5_=nh|!OqMNif z7UV)#%WYa=w>ig0@pUfl5c_(NCH>Ov!^}`=X(n6$4HrAmL7SCI^LQN)Yw?EaHm3l` zJ=4H7mEdC#{OG&(v!3@^`{@q5Wgzf=(`Y8`Nvz?{m}C$2Mrczg@kauRCG;jQRgl~mhKt6v~4LNmesI}rsosYXv-W`m(2i^XMl`hhk{VE zFvF({f!J>sCP*sLMckL&v2tBs^Y-=WHA*XGP0z5pUD31hpiRt2&WnvI;6AYKrKo{< zcq-wd#;8{>in*bQhvEUpFeEe4%jiY7~I7PYG(+z${b!P-!f$@ z-*TF(f&{C+fYN3>=2FydB`bl3oR=qFH3RqZhU%G4@RuA6oehccN_!Xp76%7i5~;VI zXY~I_tB$dUyQ&E5DvY2O>F>0H+FQEYJk|4XcW|+T{xrm0KrJVPziJ^kqmS5+ouKPs zkJjLLIxW~hzf$OE<+`rv^{L51mbD=K5xS)F5T&*B;FXVG_f^F)Ka(m&v3@MKVTgWY zcz&Ti-{{n0cvvboZ8L6VzX?D^>0m$PLN>wtbfP!s8`ko2#0TV!bkQU!ZZk z+O95ghN*C5+a`N?dqe`L8yT9g4&smZr*-wO)*{N{m?4EGgLc3?v#gp*bdS>Tig3xP zF;&%g6mnm4)P2zLXX`cBi14PYNcpb8YCs$CWX0B^v~*zOL5CTdx?NkvsmrK~LRq2ZB&yV(<3Qrj!3D;(>@{CHOS92d`v1{gn5b#XZ` zob$?car+KmnUqk85BDXp;H(4Pm+x8!z9=I%7AJ^ zvbT4GCRhdnTU7{nVZ>7Qs z(&|RGUy{m#8XCZP?#dRGWU`Yq=#JcJWDdtk_yiH+B`#liG&HCeIw}YGG*GWz@JO3q z3ng(Cb@HV=x)w*1bjCOqR%OK$0oBke&Jv9KnluhySyHOsO}s8u=}X!X3e^_aM&+MK z%z4qTNX7sw2&0#f@4i9%>SQ?R!$)D|7+KCLDj3{o#>+2Vk|y4L<0IrisbZPmJPrY9 zg4bZfmew;f--Q5DY3=Glre;~gCbC1-&rL;|xjl#9=`fd}#T|v;#PM#Vi{- z3Ess7s?b$*b@kvCgsF{Ut=WsQ0wT!AeLofO~>0y0h}Je9P>(=}6Y33_}E5NQxD8(SP1rK8}wjheBmJDim}U%~$R6 zMAYyUs?H)`SUFqRhr*eaD9r?I?ac0+5??ij3wKzhd^08VWb=e49s@VQBI4Dd@7YO1 z;`z*r7Lwm|s_(&hE4;0CG9OVUAtN=8b914zc`lmg|#}SXw!zH|!3ZMe-afNw$)RXYJZ(p2uqTb8~OY%#SnH<(gCWWxhB9=up;HP_(LOXSm? zC4Edcas1G$A;31Vnz`O;T`s*cE zsSuopT(y8{J}D_rYD@dnzDM3yX3A|Ky?^`r){2?^TW|5#vs3`qGYOO!wfFTEk|{-o zDFt&lN^=6A)J3N;x$vxgh(>BBTpBH&md`7z(H_@veQD0+&!6?9t^UH|{*v0rQ3y?A z+?dUNlKF#S+KMt;s(deH>wTO)+LGla{*2D^mz_f#12-r<_ILV}OHW-5qwH+P6AW!A9wt5j~L zI!qGy7#CCzPjG2#3qHg$cIVlGBwf{6&1m$6Ale)Y5#XruzBVX^o_6yYD6v1+nWk3x z`7ufN{bO7gH4p2TzH=Atego6E_={ffLo@n!!Pk?4w~s2M?Oxfs*-Dh>>1fHOwYDlQ z)s%`U(Uu8riR@3-tTdgaZ!IypYlmXIb+!|CEc>( z3tqjujJbayPeh+*M zz;XE^gtz7;cSx6#a8@}6>`dMaWrL@>%G|LAn?^* zY3BXc^N>0tna2{%q~J(09`^h#$9M5l$=BX^6vPy`#rWnS8cqjHo2KEJS(MupK_({Z zvn;`Awk4_)_3lRknr$n7C1Sxk+}HT&tj$j&dDLb1UlWFI`GhfB|B+ zet&Ov%N?NT6*BxbYF!++eDlZQoVgS-YMgH|HQ^Kz?sfE6*e(P!~Y=>Gn8V@3jI zLSjTUM%G7yQ!R?cJlNc7iU?#%GM;>dWl{H=;h2XX?E^{-lZ*#*Q7vIp?srflr((#( zvdJz|TXbSv`7_d)jVbay39l(>BkySkRHx+eancRJph5diQ$gN7{IRg!T$yq*9?uQ* zlK9g{t9!3-(G54A6c;JUeJ{HSN?0__=3X=ap47_ZQyGu*@FsTLUaLDI)!ienHhV0r zPX9rrCxd#doasR;|EbS)YTnl$Y`}31oXvFG9|}3o1PM7zmX^1Fd{>^sh`4Bte1WX9 z&Eyah)F!)^FcG*nU+w>V_lbsrBvu@oB<|kJBE9Snt^Vh|TQkAkxeyl)T^aLt`&4Fl z>!dqMx0clf_75mGGr(wd(+ecB?#r-jrw!BT+HlNXFB-6-B!ToDDO&F}KY`&wCxtn_ zS?_U=6YM_y9vK)bKj4d_fjt=Q)sqVfd9hmJ5qGqqTSTwp?;Jj7P;eJRu)7iQE9LJT z{#hD;+S}N8K>2=~ev79w69->r@RP#MldkZS&qEqn7%?dewDLysfK%G@_13sX9h9>p zHu|TF8MPUTB*`9>BaTL7YzZy zO{ah6w8F*dmr>yAWusj6?&8I}OR{eR1-0SD*f(5{NBaFp_&$OYT|TlH!_1q~i|80P zMV1n}&(`kWz8L&SJZ70f_W2|40q2kb5&l3xW)6K=&JexR)6G5VeX&{^VTI=lK6{CC z%^39Xs1IW+_q`-vZcx17G@nqkYb%{#<4ym9{1mst_5{!sFO$%+B<`{ai z8^BVs&(J{TeD@Z1nC5mYFP?HIOr`iEb*+NN6`SK-=!EsdqZQ18C$6F2K~z^i-n)x6 zgKi%~dxOMQH5*H9dZ|{z=;&XcarkAW}M0gvJpeYL`zT*1GNxS16w~#NFm++J-3e|F2|;pyBg` z;h$Znh<&w>itGNXEIjN+4>^&xqSe(%$=qspz)nPWeIjoI{WIU<$$xDhm#keLsWjuo zuZeDLm&gS@t6@aYC+fkf^8QhVgt@O^J=gj`?%I zTp@-lEEfrDKr)k+>h{p@-RB>(V_YOfq__5;w2MCIiP(jUH4$h(?`Z(P_WII`c`0<$ z;ODn#_~O}?OE+MO)h@#Ljrn}KJBNNWThg8%o@WplehXRqew^Jz0ijyp0TPB3_M<++ zn{sDcg1Bwg(qmz+RS&w2ZV9c2=J7Y*S~9N7+G0gW*B!3zqP?HWEh~6o^oT@XtzT>FWAFokJ+wzeZ+4hs*rW)(XlQe%RiNOc!>ToWY~FTz)g} z01y4`Y$S;8YlL;(>(36Vdr>&jFyX=yyfw`hb9FZ3J8sR(3p3%RNFt?lb~c-tK5{sB zJyniTAiQ@V@PtRlXuaa{LLTC-nc8njW_L!jhPpkIx#^M>09zr0SsTQ2ikOAL6M zdr9?r&XU3NF-R0UE+V(%8)p{Y*Yyuuo55Nc_86^4A0=7QMXq>R_qhq%J_mS<^b(sA zTf~vK7(gEhEt2m#d3gvX864|Fczn!is~tKhCbwGN&=bLO!R=qIJao$Fc)Gb`uDh+9 zlULEO@x|(Q7$q^gj1=b+pI7TVtMQHU-&b0Th?MzOOy;dMKsiD%WPLu%-HVRTON|j8 z4SS|BlEobNRkb-(^RC@|q*@h*<%fQ+NMj>pt@E-}c4)d!{$u^)N> zE>2Nyk(TR$-NB_!pz^XuQnmD&UobHJotOM>$zfzZ*=Gyf5b~*_m|$;%iLJo=VF+R? ze)rKx$UKOe;y<5P`1gkWd;W*h3%VMA74X;2>c4?M=M)4h{?upv75LX~)1QGK5iQ*R zzxVX3oL^fIe@NO#9Le~lA@M8t*Fx(bU=`foz<(>d{wm?uis2s;+7Ouw;_<)M4}TT# zYp(N$02Q*||H6MVpI@QBhFE_VFHkewFf9*ZGGGgam2;;BSufSNLCz;Gf~U dw10yC%OvV*pdoe_0Ki6kLJ+>XlHupK{{!OgnrQ$4 literal 0 HcmV?d00001 From 9652ffe73a0f67c71c59c3b96e8fec20a6c86451 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 11 Jul 2024 20:38:22 -0700 Subject: [PATCH 20/30] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b595933..1e518a0ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Problem rendering line chart with missing plot label. [PR #4074](https://github.com/PHPOffice/PhpSpreadsheet/pull/4074) - More RTL in Xlsx/Html Comments [Issue #4004](https://github.com/PHPOffice/PhpSpreadsheet/issues/4004) [PR #4065](https://github.com/PHPOffice/PhpSpreadsheet/pull/4065) - Empty String in sharedStrings. [Issue #4063](https://github.com/PHPOffice/PhpSpreadsheet/issues/4063) [PR #4064](https://github.com/PHPOffice/PhpSpreadsheet/pull/4064) +- Changes to INDEX function. [Issue #64](https://github.com/PHPOffice/PhpSpreadsheet/issues/64) [PR #4088](https://github.com/PHPOffice/PhpSpreadsheet/pull/4088) +- Ods Reader and Whitespace Text Nodes. [Issue #804](https://github.com/PHPOffice/PhpSpreadsheet/issues/804) [PR #4087](https://github.com/PHPOffice/PhpSpreadsheet/pull/4087) ## 2024-05-11 - 2.1.0 From ed677fe6e9d2a9bb95cbad473f61cbc265835f3f Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 12 Jul 2024 09:28:23 -0700 Subject: [PATCH 21/30] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b595933..11bd48dd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Problem rendering line chart with missing plot label. [PR #4074](https://github.com/PHPOffice/PhpSpreadsheet/pull/4074) - More RTL in Xlsx/Html Comments [Issue #4004](https://github.com/PHPOffice/PhpSpreadsheet/issues/4004) [PR #4065](https://github.com/PHPOffice/PhpSpreadsheet/pull/4065) - Empty String in sharedStrings. [Issue #4063](https://github.com/PHPOffice/PhpSpreadsheet/issues/4063) [PR #4064](https://github.com/PHPOffice/PhpSpreadsheet/pull/4064) +- Html Writer Minor Fixes. [PR #4089](https://github.com/PHPOffice/PhpSpreadsheet/pull/4089) ## 2024-05-11 - 2.1.0 From 1c333d1f3d74db1634da3a5855039f9deb4c9a8a Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 15 Jul 2024 22:17:09 -0700 Subject: [PATCH 22/30] Reference to Defined Name Specifying Worksheet Name Fix #296, another entry in our magical history tour (closed as stale in 2018). Excel allows you to use a name defined on another worksheet by prefixing the sheet name, even when the scope of the defined name is its worksheet rather than the entire workbook. --- .../Calculation/Calculation.php | 9 ++-- tests/PhpSpreadsheetTests/NamedRange3Test.php | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/NamedRange3Test.php diff --git a/src/PhpSpreadsheet/Calculation/Calculation.php b/src/PhpSpreadsheet/Calculation/Calculation.php index 2f13a1634..883973cbd 100644 --- a/src/PhpSpreadsheet/Calculation/Calculation.php +++ b/src/PhpSpreadsheet/Calculation/Calculation.php @@ -5074,9 +5074,10 @@ class Calculation if ($cell === null || $pCellWorksheet === null) { return $this->raiseFormulaError("undefined name '$token'"); } + $specifiedWorksheet = trim($matches[2], "'"); $this->debugLog->writeDebugLog('Evaluating Defined Name %s', $definedName); - $namedRange = DefinedName::resolveName($definedName, $pCellWorksheet); + $namedRange = DefinedName::resolveName($definedName, $pCellWorksheet, $specifiedWorksheet); // If not Defined Name, try as Table. if ($namedRange === null && $this->spreadsheet !== null) { $table = $this->spreadsheet->getTableByName($definedName); @@ -5101,7 +5102,7 @@ class Calculation return $this->raiseFormulaError("undefined name '$definedName'"); } - $result = $this->evaluateDefinedName($cell, $namedRange, $pCellWorksheet, $stack); + $result = $this->evaluateDefinedName($cell, $namedRange, $pCellWorksheet, $stack, $specifiedWorksheet !== ''); if (isset($storeKey)) { $branchStore[$storeKey] = $result; } @@ -5580,10 +5581,10 @@ class Calculation return $args; } - private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksheet $cellWorksheet, Stack $stack): mixed + private function evaluateDefinedName(Cell $cell, DefinedName $namedRange, Worksheet $cellWorksheet, Stack $stack, bool $ignoreScope = false): mixed { $definedNameScope = $namedRange->getScope(); - if ($definedNameScope !== null && $definedNameScope !== $cellWorksheet) { + if ($definedNameScope !== null && $definedNameScope !== $cellWorksheet && !$ignoreScope) { // The defined name isn't in our current scope, so #REF $result = ExcelError::REF(); $stack->push('Error', $result, $namedRange->getName()); diff --git a/tests/PhpSpreadsheetTests/NamedRange3Test.php b/tests/PhpSpreadsheetTests/NamedRange3Test.php new file mode 100644 index 000000000..c25dec561 --- /dev/null +++ b/tests/PhpSpreadsheetTests/NamedRange3Test.php @@ -0,0 +1,52 @@ +getActiveSheet(); + $sheet1->setTitle('sheet1'); + $sheet1->setCellValue('B1', 100); + $sheet1->setCellValue('B2', 200); + $sheet1->setCellValue('B3', 300); + $sheet1->setCellValue('B4', 400); + $sheet1->setCellValue('B5', 500); + + $sheet2 = $spreadsheet->createsheet(); + $sheet2->setTitle('sheet2'); + $sheet2->setCellValue('A1', 10); + $sheet2->setCellValue('A2', 20); + $sheet2->setCellValue('A3', 30); + $sheet2->setCellValue('A4', 40); + $sheet2->setCellValue('A5', 50); + + $spreadsheet->addNamedRange( + new NamedRange('somecells', $sheet2, '$A$1:$A$5', true) + ); + $spreadsheet->addNamedRange( + new NamedRange('cellsonsheet1', $sheet1, '$B$1:$B$5') + ); + + $sheet1->getCell('G1')->setValue('=SUM(cellsonsheet1)'); + self::assertSame(1500, $sheet1->getCell('G1')->getCalculatedValue()); + $sheet1->getCell('G2')->setValue('=SUM(sheet2!somecells)'); + self::assertSame(150, $sheet1->getCell('G2')->getCalculatedValue()); + $sheet1->getCell('G3')->setValue('=SUM(somecells)'); + self::assertSame('#NAME?', $sheet1->getCell('G3')->getCalculatedValue()); + $sheet1->getCell('G4')->setValue('=SUM(sheet2!cellsonsheet1)'); + self::assertSame(1500, $sheet1->getCell('G4')->getCalculatedValue()); + $sheet1->getCell('G5')->setValue('=SUM(sheet2xxx!cellsonsheet1)'); + self::assertSame('#NAME?', $sheet1->getCell('G5')->getCalculatedValue()); + + $spreadsheet->disconnectWorksheets(); + } +} From 2a0090b9156d8e0eb4cba38fd1c88c3fb68531bb Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 17 Jul 2024 09:28:35 -0700 Subject: [PATCH 23/30] Xlsx Reader and Print/Show Gridlines Fix #912, opened in Feb. 2019, and closed as stale in Apr. 2019, and which I have re-opened to be closed properly by this PR. Another "better late than never". Original issue says that print options should not affect ShowGridlines, which seems true enough. Aside from that, the existing code isn't quite correct anyhow. Excel looks for 2 attributes, one of which must be explicitly set to true and the other of which must not be explicitly set to false, in order to determine whether PrintGridlines should be set. PhpSpreadsheet is changed to do the same. This could be treated as a BC break for the unusual situation described in the issue, but it seems more like a bug fix to me. --- .../Reader/Xlsx/SheetViewOptions.php | 9 ++-- .../Reader/Xlsx/GridlinesTest.php | 49 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php diff --git a/src/PhpSpreadsheet/Reader/Xlsx/SheetViewOptions.php b/src/PhpSpreadsheet/Reader/Xlsx/SheetViewOptions.php index 136b92fed..9d71443f4 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx/SheetViewOptions.php +++ b/src/PhpSpreadsheet/Reader/Xlsx/SheetViewOptions.php @@ -122,11 +122,12 @@ class SheetViewOptions extends BaseParserClass private function printOptions(SimpleXMLElement $printOptionsx): void { $printOptions = $printOptionsx->attributes() ?? []; - if (isset($printOptions['gridLinesSet']) && self::boolean((string) $printOptions['gridLinesSet'])) { - $this->worksheet->setShowGridlines(true); - } + // Spec is weird. gridLines (default false) + // and gridLinesSet (default true) must both be true. if (isset($printOptions['gridLines']) && self::boolean((string) $printOptions['gridLines'])) { - $this->worksheet->setPrintGridlines(true); + if (!isset($printOptions['gridLinesSet']) || self::boolean((string) $printOptions['gridLinesSet'])) { + $this->worksheet->setPrintGridlines(true); + } } if (isset($printOptions['horizontalCentered']) && self::boolean((string) $printOptions['horizontalCentered'])) { $this->worksheet->getPageSetup()->setHorizontalCentered(true); diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php new file mode 100644 index 000000000..15192711a --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php @@ -0,0 +1,49 @@ +getActiveSheet(); + $sheet2 = $spreadsheet->createSheet(); + $sheet1->setShowGridlines($display); + $sheet1->setPrintGridlines($print); + $sheet1->fromArray( + [ + [1, 2, 3], + [4, 5, 6], + [7, 8, 9], + ] + ); + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + $rsheet1 = $reloadedSpreadsheet->getSheet(0); + $rsheet2 = $reloadedSpreadsheet->getSheet(1); + self::assertSame($display, $rsheet1->getShowGridlines()); + self::assertSame($print, $rsheet1->getPrintGridlines()); + self::assertTrue($rsheet2->getShowGridlines()); + self::assertFalse($rsheet2->getPrintGridlines()); + $reloadedSpreadsheet->disconnectWorksheets(); + } + + public static function loadDataProvider(): array + { + return [ + [true, true], + [true, false], + [false, true], + [false, false], + ]; + } +} From f632732564897411be42b650d63f20b7baa19946 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 17 Jul 2024 10:45:56 -0700 Subject: [PATCH 24/30] Scrutinizer Busy Work --- tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php index 15192711a..5387ea738 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/GridlinesTest.php @@ -17,6 +17,7 @@ class GridlinesTest extends AbstractFunctional $spreadsheet = new Spreadsheet(); $sheet1 = $spreadsheet->getActiveSheet(); $sheet2 = $spreadsheet->createSheet(); + $sheet2->setTitle('deliberatelyblank'); $sheet1->setShowGridlines($display); $sheet1->setPrintGridlines($print); $sheet1->fromArray( From d76481f668b84a78115dc17bc7128f3cc6126ba6 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 17 Jul 2024 17:41:14 -0700 Subject: [PATCH 25/30] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b595933..abdee5b3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). - Problem rendering line chart with missing plot label. [PR #4074](https://github.com/PHPOffice/PhpSpreadsheet/pull/4074) - More RTL in Xlsx/Html Comments [Issue #4004](https://github.com/PHPOffice/PhpSpreadsheet/issues/4004) [PR #4065](https://github.com/PHPOffice/PhpSpreadsheet/pull/4065) - Empty String in sharedStrings. [Issue #4063](https://github.com/PHPOffice/PhpSpreadsheet/issues/4063) [PR #4064](https://github.com/PHPOffice/PhpSpreadsheet/pull/4064) +- Xlsx Writer RichText and TYPE_STRING. [Issue #476](https://github.com/PHPOffice/PhpSpreadsheet/issues/476) [PR #4094](https://github.com/PHPOffice/PhpSpreadsheet/pull/4094) +- Ods boolean data. [Issue #460](https://github.com/PHPOffice/PhpSpreadsheet/issues/460) [PR #4093](https://github.com/PHPOffice/PhpSpreadsheet/pull/4093) ## 2024-05-11 - 2.1.0 From 10823ee00ab958c1149280a5f3284ab38a3669c7 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 24 Jul 2024 01:15:11 -0700 Subject: [PATCH 26/30] Changelog Prep for 2.2.0 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1241d00ed..c731b8663 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com) and this project adheres to [Semantic Versioning](https://semver.org). -## TBD - 2.2.0 +## 2024-07-24 - 2.2.0 ### Added From b86629ff58eb8c9294da0e4647bfaaf5dca58784 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 24 Jul 2024 07:08:44 -0700 Subject: [PATCH 27/30] Prepare Changelog For Next Release --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c731b8663..cd609aed9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com) and this project adheres to [Semantic Versioning](https://semver.org). +## TBD - 3.0.0 + +### Added + +- Nothing + +### Changed + +- Nothing + +### Deprecated + +- Nothing + +### Moved + +- Nothing + +### Fixed + +- Nothing + ## 2024-07-24 - 2.2.0 ### Added From 762d73daf5b4e3072aa5a56f5a42bb0b0bba78b9 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 26 Jul 2024 08:30:52 -0700 Subject: [PATCH 28/30] Addsheet May Leave Active Sheet Uninitialized Fix #4112. Direct cause is that `applyStylesFromArray` tries to save and restore `activeSheetIndex`. However, if activeSheetIndex is -1, indicating no active sheet, the restore should not be attempted. Code is changed to test before attempting to restore. The actual problem, however, is that user specified a sheet number for `addSheet`. That method will set activeSheetIndex most of the time, but this was a gap - when the supplied sheet number (0 in this case) is greater than activeSheetIndex (-1 in this case), it was leaving activeSheetIndex as -1. It is changed to set activeSheetIndex to 0 when activeSheetIndex is negative. --- CHANGELOG.md | 2 +- src/PhpSpreadsheet/Spreadsheet.php | 3 ++ src/PhpSpreadsheet/Worksheet/Worksheet.php | 4 +- .../Worksheet/Issue4112Test.php | 43 +++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index cd609aed9..e645df82e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed -- Nothing +- Add Sheet may leave Active Sheet uninitialized. [Issue #4112](https://github.com/PHPOffice/PhpSpreadsheet/issues/4112) [PR #4113](https://github.com/PHPOffice/PhpSpreadsheet/pull/4113) ## 2024-07-24 - 2.2.0 diff --git a/src/PhpSpreadsheet/Spreadsheet.php b/src/PhpSpreadsheet/Spreadsheet.php index e571cc4f6..bcea8e6a7 100644 --- a/src/PhpSpreadsheet/Spreadsheet.php +++ b/src/PhpSpreadsheet/Spreadsheet.php @@ -558,6 +558,9 @@ class Spreadsheet implements JsonSerializable if ($this->activeSheetIndex >= $sheetIndex) { ++$this->activeSheetIndex; } + if ($this->activeSheetIndex < 0) { + $this->activeSheetIndex = 0; + } } if ($worksheet->getParent() === null) { diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 0bb64ba59..7afa82b5d 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -3684,7 +3684,9 @@ class Worksheet implements IComparable $originalSelected = $this->selectedCells; $this->getStyle($coordinate)->applyFromArray($styleArray); $this->selectedCells = $originalSelected; - $spreadsheet->setActiveSheetIndex($activeSheetIndex); + if ($activeSheetIndex >= 0) { + $spreadsheet->setActiveSheetIndex($activeSheetIndex); + } return true; } diff --git a/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php b/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php new file mode 100644 index 000000000..ec232b08e --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php @@ -0,0 +1,43 @@ +removeSheetByIndex(0); + $worksheet = new Worksheet($mySpreadsheet, 'addedsheet'); + self::assertSame(-1, $mySpreadsheet->getActiveSheetIndex()); + $mySpreadsheet->addSheet($worksheet, 0); + self::assertSame('addedsheet', $mySpreadsheet->getActiveSheet()->getTitle()); + $row = 1; + $col = 1; + $worksheet->getCell([$col, $row])->setValue('id_uti'); + self::assertSame('id_uti', $worksheet->getCell([$col, $row])->getValue()); + $mySpreadsheet->disconnectWorksheets(); + } + + public static function providerSheetNumber(): array + { + return [ + 'problem case' => [0], + 'normal case' => [null], + 'negative 1 (as if there were no sheets)' => [-1], + 'diffeent negative number' => [-4], + 'positive number' => [4], + ]; + } +} From 1df4b17d55e36d071993ff156a2ff4ed00b97b46 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 26 Jul 2024 08:53:00 -0700 Subject: [PATCH 29/30] Scrutinizer Found a Real Problem My test was imperfect,and Scrutinizer detected it. --- CHANGELOG.md | 2 +- tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e645df82e..6044ff12f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed -- Add Sheet may leave Active Sheet uninitialized. [Issue #4112](https://github.com/PHPOffice/PhpSpreadsheet/issues/4112) [PR #4113](https://github.com/PHPOffice/PhpSpreadsheet/pull/4113) +- Add Sheet may leave Active Sheet uninitialized. [Issue #4112](https://github.com/PHPOffice/PhpSpreadsheet/issues/4112) [PR #4114](https://github.com/PHPOffice/PhpSpreadsheet/pull/4114) ## 2024-07-24 - 2.2.0 diff --git a/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php b/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php index ec232b08e..9b230a9c9 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php +++ b/tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php @@ -21,7 +21,7 @@ class Issue4112Test extends AbstractFunctional $mySpreadsheet->removeSheetByIndex(0); $worksheet = new Worksheet($mySpreadsheet, 'addedsheet'); self::assertSame(-1, $mySpreadsheet->getActiveSheetIndex()); - $mySpreadsheet->addSheet($worksheet, 0); + $mySpreadsheet->addSheet($worksheet, $sheetNumber); self::assertSame('addedsheet', $mySpreadsheet->getActiveSheet()->getTitle()); $row = 1; $col = 1; From 459f442b9ec62d6b8679788d27d47d69846012d0 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Fri, 26 Jul 2024 11:52:35 -0700 Subject: [PATCH 30/30] Additional Test New test testGifIssue4112 uses the same technique as reported in the original issue, and it would fail on all PhpSpreadsheet releases, not just 2.2.0. --- .../Writer/Xls/XlsGifBmpTest.php | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/PhpSpreadsheetTests/Writer/Xls/XlsGifBmpTest.php b/tests/PhpSpreadsheetTests/Writer/Xls/XlsGifBmpTest.php index 74901dd40..21e64f2a9 100644 --- a/tests/PhpSpreadsheetTests/Writer/Xls/XlsGifBmpTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Xls/XlsGifBmpTest.php @@ -9,6 +9,7 @@ use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Drawing; use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing; +use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional; class XlsGifBmpTest extends AbstractFunctional @@ -83,6 +84,34 @@ class XlsGifBmpTest extends AbstractFunctional $reloadedSpreadsheet->disconnectWorksheets(); } + public function testGifIssue4112(): void + { + $spreadsheet = new Spreadsheet(); + $spreadsheet->removeSheetByIndex(0); + $sheet = new Worksheet($spreadsheet, 'Insured List'); + $spreadsheet->addSheet($sheet, 0); + + // Add a drawing to the worksheet + $drawing = new Drawing(); + $drawing->setName('Letters G, I, and G'); + $drawing->setDescription('Handwritten G, I, and F'); + $drawing->setPath(__DIR__ . '/../../../../samples/images/gif.gif'); + $drawing->setHeight(36); + $drawing->setWorksheet($sheet); + $drawing->setCoordinates('A1'); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xls'); + $spreadsheet->disconnectWorksheets(); + $worksheet = $reloadedSpreadsheet->getActiveSheet(); + $drawings = $worksheet->getDrawingCollection(); + self::assertCount(1, $drawings); + foreach ($worksheet->getDrawingCollection() as $drawing) { + $mimeType = ($drawing instanceof MemoryDrawing) ? $drawing->getMimeType() : 'notmemorydrawing'; + self::assertEquals('image/png', $mimeType); + } + $reloadedSpreadsheet->disconnectWorksheets(); + } + public function testInvalidTimestamp(): void { $this->expectException(ReaderException::class);