Repository navigation
Fix removeColumn() throwing "Worksheet no longer exists" on a sheet with a table - #5059
Open
aswinkumar863 wants to merge 3 commits into
Open
aswinkumar863 wants to merge 3 commits into
aswinkumar863 wants to merge 3 commits into
Conversation
When a table header cell's value changes, Cell::setValueExplicit() calls
updateIfCellIsTableHeader(), which rewrites structured references in
every formula of the workbook. That loop fetches other cells through
Worksheet::getCell(), and the cell collection detaches the current
(dirty) cell when it switches to another one. The rest of
setValueExplicit() then called $this->getWorksheet() on the detached
cell and threw "Worksheet no longer exists".
ReferenceHelper moves header cells with setValueExplicit() when a
column is inserted or removed, so e.g. removeColumn('A') on a sheet with
a table at A1:C4 always failed.
Keep a reference to the cell collection and retrieve the cell again
after the header update, which re-attaches it before its worksheet and
style are used.
Contributor
|
While testing this branch: after $sheet->fromArray([['Name', 'Qty', 'Price'], ['a', 1, 2], ['b', 3, 4]]);
$sheet->setCellValue('J1', '=COUNTA(Sales[Name])');
$sheet->setCellValue('J2', '=SUM(Sales[Qty])');
$sheet->setCellValue('J3', '=SUM(Sales[Price])');
$sheet->addTable(new Table('A1:C3', 'Sales'));
$sheet->removeColumn('B');
Outside the scope of this PR; noting it here as it is the same code path. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is:
Checklist:
Why is this change needed?
Fixes #5058.
Removing (or inserting) a column that moves the header row of a table throws
Worksheet no longer exists.When a header cell changes,
Cell::setValueExplicit()callsupdateIfCellIsTableHeader(), which updates the structured references in all formulas. It reads those cells withgetCell(), and that detaches the cell we are setting. After that,setValueExplicit()calls$this->getWorksheet()and fails.The fix keeps the cell collection in a variable and gets the cell again after the header update, so it is attached again before its worksheet and style are used.
Added
ColumnInsertRemoveTestwith 3 tests. The column A test fails without the fix. The full test suite passes.This is a different problem from #5050 (that one is in the loop at the end of
Worksheet::removeColumn()).