sw/qa/extras/uiwriter/uiwriter4.cxx | 138 +++++++++++++++++++++++++++++++++++ sw/source/uibase/dochdl/swdtflvr.cxx | 123 +++++++++++++++++++++++++++---- sw/source/uibase/inc/swdtflvr.hxx | 7 + 3 files changed, 251 insertions(+), 17 deletions(-)
New commits: commit 2bb9ad2078e355b71ab25db0c46f3d0bb19cf6d4 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: Wed Feb 9 21:18:28 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". 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> diff --git a/sw/qa/extras/uiwriter/uiwriter4.cxx b/sw/qa/extras/uiwriter/uiwriter4.cxx index 29d31a0940b2..e52318062a7d 100644 --- a/sw/qa/extras/uiwriter/uiwriter4.cxx +++ b/sw/qa/extras/uiwriter/uiwriter4.cxx @@ -39,6 +39,7 @@ #include <tblafmt.hxx> #include <com/sun/star/text/XTextField.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> @@ -2725,6 +2726,143 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest4, testTdf115065) pWrtShell->Copy(*pWrtShell, ptFrom, ptTo); } +CPPUNIT_TEST_FIXTURE(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() +} + +CPPUNIT_TEST_FIXTURE(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()); +} + CPPUNIT_TEST_FIXTURE(SwUiWriterTest4, testTdf115132) { SwDoc* pDoc = createSwDoc(); diff --git a/sw/source/uibase/dochdl/swdtflvr.cxx b/sw/source/uibase/dochdl/swdtflvr.cxx index def9704fd444..56b828187d67 100644 --- a/sw/source/uibase/dochdl/swdtflvr.cxx +++ b/sw/source/uibase/dochdl/swdtflvr.cxx @@ -3930,10 +3930,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; @@ -3955,6 +3968,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(), @@ -3964,23 +3979,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,