Spreadsheet copy removes worksheets from source

[javfres]

This is:

- [X] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

I am creating a duplicate of a spreadsheet and I expect to have a copy of the sheets while keeping the original ones.

What is the current behavior?

Copy methods removes the original sheets from the source spreadsheet. The problem is that the Worksheet@rebindParent method that calls Spreadsheet@removeSheetByIndex. I understand this method is useful when moving a sheet from one spreadsheet to another, but does not work for copies.

What are the steps to reproduce?

This is a simple code to reproduce

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();

// The source has 1 sheet
error_log("#sheets: " . $spreadsheet->getSheetCount()); 

// Let's do the copy
$copy = $spreadsheet->copy();

// Copy is fine
error_log("#sheets in copy: " . $copy->getSheetCount());

// Source spreadsheet has no sheets !!!!!
error_log("#sheets: " . $spreadsheet->getSheetCount());

Which versions of PhpSpreadsheet and PHP are affected?

At least 1.8 and 1.12

I as using 1.8 in my project and upgraded to the last 1.12 and I found the problem in both of them.

[Vagir-dev]

Hi!

Try this patch:

Index: src/PhpSpreadsheet/Spreadsheet.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/PhpSpreadsheet/Spreadsheet.php	(revision 24be4824109f3d647c3d959075129e975530b072)
+++ src/PhpSpreadsheet/Spreadsheet.php	(date 1590184383836)
@@ -974,7 +974,7 @@
         $worksheetCount = count($this->workSheetCollection);
         for ($i = 0; $i < $worksheetCount; ++$i) {
             $this->workSheetCollection[$i] = $this->workSheetCollection[$i]->copy();
-            $this->workSheetCollection[$i]->rebindParent($this);
+            $this->workSheetCollection[$i]->rebindParent($this, true);
         }
 
         return $copied;
Index: src/PhpSpreadsheet/Worksheet/Worksheet.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/PhpSpreadsheet/Worksheet/Worksheet.php	(revision 24be4824109f3d647c3d959075129e975530b072)
+++ src/PhpSpreadsheet/Worksheet/Worksheet.php	(date 1590184545387)
@@ -792,10 +792,11 @@
      * Re-bind parent.
      *
      * @param Spreadsheet $parent
+     * @param boolean $dontRemove - true if not neseccery to remove worksheet from current spreadsheet
      *
      * @return $this
      */
-    public function rebindParent(Spreadsheet $parent)
+    public function rebindParent(Spreadsheet $parent, $dontRemove = false)
     {
         if ($this->parent !== null) {
             $namedRanges = $this->parent->getNamedRanges();
@@ -803,9 +804,11 @@
                 $parent->addNamedRange($namedRange);
             }
 
-            $this->parent->removeSheetByIndex(
-                $this->parent->getIndex($this)
-            );
+            if (!$dontRemove) {
+                $this->parent->removeSheetByIndex(
+                    $this->parent->getIndex($this)
+                );
+            }
         }
         $this->parent = $parent;

Does it solve the problem?

[javfres]

Works like a charm, thanks.

I've end up doing a ugly fix myself while this is reaches a new release. Maybe it's useful for somebody:

function ugly_copy_spreadsheet(Spreadsheet $spreadsheet){

	$copied = clone $spreadsheet;
	$copied->disconnectWorksheets();

	for($i=0; $i<$spreadsheet->getSheetCount(); $i++){

	    $sheet_copy = $spreadsheet->getSheet($i)->copy;

	    // Do: $sheet_copy->parent = null;
	    $reflector = new ReflectionClass($sheet_copy);
	    $parent_property = $reflector->getProperty('parent');
	    $parent_property->setAccessible(true);
	    $parent_property->setValue($sheet_copy, null);

	    $copied->addSheet($sheet_copy);
	}

	return $copied;

}

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

[oleibman]

Supplied code works correctly now. Fixed by PR #2951 in July 2022.