Don't Interfere with SelectedCells And Other Changes

Xls Reader processing Conditionals interferes with the previously established SelectedCells. Make sure that value is restored.

StopIfTrue should always be set for Xls spreadsheet.

Set NoFormatSet to true unless any of Font, Fill, or Borders is specified in Conditional Style.

In my notes for PR PHPOffice#3372, I mentioned that I could not include some Xls tests because of errors in the software at that time. This PR fixes those errors, so I am adding the missing test, and making the equivalent Xlsx test more comprehensive.

Author: oleibman

src/PhpSpreadsheet/Reader/Xls.php

@@ -703,6 +703,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
         // Parse the individual sheets
         $this->activeSheetSet = false;
         foreach ($this->sheets as $sheet) {
+            $selectedCells = '';
             if ($sheet['sheetType'] != 0x00) {
                 // 0x00: Worksheet, 0x02: Chart, 0x06: Visual Basic module
                 continue;
@@ -910,7 +911,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
 
                         break;
                     case self::XLS_TYPE_SELECTION:
-                        $this->readSelection();
+                        $selectedCells = $this->readSelection();
 
                         break;
                     case self::XLS_TYPE_MERGEDCELLS:
@@ -1112,6 +1113,9 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                     $this->phpSheet->getComment($cellAddress)->setAuthor($noteDetails['author'])->setText($this->parseRichText($noteDetails['objTextData']['text']));
                 }
             }
+            if ($selectedCells !== '') {
+                $this->phpSheet->setSelectedCells($selectedCells);
+            }
         }
         if ($this->activeSheetSet === false) {
             $this->spreadsheet->setActiveSheetIndex(0);
@@ -4376,10 +4380,11 @@ private function readPane(): void
     /**
      * Read SELECTION record. There is one such record for each pane in the sheet.
      */
-    private function readSelection(): void
+    private function readSelection(): string
     {
         $length = self::getUInt2d($this->data, $this->pos + 2);
         $recordData = $this->readRecordData($this->data, $this->pos + 4, $length);
+        $selectedCells = '';
 
         // move stream pointer to next record
         $this->pos += 4 + $length;
@@ -4421,6 +4426,8 @@ private function readSelection(): void
 
             $this->phpSheet->setSelectedCells($selectedCells);
         }
+
+        return $selectedCells;
     }
 
     private function includeCellRangeFiltered(string $cellRangeAddress): bool
@@ -7410,6 +7417,7 @@ private function readCFRule(array $cellRangeAddresses): void
         $options = self::getInt4d($recordData, 6);
 
         $style = new Style(false, true); // non-supervisor, conditional
+        $noFormatSet = true;
         //$this->getCFStyleOptions($options, $style);
 
         $hasFontRecord = (bool) ((0x04000000 & $options) >> 26);
@@ -7429,6 +7437,7 @@ private function readCFRule(array $cellRangeAddresses): void
             $fontStyle = substr($recordData, $offset, 118);
             $this->getCFFontStyle($fontStyle, $style);
             $offset += 118;
+            $noFormatSet = false;
         }
 
         if ($hasAlignmentRecord === true) {
@@ -7441,12 +7450,14 @@ private function readCFRule(array $cellRangeAddresses): void
             $borderStyle = substr($recordData, $offset, 8);
             $this->getCFBorderStyle($borderStyle, $style, $hasBorderLeft, $hasBorderRight, $hasBorderTop, $hasBorderBottom);
             $offset += 8;
+            $noFormatSet = false;
         }
 
         if ($hasFillRecord === true) {
             $fillStyle = substr($recordData, $offset, 4);
             $this->getCFFillStyle($fillStyle, $style);
             $offset += 4;
+            $noFormatSet = false;
         }
 
         if ($hasProtectionRecord === true) {
@@ -7474,7 +7485,7 @@ private function readCFRule(array $cellRangeAddresses): void
             $offset += $size2;
         }
 
-        $this->setCFRules($cellRangeAddresses, $type, $operator, $formula1, $formula2, $style);
+        $this->setCFRules($cellRangeAddresses, $type, $operator, $formula1, $formula2, $style, $noFormatSet);
     }
 
     /*private function getCFStyleOptions(int $options, Style $style): void
@@ -7604,12 +7615,14 @@ private function readCFFormula(string $recordData, int $offset, int $size): floa
         }
     }
 
-    private function setCFRules(array $cellRanges, string $type, string $operator, null|float|int|string $formula1, null|float|int|string $formula2, Style $style): void
+    private function setCFRules(array $cellRanges, string $type, string $operator, null|float|int|string $formula1, null|float|int|string $formula2, Style $style, bool $noFormatSet): void
     {
         foreach ($cellRanges as $cellRange) {
             $conditional = new Conditional();
+            $conditional->setNoFormatSet($noFormatSet);
             $conditional->setConditionType($type);
             $conditional->setOperatorType($operator);
+            $conditional->setStopIfTrue(true);
             if ($formula1 !== null) {
                 $conditional->addCondition($formula1);
             }

tests/PhpSpreadsheetTests/Reader/Xls/Issue3202Test.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xls;
+
+use PhpOffice\PhpSpreadsheet\Reader\Xls as XlsReader;
+use PHPUnit\Framework\TestCase;
+
+class Issue3202Test extends TestCase
+{
+    public function testSelectedCellWithConditionals(): void
+    {
+        // Unknown index notice when loading
+        $filename = 'tests/data/Reader/XLS/issue.3202.xls';
+        $reader = new XlsReader();
+        $spreadsheet = $reader->load($filename);
+        $sheet = $spreadsheet->getActiveSheet();
+        self::assertSame('A2', $sheet->getSelectedCells());
+
+        $collection = $sheet->getConditionalStylesCollection();
+        self::assertCount(1, $collection);
+        $conditionalArray = $collection['A1:A5'];
+        self::assertCount(3, $conditionalArray);
+
+        $conditions = $conditionalArray[0]->getConditions();
+        self::assertCount(1, $conditions);
+        self::assertSame('$A1=3', $conditions[0]);
+        self::assertTrue($conditionalArray[0]->getNoFormatSet());
+        self::assertTrue($conditionalArray[0]->getStopIfTrue());
+
+        $conditions = $conditionalArray[1]->getConditions();
+        self::assertCount(1, $conditions);
+        self::assertSame('$A1>5', $conditions[0]);
+        self::assertFalse($conditionalArray[1]->getNoFormatSet());
+        self::assertTrue($conditionalArray[1]->getStopIfTrue());
+
+        $conditions = $conditionalArray[2]->getConditions();
+        self::assertCount(1, $conditions);
+        self::assertSame('$A1>1', $conditions[0]);
+        self::assertFalse($conditionalArray[2]->getNoFormatSet());
+        self::assertTrue($conditionalArray[2]->getStopIfTrue());
+
+        $spreadsheet->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Reader/Xlsx/ConditionalNoFormatSetTest.php

@@ -33,6 +33,31 @@ public function testNoFormatTest(): void
 
         $reader = new XlsxReader();
         $spreadsheet = $reader->load($testfile);
+        $sheet = $spreadsheet->getactiveSheet();
+
+        $collection = $sheet->getConditionalStylesCollection();
+        self::assertCount(1, $collection);
+        $conditionalArray = $collection['A1:A5'];
+        self::assertCount(3, $conditionalArray);
+
+        $conditions = $conditionalArray[0]->getConditions();
+        self::assertCount(1, $conditions);
+        self::assertSame('$A1=3', $conditions[0]);
+        self::assertTrue($conditionalArray[0]->getNoFormatSet());
+        self::assertTrue($conditionalArray[0]->getStopIfTrue());
+
+        $conditions = $conditionalArray[1]->getConditions();
+        self::assertCount(1, $conditions);
+        self::assertSame('$A1>5', $conditions[0]);
+        self::assertFalse($conditionalArray[1]->getNoFormatSet());
+        self::assertTrue($conditionalArray[1]->getStopIfTrue());
+
+        $conditions = $conditionalArray[2]->getConditions();
+        self::assertCount(1, $conditions);
+        self::assertSame('$A1>1', $conditions[0]);
+        self::assertFalse($conditionalArray[2]->getNoFormatSet());
+        self::assertFalse($conditionalArray[2]->getStopIfTrue());
+
         $outfile = File::temporaryFilename();
         $writer = new XlsxWriter($spreadsheet);
         $writer->save($outfile);