Skip to content

Commit

Permalink
Addsheet May Leave Active Sheet Uninitialized
Browse files Browse the repository at this point in the history
Fix PHPOffice#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.
  • Loading branch information
oleibman committed Jul 26, 2024
1 parent b406367 commit 762d73d
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 2 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions src/PhpSpreadsheet/Spreadsheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,9 @@ public function addSheet(Worksheet $worksheet, ?int $sheetIndex = null): Workshe
if ($this->activeSheetIndex >= $sheetIndex) {
++$this->activeSheetIndex;
}
if ($this->activeSheetIndex < 0) {
$this->activeSheetIndex = 0;
}
}

if ($worksheet->getParent() === null) {
Expand Down
4 changes: 3 additions & 1 deletion src/PhpSpreadsheet/Worksheet/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -3684,7 +3684,9 @@ public function applyStylesFromArray(string $coordinate, array $styleArray): boo
$originalSelected = $this->selectedCells;
$this->getStyle($coordinate)->applyFromArray($styleArray);
$this->selectedCells = $originalSelected;
$spreadsheet->setActiveSheetIndex($activeSheetIndex);
if ($activeSheetIndex >= 0) {
$spreadsheet->setActiveSheetIndex($activeSheetIndex);
}

return true;
}
Expand Down
43 changes: 43 additions & 0 deletions tests/PhpSpreadsheetTests/Worksheet/Issue4112Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare(strict_types=1);

namespace PhpOffice\PhpSpreadsheetTests\Worksheet;

use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;

class Issue4112Test extends AbstractFunctional
{
/**
* Problem deleting all sheets then adding one.
*
* @dataProvider providerSheetNumber
*/
public function testIssue4112(?int $sheetNumber): void
{
$mySpreadsheet = new Spreadsheet();
$mySpreadsheet->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],
];
}
}

0 comments on commit 762d73d

Please sign in to comment.