sc/source/core/data/markdata.cxx | 12 +++------ sc/source/core/data/markmulti.cxx | 47 ++++---------------------------------- sc/source/core/tool/address.cxx | 42 +++++++++++++++++++++++++++------ sc/source/ui/docshell/docfunc.cxx | 8 +++--- sc/source/ui/view/viewfunc.cxx | 28 ++++++++++++++++------ 5 files changed, 68 insertions(+), 69 deletions(-)
New commits: commit a578321434ada456be3fc7a91b3da9321ce01c5a Author: Gökay Şatır <gokaysa...@collabora.com> AuthorDate: Mon Oct 30 12:37:23 2023 +0300 Commit: Gökay ŞATIR <gokaysa...@collabora.com> CommitDate: Fri May 10 08:02:47 2024 +0200 Fix row deletion bug. When multiple users are editing a Calc document: * If one user selects a whole row and another one deletes a range of rows including or above the selected row, app crashes. * This PR fixes the crash. * Also when multiple rows are deleted, other user's selected row is moved only one row. This PR moves the selected row according to the deleted row count. * The cursor position was also causing a crash, fixed. Signed-off-by: Gökay Şatır <gokaysa...@collabora.com> Change-Id: Ie4b893fee7192492efacbb167b747434336384e3 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158650 Reviewed-by: Marco Cecchetti <marco.cecche...@collabora.com> Tested-by: Marco Cecchetti <marco.cecche...@collabora.com> Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167373 diff --git a/sc/source/core/data/markdata.cxx b/sc/source/core/data/markdata.cxx index afc875983ab1..98a3aebe5c8b 100644 --- a/sc/source/core/data/markdata.cxx +++ b/sc/source/core/data/markdata.cxx @@ -670,13 +670,11 @@ void ScMarkData::ShiftCols(const ScDocument& rDoc, SCCOL nStartCol, sal_Int32 nC void ScMarkData::ShiftRows(const ScDocument& rDoc, SCROW nStartRow, sal_Int32 nRowOffset) { if (bMarked) - { aMarkRange.IncRowIfNotLessThan(rDoc, nStartRow, nRowOffset); - } - else if (bMultiMarked) + if (bMultiMarked) { - aMultiSel.ShiftRows(nStartRow, nRowOffset); aMultiRange.IncRowIfNotLessThan(rDoc, nStartRow, nRowOffset); + aMultiSel.ShiftRows(nStartRow, nRowOffset); } } diff --git a/sc/source/core/tool/address.cxx b/sc/source/core/tool/address.cxx index ba6f73f47bec..b976443fc649 100644 --- a/sc/source/core/tool/address.cxx +++ b/sc/source/core/tool/address.cxx @@ -2405,17 +2405,30 @@ void ScRange::IncColIfNotLessThan(const ScDocument& rDoc, SCCOL nStartCol, SCCOL void ScRange::IncRowIfNotLessThan(const ScDocument& rDoc, SCROW nStartRow, SCROW nOffset) { - if (aStart.Row() >= nStartRow) + SCROW offset; + if (aStart.Row() > nStartRow) { - aStart.IncRow(nOffset); + offset = nOffset; + if (nStartRow + nOffset > aStart.Row()) + offset = aStart.Row() - nStartRow; + else if (nStartRow - nOffset > aStart.Row()) + offset = -1 * (aStart.Row() - nStartRow); + + aStart.IncRow(offset); if (aStart.Row() < 0) aStart.SetRow(0); else if(aStart.Row() > rDoc.MaxRow()) aStart.SetRow(rDoc.MaxRow()); } - if (aEnd.Row() >= nStartRow) + if (aEnd.Row() > nStartRow) { - aEnd.IncRow(nOffset); + offset = nOffset; + if (nStartRow + nOffset > aEnd.Row()) + offset = aEnd.Row() - nStartRow; + else if (nStartRow - nOffset > aEnd.Row()) + offset = -1 * (aEnd.Row() - nStartRow); + + aEnd.IncRow(offset); if (aEnd.Row() < 0) aEnd.SetRow(0); else if(aEnd.Row() > rDoc.MaxRow()) diff --git a/sc/source/ui/docshell/docfunc.cxx b/sc/source/ui/docshell/docfunc.cxx index aa8a5d64d7fe..c33daee69b14 100644 --- a/sc/source/ui/docshell/docfunc.cxx +++ b/sc/source/ui/docshell/docfunc.cxx @@ -2280,7 +2280,7 @@ bool ScDocFunc::InsertCells( const ScRange& rRange, const ScMarkData* pTabMark, if (bInsertRows) { - pViewSh->OnLOKInsertDeleteRow(rRange.aStart.Row(), 1); + pViewSh->OnLOKInsertDeleteRow(rRange.aStart.Row() - (eCmd == INS_INSROWS_BEFORE ? 1: 0), 1); } } @@ -2860,7 +2860,7 @@ bool ScDocFunc::DeleteCells( const ScRange& rRange, const ScMarkData* pTabMark, } if (eCmd == DelCellCmd::Rows) { - pViewSh->OnLOKInsertDeleteRow(rRange.aStart.Row(), -1); + pViewSh->OnLOKInsertDeleteRow(rRange.aStart.Row(), -1 * (rRange.aEnd.Row() - rRange.aStart.Row() + 1)); } } diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index 8d77023cbaa9..db2bb7c8e717 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -1779,11 +1779,17 @@ void ScViewFunc::OnLOKInsertDeleteRow(SCROW nStartRow, tools::Long nOffset) if (pTabViewShell->getPart() == nCurrentTabIndex) { SCROW nY = pTabViewShell->GetViewData().GetCurY(); - if (nY > nStartRow || (nY == nStartRow && nOffset > 0)) + if (nY > nStartRow) { + tools::Long offset = nOffset; + if (nOffset + nStartRow > nY) + offset = nY - nStartRow; + else if (nOffset < 0 && nStartRow - nOffset > nY) + offset = -1 * (nY - nStartRow); + ScInputHandler* pInputHdl = pTabViewShell->GetInputHandler(); SCCOL nX = pTabViewShell->GetViewData().GetCurX(); - pTabViewShell->SetCursor(nX, nY + nOffset); + pTabViewShell->SetCursor(nX, nY + offset); if (pInputHdl && pInputHdl->IsInputMode()) { pInputHdl->SetModified(); @@ -1792,8 +1798,8 @@ void ScViewFunc::OnLOKInsertDeleteRow(SCROW nStartRow, tools::Long nOffset) ScMarkData aMultiMark( pTabViewShell->GetViewData().GetMarkData() ); aMultiMark.SetMarking( false ); - aMultiMark.MarkToMulti(); - if (aMultiMark.IsMultiMarked()) + + if (aMultiMark.IsMultiMarked() || aMultiMark.IsMarked()) { aMultiMark.ShiftRows(pTabViewShell->GetViewData().GetDocument(), nStartRow, nOffset); pTabViewShell->SetMarkData(aMultiMark); commit aa836383900a468a3d50abe534de028e47e343fd Author: Gökay Şatır <gokaysa...@collabora.com> AuthorDate: Thu Oct 26 14:48:49 2023 +0300 Commit: Gökay ŞATIR <gokaysa...@collabora.com> CommitDate: Fri May 10 08:02:40 2024 +0200 Fix column deletion bug. When multiple users are editing a Calc document: * If one user selects a whole column and another one deletes a range of columns including or on the left of the selected column, app crashes. * This PR fixes the crash. * Also when multiple columns are deleted, other user's selected column is moved only one column to the left. This PR moves the selected column to the left according to the deleted column count. * The cursor position was also causing a crash, fixed. Signed-off-by: Gökay Şatır <gokaysa...@collabora.com> Change-Id: Ib00488f387a91aa0109bcc30feacad52e1b14a30 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158503 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-on: https://gerrit.libreoffice.org/c/core/+/167372 Reviewed-by: Marco Cecchetti <marco.cecche...@collabora.com> diff --git a/sc/source/core/data/markdata.cxx b/sc/source/core/data/markdata.cxx index 54379a554705..afc875983ab1 100644 --- a/sc/source/core/data/markdata.cxx +++ b/sc/source/core/data/markdata.cxx @@ -659,13 +659,11 @@ void ScMarkData::DeleteTab( SCTAB nTab ) void ScMarkData::ShiftCols(const ScDocument& rDoc, SCCOL nStartCol, sal_Int32 nColOffset) { if (bMarked) - { aMarkRange.IncColIfNotLessThan(rDoc, nStartCol, nColOffset); - } - else if (bMultiMarked) + if (bMultiMarked) { - aMultiSel.ShiftCols(nStartCol, nColOffset); aMultiRange.IncColIfNotLessThan(rDoc, nStartCol, nColOffset); + aMultiSel.ShiftCols(nStartCol, nColOffset); } } diff --git a/sc/source/core/data/markmulti.cxx b/sc/source/core/data/markmulti.cxx index 4c92f5f25a47..21a2c9a80d85 100644 --- a/sc/source/core/data/markmulti.cxx +++ b/sc/source/core/data/markmulti.cxx @@ -353,53 +353,18 @@ bool ScMultiSel::HasAnyMarks() const void ScMultiSel::ShiftCols(SCCOL nStartCol, sal_Int32 nColOffset) { - if (nStartCol > mrSheetLimits.mnMaxCol) + if (nStartCol > mrSheetLimits.mnMaxCol || nStartCol >= static_cast<SCCOL>(aMultiSelContainer.size())) return; - ScMultiSel aNewMultiSel(*this); - Clear(); - - if (nColOffset < 0) + if (nColOffset > 0) { - // columns that would be moved on the left of nStartCol must be removed - const SCCOL nEndPos = std::min<SCCOL>(aNewMultiSel.aMultiSelContainer.size(), nStartCol - nColOffset); - for (SCCOL nSearchPos = nStartCol; nSearchPos < nEndPos; ++nSearchPos) - aNewMultiSel.aMultiSelContainer[nSearchPos].Reset(); + aMultiSelContainer.insert(aMultiSelContainer.begin() + nStartCol, nColOffset, ScMarkArray(mrSheetLimits)); } - - SCCOL nCol = 0; - for (const auto& aSourceArray : aNewMultiSel.aMultiSelContainer) + else { - SCCOL nDestCol = nCol; - if (nDestCol >= nStartCol) - { - nDestCol += nColOffset; - if (nDestCol < 0) - nDestCol = 0; - else if (nDestCol > mrSheetLimits.mnMaxCol) - nDestCol = mrSheetLimits.mnMaxCol; - } - if (nDestCol >= static_cast<SCCOL>(aMultiSelContainer.size())) - aMultiSelContainer.resize(nDestCol, ScMarkArray(mrSheetLimits)); - aMultiSelContainer[nDestCol] = aSourceArray; - ++nCol; + sal_Int32 tempOffset = nStartCol - nColOffset >= static_cast<SCCOL>(aMultiSelContainer.size()) ? static_cast<SCCOL>(aMultiSelContainer.size()) - nStartCol -1: -1 * nColOffset; + aMultiSelContainer.erase(aMultiSelContainer.begin() + nStartCol, aMultiSelContainer.begin() + nStartCol + tempOffset); } - aRowSel = aNewMultiSel.aRowSel; - - if (!(nColOffset > 0 && nStartCol > 0 && o3tl::make_unsigned(nStartCol) < aNewMultiSel.aMultiSelContainer.size())) - return; - - // insert nColOffset new columns, and select their cells if they are selected - // both in the old column at nStartPos and in the previous column - auto& rPrevPos = aNewMultiSel.aMultiSelContainer[nStartCol - 1]; - auto& rStartPos = aNewMultiSel.aMultiSelContainer[nStartCol]; - auto& rNewCol = aMultiSelContainer[nStartCol]; - rNewCol = rStartPos; - rNewCol.Intersect(rPrevPos); - if (nStartCol + nColOffset >= static_cast<SCCOL>(aNewMultiSel.aMultiSelContainer.size())) - aNewMultiSel.aMultiSelContainer.resize(nStartCol + nColOffset, ScMarkArray(mrSheetLimits)); - for (tools::Long i = 1; i < nColOffset; ++i) - aMultiSelContainer[nStartCol + i] = rNewCol; } void ScMultiSel::ShiftRows(SCROW nStartRow, sal_Int32 nRowOffset) diff --git a/sc/source/core/tool/address.cxx b/sc/source/core/tool/address.cxx index 7b0c848a2b16..ba6f73f47bec 100644 --- a/sc/source/core/tool/address.cxx +++ b/sc/source/core/tool/address.cxx @@ -2372,17 +2372,30 @@ bool ScRange::MoveSticky( const ScDocument& rDoc, SCCOL dx, SCROW dy, SCTAB dz, void ScRange::IncColIfNotLessThan(const ScDocument& rDoc, SCCOL nStartCol, SCCOL nOffset) { - if (aStart.Col() >= nStartCol) + SCCOL offset; + if (aStart.Col() > nStartCol) { - aStart.IncCol(nOffset); + offset = nOffset; + if (nStartCol + nOffset > aStart.Col()) + offset = aStart.Col() - nStartCol; + else if (nStartCol - nOffset > aStart.Col()) + offset = -1 * (aStart.Col() - nStartCol); + + aStart.IncCol(offset); if (aStart.Col() < 0) aStart.SetCol(0); else if(aStart.Col() > rDoc.MaxCol()) aStart.SetCol(rDoc.MaxCol()); } - if (aEnd.Col() >= nStartCol) + if (aEnd.Col() > nStartCol) { - aEnd.IncCol(nOffset); + offset = nOffset; + if (nStartCol + nOffset > aEnd.Col()) + offset = aEnd.Col() - nStartCol; + else if (nStartCol - nOffset > aEnd.Col()) + offset = -1 * (aEnd.Col() - nStartCol); + + aEnd.IncCol(offset); if (aEnd.Col() < 0) aEnd.SetCol(0); else if(aEnd.Col() > rDoc.MaxCol()) diff --git a/sc/source/ui/docshell/docfunc.cxx b/sc/source/ui/docshell/docfunc.cxx index 72739573afe0..aa8a5d64d7fe 100644 --- a/sc/source/ui/docshell/docfunc.cxx +++ b/sc/source/ui/docshell/docfunc.cxx @@ -2275,7 +2275,7 @@ bool ScDocFunc::InsertCells( const ScRange& rRange, const ScMarkData* pTabMark, if (bInsertCols) { - pViewSh->OnLOKInsertDeleteColumn(rRange.aStart.Col(), 1); + pViewSh->OnLOKInsertDeleteColumn(rRange.aStart.Col() - (eCmd == INS_INSCOLS_BEFORE ? 1: 0), 1); } if (bInsertRows) @@ -2856,7 +2856,7 @@ bool ScDocFunc::DeleteCells( const ScRange& rRange, const ScMarkData* pTabMark, { if (eCmd == DelCellCmd::Cols) { - pViewSh->OnLOKInsertDeleteColumn(rRange.aStart.Col(), -1); + pViewSh->OnLOKInsertDeleteColumn(rRange.aStart.Col(), -1 * (rRange.aEnd.Col() - rRange.aStart.Col() + 1)); } if (eCmd == DelCellCmd::Rows) { diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index 88261c5e561a..8d77023cbaa9 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -1716,11 +1716,17 @@ void ScViewFunc::OnLOKInsertDeleteColumn(SCCOL nStartCol, tools::Long nOffset) if (pTabViewShell->getPart() == nCurrentTabIndex) { SCCOL nX = pTabViewShell->GetViewData().GetCurX(); - if (nX > nStartCol || (nX == nStartCol && nOffset > 0)) + if (nX > nStartCol) { + tools::Long offset = nOffset; + if (nOffset + nStartCol > nX) + offset = nX - nStartCol; + else if (nOffset < 0 && nStartCol - nOffset > nX) + offset = -1 * (nX - nStartCol); + ScInputHandler* pInputHdl = pTabViewShell->GetInputHandler(); SCROW nY = pTabViewShell->GetViewData().GetCurY(); - pTabViewShell->SetCursor(nX + nOffset, nY); + pTabViewShell->SetCursor(nX + offset, nY); if (pInputHdl && pInputHdl->IsInputMode()) { pInputHdl->SetModified(); @@ -1729,8 +1735,8 @@ void ScViewFunc::OnLOKInsertDeleteColumn(SCCOL nStartCol, tools::Long nOffset) ScMarkData aMultiMark( pTabViewShell->GetViewData().GetMarkData() ); aMultiMark.SetMarking( false ); - aMultiMark.MarkToMulti(); - if (aMultiMark.IsMultiMarked()) + + if (aMultiMark.IsMultiMarked() || aMultiMark.IsMarked()) { aMultiMark.ShiftCols(pTabViewShell->GetViewData().GetDocument(), nStartCol, nOffset); pTabViewShell->SetMarkData(aMultiMark);