sw/qa/extras/uiwriter/uiwriter4.cxx | 142 +++++++++++++++++++++++++++++++++++ sw/source/uibase/dochdl/swdtflvr.cxx | 123 ++++++++++++++++++++++++++---- sw/source/uibase/inc/swdtflvr.hxx | 7 - 3 files changed, 255 insertions(+), 17 deletions(-)
New commits: commit 9493147ad6b046a4685896d594ed257800552f67 Author: László Németh <nem...@numbertext.org> AuthorDate: Mon Feb 7 18:03:45 2022 +0100 Commit: László Németh <nem...@numbertext.org> CommitDate: Thu Feb 10 09:36:53 2022 +0100 tdf#147181 tdf#147322 sw: fix drag & drop multiple table rows With change tracking, moving multiple table rows tracked only the first row as deleted, and in the insertion point, as inserted. Without change tracking, only the first row were deleted from the original position. This is a regression of tdf#84806 from commit 5e8aa259e48d5602b932353bb146ebb523982cf2 "tdf#146967 sw table: fix freezing in Hide Changes mode". Add unit tests for the change tracking fix and for the original table moving fix in commit 7fe64353dc9950e19182a59a486a1ecac27cf98e "tdf#84806 Writer: drag and drop selected tables, don't empty". Conflicts: sw/qa/extras/uiwriter/uiwriter4.cxx Change-Id: I43250fcef4bbf482e67a7414f4f655e75d226b55 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129635 Tested-by: László Németh <nem...@numbertext.org> Reviewed-by: László Németh <nem...@numbertext.org> (cherry-picked from commit 2bb9ad2078e355b71ab25db0c46f3d0bb19cf6d4) Change-Id: Id90664ba7611bb0fb7f03bcdf029ae4f211904cb Reviewed-on: https://gerrit.libreoffice.org/c/core/+/129742 Tested-by: Jenkins Reviewed-by: László Németh <nem...@numbertext.org> diff --git a/sw/qa/extras/uiwriter/uiwriter4.cxx b/sw/qa/extras/uiwriter/uiwriter4.cxx index 090938240c63..f7ff97ca1f02 100644 --- a/sw/qa/extras/uiwriter/uiwriter4.cxx +++ b/sw/qa/extras/uiwriter/uiwriter4.cxx @@ -103,6 +103,7 @@ #include <com/sun/star/chart2/data/XDataSource.hpp> #include <com/sun/star/document/XEmbeddedObjectSupplier2.hpp> #include <com/sun/star/drawing/XShape.hpp> +#include <com/sun/star/text/XTextTable.hpp> #include <com/sun/star/linguistic2/XLinguProperties.hpp> #include <com/sun/star/text/XTextViewCursorSupplier.hpp> #include <com/sun/star/text/XPageCursor.hpp> @@ -260,6 +261,8 @@ public: void testTdf115013(); void testTdf114536(); void testTdf115065(); + void testTdf84806_MovingMultipleTableRows(); + void testTdf147181_TrackedMovingOfMultipleTableRows(); void testTdf115132(); void testXDrawPagesSupplier(); void testTdf116403(); @@ -382,6 +385,8 @@ public: CPPUNIT_TEST(testTdf115013); CPPUNIT_TEST(testTdf114536); CPPUNIT_TEST(testTdf115065); + CPPUNIT_TEST(testTdf84806_MovingMultipleTableRows); + CPPUNIT_TEST(testTdf147181_TrackedMovingOfMultipleTableRows); CPPUNIT_TEST(testTdf115132); CPPUNIT_TEST(testXDrawPagesSupplier); CPPUNIT_TEST(testTdf116403); @@ -2922,6 +2927,143 @@ void SwUiWriterTest4::testTdf115065() pWrtShell->Copy(*pWrtShell, ptFrom, ptTo); } +void SwUiWriterTest4::testTdf84806_MovingMultipleTableRows() +{ + // Moving of multiple table rows. + // Source table (first one) has two rows; + // destination (second one) has only one row + SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf115065.odt"); + CPPUNIT_ASSERT(pDoc); + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + CPPUNIT_ASSERT(pWrtShell); + + uno::Reference<text::XTextTablesSupplier> xTablesSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xTables(xTablesSupplier->getTextTables(), + uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables->getCount()); + uno::Reference<container::XNameAccess> xTableNames = xTablesSupplier->getTextTables(); + CPPUNIT_ASSERT(xTableNames->hasByName("Table1")); + CPPUNIT_ASSERT(xTableNames->hasByName("Table2")); + uno::Reference<text::XTextTable> xTable1(xTableNames->getByName("Table1"), uno::UNO_QUERY); + uno::Reference<text::XTextTable> xTable2(xTableNames->getByName("Table2"), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTable1->getRows()->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable2->getRows()->getCount()); + + // without redlining + CPPUNIT_ASSERT_MESSAGE("redlining should be off", + !pDoc->getIDocumentRedlineAccess().IsRedlineOn()); + + sw::UndoManager& rUndoManager = pDoc->GetUndoManager(); + + pWrtShell->GotoTable("Table2"); + SwRect aRect = pWrtShell->GetCurrFrame()->getFrameArea(); + // Destination point is the middle of the first cell of second table + Point ptTo(aRect.Left() + aRect.Width() / 2, aRect.Top() + aRect.Height() / 2); + + // Move rows of the first table into the second table + pWrtShell->GotoTable("Table1"); + pWrtShell->SelTable(); + rtl::Reference<SwTransferable> xTransfer = new SwTransferable(*pWrtShell); + xTransfer->PrivateDrop(*pWrtShell, ptTo, /*bMove=*/true, /*bXSelection=*/true); + + // This was 2 tables + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTables->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTable2->getRows()->getCount()); + + // Undo results 2 tables + rUndoManager.Undo(); + uno::Reference<container::XIndexAccess> xTables2(xTablesSupplier->getTextTables(), + uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables2->getCount()); + uno::Reference<text::XTextTable> xTable1b(xTableNames->getByName("Table1"), uno::UNO_QUERY); + uno::Reference<text::XTextTable> xTable2b(xTableNames->getByName("Table2"), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTable1b->getRows()->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable2b->getRows()->getCount()); + + // FIXME assert with Redo() +} + +void SwUiWriterTest4::testTdf147181_TrackedMovingOfMultipleTableRows() +{ + // Tracked moving of multiple table rows. + // Source table (first one) has two rows; + // destination (second one) has only one row + SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "tdf115065.odt"); + CPPUNIT_ASSERT(pDoc); + SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell(); + CPPUNIT_ASSERT(pWrtShell); + + uno::Reference<text::XTextTablesSupplier> xTablesSupplier(mxComponent, uno::UNO_QUERY); + uno::Reference<container::XIndexAccess> xTables(xTablesSupplier->getTextTables(), + uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables->getCount()); + uno::Reference<container::XNameAccess> xTableNames = xTablesSupplier->getTextTables(); + CPPUNIT_ASSERT(xTableNames->hasByName("Table1")); + CPPUNIT_ASSERT(xTableNames->hasByName("Table2")); + uno::Reference<text::XTextTable> xTable1(xTableNames->getByName("Table1"), uno::UNO_QUERY); + uno::Reference<text::XTextTable> xTable2(xTableNames->getByName("Table2"), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTable1->getRows()->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable2->getRows()->getCount()); + + // FIXME: doesn't work with empty rows, yet + pWrtShell->Insert("x"); + pWrtShell->Down(false); + pWrtShell->Insert("x"); + + // enable redlining + dispatchCommand(mxComponent, ".uno:TrackChanges", {}); + CPPUNIT_ASSERT_MESSAGE("redlining should be on", + pDoc->getIDocumentRedlineAccess().IsRedlineOn()); + + // show changes + CPPUNIT_ASSERT_MESSAGE( + "redlines should be visible", + IDocumentRedlineAccess::IsShowChanges(pDoc->getIDocumentRedlineAccess().GetRedlineFlags())); + + sw::UndoManager& rUndoManager = pDoc->GetUndoManager(); + + pWrtShell->GotoTable("Table2"); + SwRect aRect = pWrtShell->GetCurrFrame()->getFrameArea(); + // Destination point is the middle of the first cell of second table + Point ptTo(aRect.Left() + aRect.Width() / 2, aRect.Top() + aRect.Height() / 2); + + // Move rows of the first table into the second table + pWrtShell->GotoTable("Table1"); + pWrtShell->SelTable(); + rtl::Reference<SwTransferable> xTransfer = new SwTransferable(*pWrtShell); + xTransfer->PrivateDrop(*pWrtShell, ptTo, /*bMove=*/true, /*bXSelection=*/true); + + // still 2 tables, but the second one has got 3 rows + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTable1->getRows()->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTable2->getRows()->getCount()); + + // accept changes results 1 table (removing moved table) + dispatchCommand(mxComponent, ".uno:AcceptAllTrackedChanges", {}); + Scheduler::ProcessEventsToIdle(); + uno::Reference<container::XIndexAccess> xTables2(xTablesSupplier->getTextTables(), + uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTables2->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTable2->getRows()->getCount()); + + // Undo results 2 tables + rUndoManager.Undo(); + rUndoManager.Undo(); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables2->getCount()); + uno::Reference<text::XTextTable> xTable1b(xTableNames->getByName("Table1"), uno::UNO_QUERY); + uno::Reference<text::XTextTable> xTable2b(xTableNames->getByName("Table2"), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTable1b->getRows()->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable2b->getRows()->getCount()); + + // reject changes results 2 table again, with the original row counts + dispatchCommand(mxComponent, ".uno:RejectAllTrackedChanges", {}); + uno::Reference<container::XIndexAccess> xTables3(xTablesSupplier->getTextTables(), + uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables3->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTable1b->getRows()->getCount()); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTable2b->getRows()->getCount()); +} + void SwUiWriterTest4::testTdf115132() { SwDoc* pDoc = createSwDoc(); diff --git a/sw/source/uibase/dochdl/swdtflvr.cxx b/sw/source/uibase/dochdl/swdtflvr.cxx index ddc47cda43e4..427d7aecbd00 100644 --- a/sw/source/uibase/dochdl/swdtflvr.cxx +++ b/sw/source/uibase/dochdl/swdtflvr.cxx @@ -3937,10 +3937,23 @@ bool SwTransferable::PrivateDrop( SwWrtShell& rSh, const Point& rDragPt, { bool bTableCol(SelectionType::TableCol & nSelection); - ::sw::mark::IMark* pMarkMoveFrom = rSh.SetBookmark( + ::sw::mark::IMark* pMarkMoveFrom = bMove + ? rSh.SetBookmark( vcl::KeyCode(), OUString(), - IDocumentMarkAccess::MarkType::UNO_BOOKMARK ); + IDocumentMarkAccess::MarkType::UNO_BOOKMARK ) + : nullptr; + + // row count and direction of the table selection: + // up to down, if the cursor is there in its last table row + const SwSelBoxes& rBoxes = rSrcSh.GetTableCursor()->GetSelectedBoxes(); + const SwTableNode* pTableNd = rSh.IsCursorInTable(); + sal_Int32 nSelRows = !rBoxes.back() + ? 0 + : pTableNd->GetTable().GetTabLines().GetPos( rBoxes.back()->GetUpper() ) - + pTableNd->GetTable().GetTabLines().GetPos( rBoxes.front()->GetUpper() ) + 1; + bool bSelUpToDown = rBoxes.back() && rBoxes.back()->GetUpper() == + rSh.GetCursor()->GetNode().GetTableBox()->GetUpper(); SwUndoId eUndoId = bMove ? SwUndoId::UI_DRAG_AND_MOVE : SwUndoId::UI_DRAG_AND_COPY; @@ -3962,6 +3975,8 @@ bool SwTransferable::PrivateDrop( SwWrtShell& rSh, const Point& rDragPt, rSh.EnterStdMode(); rSh.SwCursorShell::SetCursor(rDragPt, false); + bool bPasteIntoTable = rSh.GetCursor()->GetNode().GetTableBox() != nullptr; + // store cursor ::sw::mark::IMark* pMark = rSh.SetBookmark( vcl::KeyCode(), @@ -3971,23 +3986,103 @@ bool SwTransferable::PrivateDrop( SwWrtShell& rSh, const Point& rDragPt, // paste rows above/columns before pDispatch->Execute(bTableCol ? FN_TABLE_PASTE_COL_BEFORE : FN_TABLE_PASTE_ROW_BEFORE, SfxCallMode::SYNCHRON); - // go to the previously inserted table row and set it to tracked insertion - rSh.Up(false); - SvxPrintItem aTracked(RES_PRINT, false); - rSh.GetDoc()->SetRowNotTracked( *rSh.GetCursor(), aTracked ); + // go to the previously inserted table rows and set them to tracked insertion, if needed + bool bNeedTrack = !bTableCol && rSh.getIDocumentRedlineAccess().IsRedlineOn(); - rSrcSh.Pop(SwCursorShell::PopMode::DeleteCurrent); // restore selection... + // restore cursor position + if (bNeedTrack && pMark != nullptr) + rSh.GotoMark( pMark ); - // delete source rows/columns - if (bMove) + if ( !bNeedTrack && !bPasteIntoTable ) { - // restore cursor position - if (pMarkMoveFrom != nullptr) + rSrcSh.Pop(SwCursorShell::PopMode::DeleteCurrent); // restore selection... + + // delete source rows/columns + if (bMove) + pDispatch->Execute(bTableCol + ? FN_TABLE_DELETE_COL + : FN_TABLE_DELETE_ROW, SfxCallMode::SYNCHRON); + } + else + { + const SwTableBox* pBoxStt = rSh.GetCursor()->GetNode().GetTableBox(); + SwTableLine* pLine = pBoxStt ? const_cast<SwTableLine*>( pBoxStt->GetUpper()): nullptr; + + for (sal_Int32 nDeleted = 0; bNeedTrack && nDeleted < nSelRows;) + { + // move up text cursor (note: "true" is important for the layout level) + if ( !rSh.Up(false) ) + break; + + const SwTableBox* pBox = rSh.GetCursor()->GetNode().GetTableBox(); + + if ( !pBox ) + break; + + // Up() reaches a new row + if ( pBox->GetUpper() != pLine ) + { + //rSh.SelTableRow(); + SvxPrintItem aTracked(RES_PRINT, false); + rSh.GetDoc()->SetRowNotTracked( *rSh.GetCursor(), aTracked ); + ++nDeleted; + pLine = const_cast<SwTableLine*>(pBox->GetUpper()); + } + } + + rSrcSh.Pop(SwCursorShell::PopMode::DeleteCurrent); // restore selection... + + // delete source rows/columns + if (bMove) { - rSh.GotoMark( pMarkMoveFrom ); - rSh.getIDocumentMarkAccess()->deleteMark( pMarkMoveFrom ); + // restore cursor position + if (pMarkMoveFrom != nullptr) + { + rSh.GotoMark( pMarkMoveFrom ); + rSh.getIDocumentMarkAccess()->deleteMark( pMarkMoveFrom ); + } + + // set all row as tracked deletion, otherwise go to the first moved row + if ( bNeedTrack || ( bSelUpToDown && nSelRows > 1 ) ) + { + pLine = nullptr; + + for (sal_Int32 nDeleted = 0; nDeleted < nSelRows - int(!bNeedTrack);) + { + const SwTableBox* pBox = rSh.GetCursor()->GetNode().GetTableBox(); + + if ( !pBox ) + break; + + if ( pBox->GetUpper() != pLine ) + { + pLine = const_cast<SwTableLine*>(pBox->GetUpper()); + if (bNeedTrack) + pDispatch->Execute(bTableCol + ? FN_TABLE_DELETE_COL + : FN_TABLE_DELETE_ROW, SfxCallMode::SYNCHRON); + ++nDeleted; + } + + bool bMoved = false; + if (bSelUpToDown) + bMoved = rSh.Up(false); + else + bMoved = rSh.Down(false); + if (!bMoved) + break; + } + } + + // delete rows without track changes + if ( !bNeedTrack ) + { + for (sal_Int32 nDeleted = 0; nDeleted < nSelRows; ++nDeleted) + pDispatch->Execute(bTableCol + ? FN_TABLE_DELETE_COL + : FN_TABLE_DELETE_ROW, SfxCallMode::SYNCHRON); + } } - pDispatch->Execute(bTableCol ? FN_TABLE_DELETE_COL : FN_TABLE_DELETE_ROW, SfxCallMode::SYNCHRON); } // restore cursor position diff --git a/sw/source/uibase/inc/swdtflvr.hxx b/sw/source/uibase/inc/swdtflvr.hxx index ce3cfa271253..b165cd7b6b4a 100644 --- a/sw/source/uibase/inc/swdtflvr.hxx +++ b/sw/source/uibase/inc/swdtflvr.hxx @@ -145,9 +145,6 @@ class SW_DLLPUBLIC SwTransferable final : public TransferableHelper SwWrtShell& rSh, bool bLink, const Point* pPt, bool bMsg ); - bool PrivateDrop( SwWrtShell& rSh, const Point& rDragPt, bool bMove, - bool bIsXSelection ); - bool PrivatePaste( SwWrtShell& rShell, SwPasteContext* pContext = nullptr, PasteTableType ePasteTable = PasteTableType::PASTE_DEFAULT ); void SetDataForDragAndDrop( const Point& rSttPos ); @@ -226,6 +223,10 @@ public: SwWrtShell* GetShell() { return m_pWrtShell; } void SetCleanUp( bool bFlag ) { m_bCleanUp = bFlag; } + // public only for testing + bool PrivateDrop( SwWrtShell& rSh, const Point& rDragPt, bool bMove, + bool bIsXSelection ); + // Interfaces for Selection /* #96392# Added pCreator to distinguish SwFrameShell from SwWrtShell. */ static void CreateSelection( SwWrtShell & rSh,