sc/qa/unit/uicalc/uicalc.cxx | 29 +++ sc/source/ui/view/tabview2.cxx | 375 +++++++++++++++++++++++------------------ 2 files changed, 247 insertions(+), 157 deletions(-)
New commits: commit c5a98bb91b4994b2197a104bf209f0d77596ec16 Author: Jaume Pujantell <jaume.pujant...@collabora.com> AuthorDate: Thu Jul 6 09:22:15 2023 +0200 Commit: Jaume Pujantell <jaume.pujant...@collabora.com> CommitDate: Fri Jul 7 09:00:01 2023 +0200 tdf#155796 sc: fix select with merged cells When selecting multiple cells or modifying a selection with shift+arrow make sure that a merge group is never partially selected. This also fixes tdf#128678 Change-Id: Ida00939cec11240c0d06375feb21afa82a6876da Reviewed-on: https://gerrit.libreoffice.org/c/core/+/154048 Reviewed-by: Szymon Kłos <szymon.k...@collabora.com> Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Jaume Pujantell <jaume.pujant...@collabora.com> diff --git a/sc/qa/unit/uicalc/uicalc.cxx b/sc/qa/unit/uicalc/uicalc.cxx index 056a34e9b4e8..6f1c490b90f4 100644 --- a/sc/qa/unit/uicalc/uicalc.cxx +++ b/sc/qa/unit/uicalc/uicalc.cxx @@ -1997,6 +1997,35 @@ CPPUNIT_TEST_FIXTURE(ScUiCalcTest, testUnallocatedColumnsAttributes) CPPUNIT_ASSERT_EQUAL_MESSAGE("font should be bold", WEIGHT_BOLD, aFont.GetWeight()); } +CPPUNIT_TEST_FIXTURE(ScUiCalcTest, testTdf155796) +{ + mxComponent = loadFromDesktop("private:factory/scalc"); + ScModelObj* pModelObj = dynamic_cast<ScModelObj*>(mxComponent.get()); + CPPUNIT_ASSERT(pModelObj); + ScDocument* pDoc = pModelObj->GetDocument(); + CPPUNIT_ASSERT(pDoc); + + goToCell("A1:A3"); + dispatchCommand(mxComponent, ".uno:ToggleMergeCells", {}); + goToCell("A4:A6"); + dispatchCommand(mxComponent, ".uno:ToggleMergeCells", {}); + + goToCell("A1:A6"); + + pModelObj->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 0, KEY_SHIFT | KEY_UP); + Scheduler::ProcessEventsToIdle(); + + ScRangeList aMarkedArea = ScDocShell::GetViewData()->GetMarkData().GetMarkedRanges(); + OUString aMarkedAreaString; + ScRangeStringConverter::GetStringFromRangeList(aMarkedAreaString, &aMarkedArea, pDoc, + formula::FormulaGrammar::CONV_OOO); + + // Without the fix in place, this test would have failed with + // - Expected: Sheet1.A1:Sheet1.A3 + // - Actual : Sheet1.A1:Sheet1.A5 + CPPUNIT_ASSERT_EQUAL(OUString("Sheet1.A1:Sheet1.A3"), aMarkedAreaString); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sc/source/ui/view/tabview2.cxx b/sc/source/ui/view/tabview2.cxx index 226023b2914e..6e5ee8527d54 100644 --- a/sc/source/ui/view/tabview2.cxx +++ b/sc/source/ui/view/tabview2.cxx @@ -58,6 +58,19 @@ bool isCellQualified(const ScDocument* pDoc, SCCOL nCol, SCROW nRow, SCTAB nTab, return true; } +bool areCellsQualified(const ScDocument* pDoc, SCCOL nColStart, SCROW nRowStart, SCCOL nColEnd, + SCROW nRowEnd, SCTAB nTab, bool bSelectLocked, bool bSelectUnlocked) +{ + PutInOrder(nColStart, nColEnd); + PutInOrder(nRowStart, nRowEnd); + for (SCCOL col = nColStart; col <= nColEnd; ++col) + for (SCROW row = nRowStart; row <= nRowEnd; ++row) + if (!isCellQualified(pDoc, col, row, nTab, bSelectLocked, bSelectUnlocked)) + return false; + + return true; +} + void moveCursorByProtRule( SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, SCTAB nTab, const ScDocument* pDoc) { @@ -180,13 +193,9 @@ bool checkBoundary(const ScDocument* pDoc, SCCOL& rCol, SCROW& rRow) return bGood; } -void moveCursorByMergedCell( - SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, SCTAB nTab, - const ScDocument* pDoc, const ScViewData& rViewData) +void moveCursorByMergedCell(SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, SCCOL nStartX, + SCROW nStartY, SCTAB nTab, const ScDocument* pDoc) { - SCCOL nOrigX = rViewData.GetCurX(); - SCROW nOrigY = rViewData.GetCurY(); - const ScTableProtection* pTabProtection = pDoc->GetTabProtection(nTab); bool bSelectLocked = true; bool bSelectUnlocked = true; @@ -196,108 +205,198 @@ void moveCursorByMergedCell( bSelectUnlocked = pTabProtection->isOptionEnabled(ScTableProtection::SELECT_UNLOCKED_CELLS); } - const ScMergeAttr* pMergeAttr = pDoc->GetAttr(nOrigX, nOrigY, nTab, ATTR_MERGE); - - bool bOriginMerged = false; - SCCOL nColSpan = 1; - SCROW nRowSpan = 1; - if (pMergeAttr && pMergeAttr->IsMerged()) - { - nColSpan = pMergeAttr->GetColMerge(); - nRowSpan = pMergeAttr->GetRowMerge(); - bOriginMerged = true; - } - if (nMovX > 0) { - SCCOL nOld = rCol; - if (bOriginMerged) - { - // Original cell is merged. Push the block end outside the merged region. - if (nOrigX < pDoc->MaxCol() && nOrigX < rCol && rCol <= nOrigX + nColSpan - 1) - rCol = nOrigX + nColSpan; - } - else - { - pDoc->SkipOverlapped(rCol, rRow, nTab); - } + SCROW rowStart = std::min(rRow, nStartY); + SCROW rowEnd = std::max(rRow, nStartY); - if (nOld < rCol) + for (SCROW i = rowStart; i <= rowEnd && rCol < nStartX;) { - // The block end has moved. Check the protection setting and move back if needed. - checkBoundary(pDoc, rCol, rRow); - if (!isCellQualified(pDoc, rCol, rRow, nTab, bSelectLocked, bSelectUnlocked)) - --rCol; + SCCOL tmpCol = rCol; + while (tmpCol < pDoc->MaxCol() && pDoc->IsHorOverlapped(tmpCol, i, nTab)) + ++tmpCol; + if (tmpCol != rCol) + { + i = rowStart; + if (tmpCol > nStartX) + --tmpCol; + if (!areCellsQualified(pDoc, rCol + 1, rowStart, tmpCol, rowEnd, nTab, + bSelectLocked, bSelectUnlocked)) + break; + rCol = tmpCol; + } + else + ++i; } } - if (nMovX < 0) + else if (nMovX < 0) { - SCCOL nOld = rCol; - if (bOriginMerged) - { - if (nOrigX > 0 && nOrigX <= rCol && rCol < nOrigX + nColSpan - 1) - // Block end is still within the merged region. Push it outside. - rCol = nOrigX - 1; - } - else - { - pDoc->SkipOverlapped(rCol, rRow, nTab); - } + SCROW rowStart = std::min(rRow, nStartY); + SCROW rowEnd = std::max(rRow, nStartY); - if (nOld > rCol) + for (SCROW i = rowStart; i <= rowEnd && rCol > nStartX;) { - // The block end has moved. Check the protection setting and move back if needed. - checkBoundary(pDoc, rCol, rRow); - if (!isCellQualified(pDoc, rCol, rRow, nTab, bSelectLocked, bSelectUnlocked)) - ++rCol; + SCCOL tmpCol = rCol; + while (tmpCol >= 0 && pDoc->IsHorOverlapped(tmpCol + 1, i, nTab)) + --tmpCol; + if (tmpCol != rCol) + { + i = rowStart; + if (tmpCol < nStartX) + ++tmpCol; + if (!areCellsQualified(pDoc, rCol - 1, rowStart, tmpCol, rowEnd, nTab, + bSelectLocked, bSelectUnlocked)) + break; + rCol = tmpCol; + } + else + ++i; } } + if (nMovY > 0) { - SCROW nOld = rRow; - if (bOriginMerged) - { - // Original cell is merged. Push the block end outside the merged region. - if (nOrigY < pDoc->MaxRow() && nOrigY < rRow && rRow <= nOrigY + nRowSpan - 1) - rRow = nOrigY + nRowSpan; - } - else + SCCOL colStart = std::min(rCol, nStartX); + SCCOL colEnd = std::max(rCol, nStartX); + + for (SCCOL i = colStart; i <= colEnd && rRow < nStartY;) { - pDoc->SkipOverlapped(rCol, rRow, nTab); + SCROW tmpRow = rRow; + while (tmpRow < pDoc->MaxRow() && pDoc->IsVerOverlapped(i, tmpRow, nTab)) + ++tmpRow; + if (tmpRow != rRow) + { + i = colStart; + if (tmpRow > nStartY) + --tmpRow; + if (!areCellsQualified(pDoc, colStart, rRow + 1, colEnd, tmpRow, nTab, + bSelectLocked, bSelectUnlocked)) + break; + rRow = tmpRow; + } + else + ++i; } + } + else if (nMovY < 0) + { + SCCOL colStart = std::min(rCol, nStartX); + SCCOL colEnd = std::max(rCol, nStartX); - if (nOld < rRow) + for (SCCOL i = colStart; i <= colEnd && rRow > nStartY;) { - // The block end has moved. Check the protection setting and move back if needed. - checkBoundary(pDoc, rCol, rRow); - if (!isCellQualified(pDoc, rCol, rRow, nTab, bSelectLocked, bSelectUnlocked)) - --rRow; + SCROW tmpRow = rRow; + while (tmpRow >= 0 && pDoc->IsVerOverlapped(i, tmpRow + 1, nTab)) + --tmpRow; + if (tmpRow != rRow) + { + i = colStart; + if (tmpRow < nStartY) + ++tmpRow; + if (!areCellsQualified(pDoc, colStart, rRow - 1, colEnd, tmpRow, nTab, + bSelectLocked, bSelectUnlocked)) + break; + rRow = tmpRow; + } + else + ++i; } } - if (nMovY >= 0) - return; +} - SCROW nOld = rRow; - if (bOriginMerged) +void moveCursorToProperSide(SCCOL& rCol, SCROW& rRow, SCCOL nMovX, SCROW nMovY, SCCOL nStartX, + SCROW nStartY, SCTAB nTab, const ScDocument* pDoc) +{ + SCCOL tmpCol = rCol; + SCROW tmpRow = rRow; + + if (nMovX > 0 && nStartX < pDoc->MaxCol() && rCol < nStartX) { - if (nOrigY > 0 && nOrigY <= rRow && rRow < nOrigY + nRowSpan - 1) - // Block end is still within the merged region. Push it outside. - rRow = nOrigY - 1; + SCROW rowStart = std::min(rRow, nStartY); + SCROW rowEnd = std::max(rRow, nStartY); + for (SCROW i = rowStart; i <= rowEnd && tmpCol < nStartX;) + { + if (pDoc->IsHorOverlapped(tmpCol + 1, i, nTab)) + { + do + { + ++tmpCol; + } while (pDoc->IsHorOverlapped(tmpCol + 1, i, nTab)); + i = rowStart; + } + else + ++i; + } + if (tmpCol < nStartX) + tmpCol = rCol; } - else + else if (nMovX < 0 && nStartX > 0 && rCol > nStartX) { - pDoc->SkipOverlapped(rCol, rRow, nTab); + SCROW rowStart = std::min(rRow, nStartY); + SCROW rowEnd = std::max(rRow, nStartY); + for (SCROW i = rowStart; i <= rowEnd && tmpCol > nStartX;) + { + if (pDoc->IsHorOverlapped(tmpCol, i, nTab)) + { + do + { + --tmpCol; + } while (pDoc->IsHorOverlapped(tmpCol, i, nTab)); + i = rowStart; + } + else + ++i; + } + if (tmpCol > nStartX) + tmpCol = rCol; } - if (nOld > rRow) + if (nMovY > 0 && nStartY < pDoc->MaxRow() && rRow < nStartY) { - // The block end has moved. Check the protection setting and move back if needed. - checkBoundary(pDoc, rCol, rRow); - if (!isCellQualified(pDoc, rCol, rRow, nTab, bSelectLocked, bSelectUnlocked)) - ++rRow; + SCCOL colStart = std::min(rCol, nStartX); + SCCOL colEnd = std::max(rCol, nStartX); + for (SCCOL i = colStart; i <= colEnd && tmpRow < nStartY;) + { + if (pDoc->IsVerOverlapped(i, tmpRow + 1, nTab)) + { + do + { + ++tmpRow; + } while (pDoc->IsVerOverlapped(i, tmpRow + 1, nTab)); + i = colStart; + } + else + ++i; + } + if (tmpRow < nStartY) + tmpRow = rRow; + } + else if (nMovY < 0 && nStartY > 0 && rRow > nStartY) + { + SCCOL colStart = std::min(rCol, nStartX); + SCCOL colEnd = std::max(rCol, nStartX); + for (SCCOL i = colStart; i <= colEnd && tmpRow > nStartY;) + { + if (pDoc->IsVerOverlapped(i, tmpRow, nTab)) + { + do + { + --tmpRow; + } while (pDoc->IsVerOverlapped(i, tmpRow, nTab)); + i = colStart; + } + else + ++i; + } + if (tmpRow > nStartY) + tmpRow = rRow; } -} + if (tmpCol != rCol) + rCol = tmpCol; + if (tmpRow != rRow) + rRow = tmpRow; +} } void ScTabView::PaintMarks(SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow ) @@ -491,88 +590,39 @@ void ScTabView::MarkCursor( SCCOL nCurX, SCROW nCurY, SCTAB nCurZ, if ( bCellSelection ) { - // Expand selection area accordingly when the current selection ends - // with a merged cell. - SCCOL nCurXOffset = 0; - SCCOL nBlockStartXOffset = 0; - SCROW nCurYOffset = 0; - SCROW nBlockStartYOffset = 0; - bool bBlockStartMerged = false; - - // The following block checks whether or not the "BlockStart" (anchor) - // cell is merged. If it's merged, it'll then move the position of the - // anchor cell to the corner that's diagonally opposite of the - // direction of a current selection area. For instance, if a current - // selection is moving in the upperleft direction, the anchor cell will - // move to the lower-right corner of the merged anchor cell, and so on. - - const ScMergeAttr* pMergeAttr = - rDocument.GetAttr( nBlockStartXOrig, nBlockStartYOrig, nTab, ATTR_MERGE ); - if ( pMergeAttr->IsMerged() ) + // Expand selection area accordingly when the current selection cuts + // through a merged cell. + ScRange cellSel(nBlockStartXOrig, nBlockStartYOrig, nTab, nCurX, nCurY, nTab); + cellSel.PutInOrder(); + ScRange oldSel; + do { - SCCOL nColSpan = pMergeAttr->GetColMerge(); - SCROW nRowSpan = pMergeAttr->GetRowMerge(); + oldSel = cellSel; + rDocument.ExtendOverlapped(cellSel); + rDocument.ExtendMerge(cellSel); + } while (oldSel != cellSel); - if ( nCurX < nBlockStartXOrig + nColSpan - 1 || nCurY < nBlockStartYOrig + nRowSpan - 1 ) - { - nBlockStartX = nCurX >= nBlockStartXOrig ? nBlockStartXOrig : nBlockStartXOrig + nColSpan - 1; - nBlockStartY = nCurY >= nBlockStartYOrig ? nBlockStartYOrig : nBlockStartYOrig + nRowSpan - 1; - nCurXOffset = (nCurX >= nBlockStartXOrig && nCurX < nBlockStartXOrig + nColSpan - 1) ? - nBlockStartXOrig - nCurX + nColSpan - 1 : 0; - nCurYOffset = (nCurY >= nBlockStartYOrig && nCurY < nBlockStartYOrig + nRowSpan - 1) ? - nBlockStartYOrig - nCurY + nRowSpan - 1 : 0; - bBlockStartMerged = true; - } + // Preserve the directionallity of the selection + if (nCurX >= nBlockStartXOrig) + { + nBlockStartX = cellSel.aStart.Col(); + nBlockEndX = cellSel.aEnd.Col(); } - - // The following block checks whether or not the current cell is - // merged. If it is, it'll then set the appropriate X & Y offset - // values (nCurXOffset & nCurYOffset) such that the selection area will - // grow by those specified offset amounts. Note that the values of - // nCurXOffset/nCurYOffset may also be specified in the previous code - // block, in which case whichever value is greater will take on. - - pMergeAttr = rDocument.GetAttr( nCurX, nCurY, nTab, ATTR_MERGE ); - if ( pMergeAttr->IsMerged() ) + else { - SCCOL nColSpan = pMergeAttr->GetColMerge(); - SCROW nRowSpan = pMergeAttr->GetRowMerge(); - - if ( nBlockStartX < nCurX + nColSpan - 1 || nBlockStartY < nCurY + nRowSpan - 1 ) - { - if ( nBlockStartX <= nCurX + nColSpan - 1 ) - { - SCCOL nCurXOffsetTemp = (nCurX < nCurX + nColSpan - 1) ? nColSpan - 1 : 0; - nCurXOffset = std::max(nCurXOffset, nCurXOffsetTemp); - } - if ( nBlockStartY <= nCurY + nRowSpan - 1 ) - { - SCROW nCurYOffsetTemp = (nCurY < nCurY + nRowSpan - 1) ? nRowSpan - 1 : 0; - nCurYOffset = std::max(nCurYOffset, nCurYOffsetTemp); - } - if ( ( nBlockStartX > nCurX || nBlockStartY > nCurY ) && - ( nBlockStartX <= nCurX + nColSpan - 1 || nBlockStartY <= nCurY + nRowSpan - 1 ) ) - { - nBlockStartXOffset = (nBlockStartX > nCurX && nBlockStartX <= nCurX + nColSpan - 1) ? nCurX - nBlockStartX : 0; - nBlockStartYOffset = (nBlockStartY > nCurY && nBlockStartY <= nCurY + nRowSpan - 1) ? nCurY - nBlockStartY : 0; - } - } + nBlockStartX = cellSel.aEnd.Col(); + nBlockEndX = cellSel.aStart.Col(); + } + if (nCurY >= nBlockStartYOrig) + { + nBlockStartY = cellSel.aStart.Row(); + nBlockEndY = cellSel.aEnd.Row(); } else { - // The current cell is not merged. Move the anchor cell to its - // original position. - if ( !bBlockStartMerged ) - { - nBlockStartX = nBlockStartXOrig; - nBlockStartY = nBlockStartYOrig; - } + nBlockStartY = cellSel.aEnd.Row(); + nBlockEndY = cellSel.aStart.Row(); } - - nBlockStartX = nBlockStartX + nBlockStartXOffset >= 0 ? nBlockStartX + nBlockStartXOffset : 0; - nBlockStartY = nBlockStartY + nBlockStartYOffset >= 0 ? nBlockStartY + nBlockStartYOffset : 0; - nBlockEndX = std::min<SCCOL>(nCurX + nCurXOffset, rDocument.MaxCol()); - nBlockEndY = std::min(nCurY + nCurYOffset, rDocument.MaxRow()); } else { @@ -586,8 +636,8 @@ void ScTabView::MarkCursor( SCCOL nCurX, SCROW nCurY, SCTAB nCurZ, UpdateSelectionOverlay(); SelectionChanged(); - nOldCurX = nCurX; - nOldCurY = nCurY; + nOldCurX = nBlockEndX; + nOldCurY = nBlockEndY; aViewData.GetViewShell()->UpdateInputHandler(); } @@ -973,11 +1023,22 @@ void ScTabView::ExpandBlock(SCCOL nMovX, SCROW nMovY, ScFollowMode eMode) // Note that the origin position *never* moves during selection. if (!IsBlockMode()) + { InitBlockMode(nOrigX, nOrigY, nTab, true); + const ScMergeAttr* pMergeAttr = rDoc.GetAttr(nOrigX, nOrigY, nTab, ATTR_MERGE); + if (pMergeAttr && pMergeAttr->IsMerged()) + { + nBlockEndX = nOrigX + pMergeAttr->GetColMerge() - 1; + nBlockEndY = nOrigY + pMergeAttr->GetRowMerge() - 1; + } + } + moveCursorToProperSide(nBlockEndX, nBlockEndY, nMovX, nMovY, nBlockStartX, nBlockStartY, + nTab, &rDoc); moveCursorByProtRule(nBlockEndX, nBlockEndY, nMovX, nMovY, nTab, &rDoc); checkBoundary(&rDoc, nBlockEndX, nBlockEndY); - moveCursorByMergedCell(nBlockEndX, nBlockEndY, nMovX, nMovY, nTab, &rDoc, aViewData); + moveCursorByMergedCell(nBlockEndX, nBlockEndY, nMovX, nMovY, nBlockStartX, nBlockStartY, + nTab, &rDoc); checkBoundary(&rDoc, nBlockEndX, nBlockEndY); MarkCursor(nBlockEndX, nBlockEndY, nTab, false, false, true);