sc/qa/unit/tiledrendering/SheetViewTest.cxx | 212 ++++++++++++++++++++ sc/source/ui/docshell/SheetViewOperationsTester.cxx | 11 - sc/source/ui/inc/SheetViewOperationsTester.hxx | 2 sc/source/ui/operation/ApplyAttributesOperation.cxx | 18 + sc/source/ui/operation/Operation.cxx | 84 +++++-- sc/source/ui/view/viewfunc.cxx | 22 +- 6 files changed, 307 insertions(+), 42 deletions(-)
New commits: commit 2a859b924f2b503371b49f20dea9eaec3a78e21b Author: Tomaž Vajngerl <[email protected]> AuthorDate: Fri Feb 13 21:50:20 2026 +0900 Commit: Miklos Vajna <[email protected]> CommitDate: Tue Feb 17 08:40:42 2026 +0100 sc: Add sync for apply attribute operations Add sync for the apply attribute operations. When enabling sync and testing that, a couple of issues popped up with address and mark conversion. For address and mark conversion it is important to change always change the tab to be the one from default view, which is now done in consistently. It is also needed to take into account the all the combinations of sheet view and default view being sorted or not sorted, so depending on that we reverse the appropriate sorting. This change adds tests, which should now check all the scenarios when setting the attributes, but those cases are also applicable for other operations. One issue that also popped up is that the mark in ViewData does not use the SCTAB from sheet view, but from the default view. This became a problem when running ScDocument::GetSelectionPattern, which uses ScMarkData and not ScAddress as an input parameter. In this case the returned pattern was wrong as it was from default view instead of sheet view. To solve this it was necessary to convert the mark to use the correct SCTAB value. This was done in ScViewFunc::GetSelectionPattern but maybe it makes sense to do it in a more general place. Change-Id: I3d0c1462aae829c6387dc46f13cd2753f86de0e0 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/199344 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Miklos Vajna <[email protected]> diff --git a/sc/qa/unit/tiledrendering/SheetViewTest.cxx b/sc/qa/unit/tiledrendering/SheetViewTest.cxx index b7f412882343..b5f8d2eeb1fb 100644 --- a/sc/qa/unit/tiledrendering/SheetViewTest.cxx +++ b/sc/qa/unit/tiledrendering/SheetViewTest.cxx @@ -88,6 +88,47 @@ protected: return aString; } + static OUString getTextWeight(ScDocument* pDocument, SCCOL nCol, SCROW nStartRow, SCROW nEndRow, + SCTAB nTab) + { + OUString aString; + + size_t nSize = nEndRow - nStartRow + 1; + + bool bFirst = true; + vcl::Font aFont; + const ScPatternAttr* pPattern; + for (size_t nIndex = 0; nIndex < nSize; nIndex++) + { + pPattern = pDocument->GetPattern(nCol, SCROW(nStartRow + nIndex), nTab); + pPattern->fillFontOnly(aFont); + OUString aWeight; + + switch (aFont.GetWeight()) + { + case WEIGHT_NORMAL: + aWeight = "N"; + break; + case WEIGHT_BOLD: + aWeight = "B"; + break; + default: + aWeight = "?"; + break; + } + + if (bFirst) + { + bFirst = false; + aString = u"\""_ustr + aWeight + u"\""_ustr; + } + else + aString += u", \""_ustr + aWeight + u"\""_ustr; + } + + return aString; + } + void gotoCell(std::u16string_view aCellAddress) { dispatchCommand( @@ -111,6 +152,12 @@ protected: gotoCell(aCellAddress); dispatchCommand(mxComponent, u".uno:SortDescending"_ustr, {}); } + + void setCellBold(std::u16string_view aCellAddress) + { + gotoCell(aCellAddress); + dispatchCommand(mxComponent, u".uno:Bold"_ustr, {}); + } }; /** Test class that contains methods commonly used for testing sync of sheet views. */ @@ -1153,6 +1200,171 @@ CPPUNIT_TEST_FIXTURE(SyncTest, testSyncAfterSorting_SortInDefaultAndSheetView_Fo } } +CPPUNIT_TEST_FIXTURE(SyncTest, testSyncAfterSorting_SortInDefaultView_Attributes) +{ + // Instead of the number, we set the attribute + ScModelObj* pModelObj = createDoc("SheetView_AutoFilter.ods"); + pModelObj->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>()); + ScDocument* pDocument = pModelObj->GetDocument(); + + setupViews(); + + // Create new sheet view + { + switchToSheetView(); + createNewSheetViewInCurrentView(); + } + + // Sort autofilter descending in default view + { + switchToDefaultView(); + sortDescendingForCell(u"A1"); + } + + // Switch to Sheet view and set "bold" text attribute to all cells in the auto filter one by one + { + switchToSheetView(); + + // Current state default view + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"7", u"5", u"4", u"3" }), + getValues(mpTabViewDefaultView, 0, 1, 4)); + + // Current state sheet view + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"4", u"5", u"3", u"7" }), + getValues(mpTabViewSheetView, 0, 1, 4)); + + // Current font weight state + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A4 in sheet view -> A5 in default view + setCellBold(u"A4"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"N", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"B", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A2 in sheet view -> A4 in default view + setCellBold(u"A2"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"N", u"B", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A3 in sheet view -> A3 in default view + setCellBold(u"A3"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"B", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"B", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A5 in sheet view -> A2 in default view + setCellBold(u"A5"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set range A4:A5, which are A2, A5 in default view + setCellBold(u"A4:A5"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"B", u"B", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + } +} + +CPPUNIT_TEST_FIXTURE(SyncTest, testSyncAfterSorting_SortInDefaultAndSheetView_Attributes) +{ + // Instead of the number, we set the attribute + ScModelObj* pModelObj = createDoc("SheetView_AutoFilter.ods"); + pModelObj->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>()); + ScDocument* pDocument = pModelObj->GetDocument(); + + setupViews(); + + // Create new sheet view and sort autofilter ascending + { + switchToSheetView(); + createNewSheetViewInCurrentView(); + sortAscendingForCell(u"A1"); + } + + // Sort autofilter descending in default view + { + switchToDefaultView(); + sortDescendingForCell(u"A1"); + } + + // Switch to Sheet view and set "bold" text attribute to all cells in the auto filter one by one + { + switchToSheetView(); + + // Current state default view + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"7", u"5", u"4", u"3" }), + getValues(mpTabViewDefaultView, 0, 1, 4)); + + // Current state sheet view + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"3", u"4", u"5", u"7" }), + getValues(mpTabViewSheetView, 0, 1, 4)); + + // Current font weight state + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A5 in sheet view -> A2 in default view + setCellBold(u"A5"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"N", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"N", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A4 in sheet view -> A3 in default view + setCellBold(u"A4"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"N", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"N", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A3 in sheet view -> A4 in default view + setCellBold(u"A3"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"B", u"N" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"N", u"B", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + + // Set A2 in sheet view -> A4 in default view + setCellBold(u"A2"); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 0)); + + CPPUNIT_ASSERT_EQUAL(expectedValues({ u"B", u"B", u"B", u"B" }), + getTextWeight(pDocument, 0, 1, 4, 1)); + } +} + CPPUNIT_TEST_FIXTURE(SyncTest, testSync_DefaultView_DeleteCellOperation) { // Create two views, and leave the second one current. diff --git a/sc/source/ui/docshell/SheetViewOperationsTester.cxx b/sc/source/ui/docshell/SheetViewOperationsTester.cxx index 5740018ef0b8..599d28f1d0e8 100644 --- a/sc/source/ui/docshell/SheetViewOperationsTester.cxx +++ b/sc/source/ui/docshell/SheetViewOperationsTester.cxx @@ -29,6 +29,10 @@ constexpr std::string_view getOperationName(OperationType eOperation) return "Unknown"; case OperationType::ApplyAttributes: return "ApplyAttributes"; + case OperationType::ApplyAttributesWithChangedRange: + return "ApplyAttributesWithChangedRange"; + case OperationType::ApplyAttributesToCell: + return "ApplyAttributesToCell"; case OperationType::DeleteContent: return "DeleteContent"; case OperationType::DeleteCell: @@ -155,21 +159,16 @@ bool SheetViewOperationsTester::doesUnsync(OperationType /*eOperationType*/) { r void SheetViewOperationsTester::sync() { if (!mpViewData) - { return; - } auto& rDocument = mpViewData->GetDocument(); SCTAB nTab = mpViewData->GetTabNumber(); if (rDocument.IsSheetViewHolder(nTab)) - { return; - } std::shared_ptr<sc::SheetViewManager> pManager = rDocument.GetSheetViewManager(nTab); - - if (pManager->isEmpty()) + if (!pManager || pManager->isEmpty()) return; for (std::shared_ptr<SheetView> const& pSheetView : pManager->getSheetViews()) diff --git a/sc/source/ui/inc/SheetViewOperationsTester.hxx b/sc/source/ui/inc/SheetViewOperationsTester.hxx index f1c882f9bae3..250a6fbf2a9f 100644 --- a/sc/source/ui/inc/SheetViewOperationsTester.hxx +++ b/sc/source/ui/inc/SheetViewOperationsTester.hxx @@ -20,6 +20,8 @@ enum class OperationType { Unknown, ApplyAttributes, + ApplyAttributesWithChangedRange, + ApplyAttributesToCell, DeleteContent, DeleteCell, TransliterateText, diff --git a/sc/source/ui/operation/ApplyAttributesOperation.cxx b/sc/source/ui/operation/ApplyAttributesOperation.cxx index 7887cdbe11c2..c79d221ea8e8 100644 --- a/sc/source/ui/operation/ApplyAttributesOperation.cxx +++ b/sc/source/ui/operation/ApplyAttributesOperation.cxx @@ -42,7 +42,7 @@ bool ApplyAttributesOperation::runImplementation() if (!rDoc.IsUndoEnabled()) bRecord = false; - ScMarkData aMark = mrMark; + ScMarkData aMark = convertMark(mrMark); bool bImportingXML = rDoc.IsImportingXML(); bool bImportingXLSX = rDoc.IsImportingXLSX(); @@ -57,6 +57,7 @@ bool ApplyAttributesOperation::runImplementation() return false; } + sc::SheetViewOperationsTester aSheetViewTester(ScDocShell::GetViewData()); ScDocShellModificator aModificator(mrDocShell); //! Border @@ -98,6 +99,7 @@ bool ApplyAttributesOperation::runImplementation() else if (nExtFlags & SC_PF_LINES) ScDocFunc::PaintAbove(mrDocShell, aMultiRange); // because of lines above the range + aSheetViewTester.sync(); aModificator.SetDocumentModified(); } @@ -107,7 +109,7 @@ bool ApplyAttributesOperation::runImplementation() ApplyAttributesWithChangedRangeOperation::ApplyAttributesWithChangedRangeOperation( ScDocShell& rDocShell, const ScMarkData& rMark, bool bMultiMarked, const ScPatternAttr& rPattern, sal_uInt16 nExtFlags, bool bApi) - : Operation(OperationType::ApplyAttributes, true, bApi) + : Operation(OperationType::ApplyAttributesWithChangedRange, true, bApi) , mrDocShell(rDocShell) , mrMark(rMark) , mrPattern(rPattern) @@ -128,8 +130,9 @@ bool ApplyAttributesWithChangedRangeOperation::runImplementation() if (!rDoc.IsUndoEnabled()) bRecord = false; - ScMarkData aMark = mrMark; + ScMarkData aMark = convertMark(mrMark); + sc::SheetViewOperationsTester aSheetViewTester(ScDocShell::GetViewData()); ScDocShellModificator aModificator(mrDocShell); const ScRange& aMarkRange = aMark.GetMultiMarkArea(); @@ -175,6 +178,8 @@ bool ApplyAttributesWithChangedRangeOperation::runImplementation() rDoc.ApplySelectionPattern(mrPattern, aMark, pEditDataArray); + aSheetViewTester.sync(); + mrDocShell.PostPaint(nStartCol, nStartRow, nStartTab, nEndCol, nEndRow, nEndTab, PaintPartFlags::Grid, mnExtFlags | SC_PF_TESTMERGE); mrDocShell.UpdateOle(*pViewData); @@ -188,7 +193,7 @@ ApplyAttributesToCellOperation::ApplyAttributesToCellOperation(ScDocShell& rDocS ScAddress const& rPosition, const ScPatternAttr& rPattern, sal_uInt16 nExtFlags, bool bApi) - : Operation(OperationType::ApplyAttributes, true, bApi) + : Operation(OperationType::ApplyAttributesToCell, true, bApi) , mrDocShell(rDocShell) , mrPosition(rPosition) , mrPattern(rPattern) @@ -207,8 +212,9 @@ bool ApplyAttributesToCellOperation::runImplementation() if (!rDoc.IsUndoEnabled()) bRecord = false; - ScAddress aPosition = mrPosition; + ScAddress aPosition = convertAddress(mrPosition); + sc::SheetViewOperationsTester aSheetViewTester(ScDocShell::GetViewData()); ScDocShellModificator aModificator(mrDocShell); SCCOL nCol = aPosition.Col(); @@ -244,6 +250,8 @@ bool ApplyAttributesToCellOperation::runImplementation() } pOldPat.reset(); // is copied in undo (Pool) + aSheetViewTester.sync(); + mrDocShell.PostPaint(nCol, nRow, nTab, nCol, nRow, nTab, PaintPartFlags::Grid, mnExtFlags | SC_PF_TESTMERGE); mrDocShell.UpdateOle(*pViewData); diff --git a/sc/source/ui/operation/Operation.cxx b/sc/source/ui/operation/Operation.cxx index 9abbf62e862e..3995010d2ef7 100644 --- a/sc/source/ui/operation/Operation.cxx +++ b/sc/source/ui/operation/Operation.cxx @@ -44,29 +44,28 @@ ScAddress Operation::convertAddress(ScAddress const& rAddress) ScViewData* pViewData = ScDocShell::GetViewData(); std::shared_ptr<SheetView> pSheetView = getCurrentSheetView(pViewData); + + // We get a valid pSheetView if we currently are in a sheet view, otherwise we don't need to convert if (!pSheetView) return rAddress; ScAddress aAddress = rAddress; - SCTAB nTab = aAddress.Tab(); // Change the tab number if it is the one from sheet view - if (pViewData->CurrentTabForData() == nTab) - nTab = pViewData->GetTabNumber(); - aAddress.SetTab(nTab); + if (aAddress.Tab() == pViewData->CurrentTabForData()) + aAddress.SetTab(pViewData->GetTabNumber()); - std::optional<SortOrderReverser> const& oSortOrder = pSheetView->getSortOrder(); - if (!oSortOrder) + // Should be the default view tab + if (aAddress.Tab() != pViewData->GetTabNumber()) return aAddress; SCCOL nColumn = aAddress.Col(); SCROW nRow = aAddress.Row(); - - if (pViewData->GetTabNumber() != nTab) - return aAddress; + SCTAB nTab = aAddress.Tab(); SCROW nReversedRow = pSheetView->reverseSortingToDefaultView(nRow, nColumn); + // Check if there was a change if (nReversedRow == nRow) return aAddress; @@ -83,10 +82,6 @@ ScMarkData Operation::convertMark(ScMarkData const& rMarkData) if (!pSheetView) return rMarkData; - std::optional<SortOrderReverser> const& oSortOrder = pSheetView->getSortOrder(); - if (!oSortOrder) - return rMarkData; - ScMarkData aNewMark(rMarkData); aNewMark.MarkToMulti(); bool bChanged = false; @@ -111,28 +106,57 @@ ScMarkData Operation::convertMark(ScMarkData const& rMarkData) // Take sorting into account when we convert to default view if (aNewMark.GetTableSelect(nDefaultViewTab)) { - std::vector<std::pair<SCCOL, SCROW>> aMarkedCells; - SortOrderInfo const& rSortInfo = oSortOrder->maSortInfo; - for (SCROW nRow = rSortInfo.mnFirstRow; nRow <= rSortInfo.mnLastRow; ++nRow) + std::optional<SortOrderReverser> const& oSortOrder = pSheetView->getSortOrder(); + std::optional<ReorderParam> const& oReorderParams = pSheetView->getReorderParameters(); + if (oSortOrder || oReorderParams) { - for (SCROW nColumn = rSortInfo.mnFirstColumn; nColumn <= rSortInfo.mnLastColumn; - ++nColumn) + std::vector<std::pair<SCCOL, SCROW>> aMarkedCells; + + SCROW nRowStart = -1; + SCROW nRowEnd = -1; + SCCOL nColumnStart = -1; + SCCOL nColumnEnd = -1; + + if (oSortOrder) { - if (aNewMark.IsCellMarked(nColumn, nRow)) + SortOrderInfo const& rSortInfo = oSortOrder->maSortInfo; + nRowStart = rSortInfo.mnFirstRow; + nRowEnd = rSortInfo.mnLastRow; + nColumnStart = rSortInfo.mnFirstColumn; + nColumnEnd = rSortInfo.mnLastColumn; + } + else + { + ScRange const& aSortRange = oReorderParams->maSortRange; + nRowStart = aSortRange.aStart.Row(); + nRowEnd = aSortRange.aEnd.Row(); + nColumnStart = aSortRange.aStart.Col(); + nColumnEnd = aSortRange.aEnd.Col(); + } + + for (SCROW nRow = nRowStart; nRow <= nRowEnd; ++nRow) + { + for (SCROW nColumn = nColumnStart; nColumn <= nColumnEnd; ++nColumn) { - ScRange aRange(nColumn, nRow, nDefaultViewTab, nColumn, nRow, nDefaultViewTab); - aNewMark.SetMultiMarkArea(aRange, false); - aMarkedCells.emplace_back(nColumn, nRow); + if (aNewMark.IsCellMarked(nColumn, nRow)) + { + // unmark the cell here, but remember which cell it was + ScRange aRange(nColumn, nRow, nDefaultViewTab, nColumn, nRow, + nDefaultViewTab); + aNewMark.SetMultiMarkArea(aRange, false); + aMarkedCells.emplace_back(nColumn, nRow); + } } } - } - for (auto & [ nColumn, nRow ] : aMarkedCells) - { - SCROW nReversedRow = pSheetView->reverseSortingToDefaultView(nRow, nColumn); - ScRange aRange(nColumn, nReversedRow, nDefaultViewTab, nColumn, nReversedRow, - nDefaultViewTab); - aNewMark.SetMultiMarkArea(aRange, true); - bChanged = true; + for (auto & [ nColumn, nRow ] : aMarkedCells) + { + // reverse the row and mark the cell again + SCROW nReversedRow = pSheetView->reverseSortingToDefaultView(nRow, nColumn); + ScRange aRange(nColumn, nReversedRow, nDefaultViewTab, nColumn, nReversedRow, + nDefaultViewTab); + aNewMark.SetMultiMarkArea(aRange, true); + bChanged = true; + } } } diff --git a/sc/source/ui/view/viewfunc.cxx b/sc/source/ui/view/viewfunc.cxx index 7a11c3baf99c..716d36b8a3a1 100644 --- a/sc/source/ui/view/viewfunc.cxx +++ b/sc/source/ui/view/viewfunc.cxx @@ -1096,7 +1096,27 @@ const ScPatternAttr* ScViewFunc::GetSelectionPattern() // copy sheet selection aMark.SetMarkArea( ScRange( nCol, nRow, nTab ) ); - const ScPatternAttr* pAttr = rDoc.GetSelectionPattern( aMark ); + + // We want to get the state of the sheet view, so we need to convert what is marked + if (GetViewData().GetSheetViewID() != sc::DefaultSheetViewID) + { + SCTAB nDefaultViewTab = GetViewData().GetTabNumber(); + SCTAB nSheetViewTab = GetViewData().CurrentTabForData(); + + if (nDefaultViewTab == aMark.GetArea().aStart.Tab() + && nDefaultViewTab == aMark.GetArea().aEnd.Tab()) + { + aMark.SetAreaTab(nSheetViewTab); + } + + if (aMark.GetTableSelect(nDefaultViewTab)) + { + aMark.SelectTable(nDefaultViewTab, false); + aMark.SelectTable(nSheetViewTab, true); + } + } + + const ScPatternAttr* pAttr = rDoc.GetSelectionPattern(aMark); return pAttr; } }
