Fix for Xls writer wrong selected cells and active sheet

PHPOffice · Mar 2, 2020 · 4969d9d · 4969d9d
1 parent cb18163
commit 4969d9d
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,13 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 ## [Unreleased]
 
+### Added
+
 - Added support for the BASE function
 - Added support for the ARABIC function
 - Conditionals - Extend Support for (NOT)CONTAINSBLANKS [#1278](https://github.com/PHPOffice/PhpSpreadsheet/pull/1278)
+
+### Fixed
+
 - Handle Error in Formula Processing Better for Xls [#1267](https://github.com/PHPOffice/PhpSpreadsheet/pull/1267)
 - Handle ConditionalStyle NumberFormat When Reading Xlsx File [#1296](https://github.com/PHPOffice/PhpSpreadsheet/pull/1296)
 - Fix Xlsx Writer's handling of decimal commas [#1282](https://github.com/PHPOffice/PhpSpreadsheet/pull/1282)
 - Fix for issue by removing test code mistakenly left in [#1328](https://github.com/PHPOffice/PhpSpreadsheet/pull/1328)
+- Fix for Xls writer wrong selected cells and active sheet [#1256](https://github.com/PHPOffice/PhpSpreadsheet/pull/1256)
 
 ## [1.10.1] - 2019-12-02
 

diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php
@@ -281,6 +281,10 @@ public function close()
     {
         $phpSheet = $this->phpSheet;
 
+        // Storing selected cells and active sheet because it changes while parsing cells with formulas.
+        $selectedCells = $this->phpSheet->getSelectedCells();
+        $activeSheetIndex = $this->phpSheet->getParent()->getActiveSheetIndex();
+
         // Write BOF record
         $this->storeBof(0x0010);
 
@@ -481,6 +485,9 @@ public function close()
         // Append
         $this->writeMsoDrawing();
 
+        // Restoring active sheet.
+        $this->phpSheet->getParent()->setActiveSheetIndex($activeSheetIndex);
+
         // Write WINDOW2 record
         $this->writeWindow2();
 
@@ -493,6 +500,9 @@ public function close()
             $this->writePanes();
         }
 
+        // Restoring selected cells.
+        $this->phpSheet->setSelectedCells($selectedCells);
+
         // Write SELECTION record
         $this->writeSelection();
 
@@ -1250,7 +1260,6 @@ private function writeWindow2()
         $fFrozenNoSplit = 0; // 0 - bit
         // no support in PhpSpreadsheet for selected sheet, therefore sheet is only selected if it is the active sheet
         $fSelected = ($this->phpSheet === $this->phpSheet->getParent()->getActiveSheet()) ? 1 : 0;
-        $fPaged = 1; // 2
         $fPageBreakPreview = $this->phpSheet->getSheetView()->getView() === SheetView::SHEETVIEW_PAGE_BREAK_PREVIEW;
 
         $grbit = $fDspFmla;
@@ -1262,8 +1271,8 @@ private function writeWindow2()
         $grbit |= $fArabic << 6;
         $grbit |= $fDspGuts << 7;
         $grbit |= $fFrozenNoSplit << 8;
-        $grbit |= $fSelected << 9;
-        $grbit |= $fPaged << 10;
+        $grbit |= $fSelected << 9; // Selected sheets.
+        $grbit |= $fSelected << 10; // Active sheet.
         $grbit |= $fPageBreakPreview << 11;
 
         $header = pack('vv', $record, $length);

diff --git a/tests/PhpSpreadsheetTests/Functional/ActiveSheetTest.php b/tests/PhpSpreadsheetTests/Functional/ActiveSheetTest.php
@@ -0,0 +1,68 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Functional;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+
+class ActiveSheetTest extends AbstractFunctional
+{
+    public function providerFormats()
+    {
+        return [
+            ['Xls'],
+        ];
+    }
+
+    /**
+     * Test load file with correct active sheet.
+     *
+     * @dataProvider providerFormats
+     *
+     * @param string $format
+     */
+    public function testActiveSheet($format)
+    {
+        $spreadsheet = new Spreadsheet();
+
+        $spreadsheet->setActiveSheetIndex(0)
+            ->setTitle('Test1')
+            ->setCellValue('D1', 1)
+            ->setCellValue('D2', 2)
+            ->setCellValue('D3', 3)
+            ->setCellValue('D4', 4)
+            ->setCellValue('D5', '=SUM(D1:D4)')
+            ->setSelectedCell('B2');
+
+        $spreadsheet->createSheet(1);
+
+        $spreadsheet->setActiveSheetIndex(1)
+            ->setTitle('Test2')
+            ->setCellValue('D1', 4)
+            ->setCellValue('E1', 3)
+            ->setCellValue('F1', 2)
+            ->setCellValue('G1', 1)
+            ->setCellValue('H1', '=SUM(D1:G4)')
+            ->setSelectedCells('A1:B2');
+
+        $spreadsheet->createSheet(2);
+
+        $spreadsheet->setActiveSheetIndex(2)
+            ->setTitle('Test3')
+            ->setCellValue('A1', 4)
+            ->setCellValue('B1', 3)
+            ->setCellValue('C1', 2)
+            ->setCellValue('D1', 1)
+            ->setCellValue('E1', '=SUM(A1:D4)')
+            ->setSelectedCells('A1:D1');
+
+        $spreadsheet->setActiveSheetIndex(1);
+
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format);
+
+        // Original object.
+        self::assertSame(1, $spreadsheet->getActiveSheetIndex());
+
+        // Saved and reloaded file.
+        self::assertSame(1, $reloadedSpreadsheet->getActiveSheetIndex());
+    }
+}
diff --git a/tests/PhpSpreadsheetTests/Functional/SelectedCellsTest.php b/tests/PhpSpreadsheetTests/Functional/SelectedCellsTest.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Functional;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+
+class SelectedCellsTest extends AbstractFunctional
+{
+    public function providerFormats()
+    {
+        return [
+            ['Xls'],
+        ];
+    }
+
+    /**
+     * Test load file with correct selected cells.
+     *
+     * @dataProvider providerFormats
+     *
+     * @param string $format
+     */
+    public function testSelectedCells($format)
+    {
+        $spreadsheet = new Spreadsheet();
+
+        $spreadsheet->setActiveSheetIndex(0)
+            ->setTitle('Test1')
+            ->setCellValue('D1', 1)
+            ->setCellValue('D2', 2)
+            ->setCellValue('D3', 3)
+            ->setCellValue('D4', 4)
+            ->setCellValue('D5', '=SUM(D1:D4)')
+            ->setSelectedCell('B2');
+
+        $spreadsheet->createSheet(1);
+
+        $spreadsheet->setActiveSheetIndex(1)
+            ->setTitle('Test2')
+            ->setCellValue('D1', 4)
+            ->setCellValue('E1', 3)
+            ->setCellValue('F1', 2)
+            ->setCellValue('G1', 1)
+            ->setCellValue('H1', '=SUM(D1:G4)')
+            ->setSelectedCells('A1:B2');
+
+        $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format);
+
+        // Original object.
+        self::assertSame('B2', $spreadsheet->setActiveSheetIndex(0)->getSelectedCells());
+        self::assertSame('A1:B2', $spreadsheet->setActiveSheetIndex(1)->getSelectedCells());
+
+        // Saved and reloaded file.
+        self::assertSame('B2', $reloadedSpreadsheet->setActiveSheetIndex(0)->getSelectedCells());
+        self::assertSame('A1:B2', $reloadedSpreadsheet->setActiveSheetIndex(1)->getSelectedCells());
+    }
+}