tdf#146622 sw crash fix: don't delete already deleted rows

Delete tables and rows removed also tables and table rows
with tracked row deletion/insertion. This resulted not only
lost change tracking, but a crashing Undo.

Crash regression from commit 99059a1ececa3621c2fe46fabdd79eed9d626c42
"tdf#143359 sw: track deletion of empty table rows".

Non-tracked deletion of the tracked row changes was a
temporary solution for the missing UI of tracked
row changes, implemented in commit 05366b8e6683363688de8708a3d88cf144c7a2bf
"tdf#60382 sw offapi: add change tracking of table/row deletion".

Note: UI was added in commit f348440e17debacbcba9153e238e010e8c020bdc
"tdf#146120 sw: show tracked table changes with different color",
commit 95c003d75e0f8b255344715a35358072b5eba99d
"tdf#146145 sw: 1-click Accept/Reject of table row changes",
commit 84fbb3398f7486f00e7b7dea415e1ea2510a9535
"tdf#146144 sw: add tooltip to table rows with change tracking",
commit eebe4747d2d13545004937bb0267ccfc8ab9d63f
"tdf#144270 sw: manage tracked table (row) deletion/insertion",
commit f481c2c8e74bded11fac754e493560391229dbcd
"tdf#144057 sw track changes: hide deleted table rows" and
commit 23846867ea32667ccf328c36142394dd6aaee8ba
"tdf#147182 sw: accept/reject all changes of a table selection".

(cherry-picked from commit 95213407dfcbf34056037d60243ff915340d1a2e)

Change-Id: I384b750b0d3626fa8b3f256c7eaf5b93f382e4e5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130011
Tested-by: László Németh <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>
(cherry picked from commit 6b80222d6bc043eb78a2986e9051cee3e5ee0967)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/130019
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

4 files changed, 82 insertions, 7 deletions

diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx
index 831c59d2cc05..f7749190e3ea 100644
--- a/sw/inc/doc.hxx
+++ b/sw/inc/doc.hxx
@@ -1491,6 +1491,8 @@ public:
     /// rNotTracked = false means that the row was deleted or inserted with its tracked cell content
     /// bAll: delete all table rows without selection
     void SetRowNotTracked( const SwCursor& rCursor, const SvxPrintItem &rNotTracked, bool bAll = false );
+    /// don't call SetRowNotTracked() for rows with tracked row change
+    static bool HasRowNotTracked( const SwCursor& rCursor );
     void SetTabBorders( const SwCursor& rCursor, const SfxItemSet& rSet );
     void SetTabLineStyle( const SwCursor& rCursor,
                           const Color* pColor, bool bSetLine,
diff --git a/sw/qa/extras/uiwriter/uiwriter3.cxx b/sw/qa/extras/uiwriter/uiwriter3.cxx
index cf8c4e07e9b6..d2123c33b20c 100644
--- a/sw/qa/extras/uiwriter/uiwriter3.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter3.cxx
@@ -1890,6 +1890,59 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest3, testTdf132744)
     CPPUNIT_ASSERT_EQUAL(1, getShapes());
 }
 
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest3, testTdf146622)
+{
+    SwDoc* pDoc = createSwDoc(DATA_DIRECTORY, "TC-table-del-add.docx");
+    CPPUNIT_ASSERT(pDoc);
+    SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+    CPPUNIT_ASSERT(pWrtShell);
+
+    CPPUNIT_ASSERT_MESSAGE("redlining should be on",
+                           pDoc->getIDocumentRedlineAccess().IsRedlineOn());
+
+    uno::Reference<text::XTextTablesSupplier> xTablesSupplier(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XIndexAccess> xTables(xTablesSupplier->getTextTables(),
+                                                    uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTables->getCount());
+    uno::Reference<container::XNameAccess> xTableNames = xTablesSupplier->getTextTables();
+    CPPUNIT_ASSERT(xTableNames->hasByName("Table1"));
+    uno::Reference<text::XTextTable> xTable1(xTableNames->getByName("Table1"), uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), xTable1->getRows()->getCount());
+
+    dispatchCommand(mxComponent, ".uno:DeleteRows", {});
+
+    // This was 3 (deleting the already deleted row with change tracking)
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), xTable1->getRows()->getCount());
+
+    dispatchCommand(mxComponent, ".uno:SelectAll", {});
+    dispatchCommand(mxComponent, ".uno:SelectAll", {});
+    Scheduler::ProcessEventsToIdle();
+
+    dispatchCommand(mxComponent, ".uno:DeleteRows", {});
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), xTable1->getRows()->getCount());
+
+    dispatchCommand(mxComponent, ".uno:SelectAll", {});
+    dispatchCommand(mxComponent, ".uno:SelectAll", {});
+    Scheduler::ProcessEventsToIdle();
+
+    dispatchCommand(mxComponent, ".uno:DeleteRows", {});
+    // This was 2 (deleting the already deleted table with change tracking)
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), xTables->getCount());
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4), xTable1->getRows()->getCount());
+
+    // check that the first table was deleted with change tracking
+    dispatchCommand(mxComponent, ".uno:AcceptAllTrackedChanges", {});
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xTables->getCount());
+
+    // Undo AcceptAllTrackedChanges and DeleteRows
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+
+    // now only the second table deleted by AcceptAllTrackedChanges
+    dispatchCommand(mxComponent, ".uno:AcceptAllTrackedChanges", {});
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), xTables->getCount());
+}
+
 CPPUNIT_TEST_FIXTURE(SwUiWriterTest3, testTdf135014)
 {
     createSwDoc();
diff --git a/sw/source/core/docnode/ndtbl1.cxx b/sw/source/core/docnode/ndtbl1.cxx
index 1cb5fc651b6c..4d0e38b794ab 100644
--- a/sw/source/core/docnode/ndtbl1.cxx
+++ b/sw/source/core/docnode/ndtbl1.cxx
@@ -540,6 +540,28 @@ bool SwDoc::GetRowBackground( const SwCursor& rCursor, std::unique_ptr<SvxBrushI
     return bRet;
 }
 
+bool SwDoc::HasRowNotTracked( const SwCursor& rCursor )
+{
+    SwTableNode* pTableNd = rCursor.GetPoint()->nNode.GetNode().FindTableNode();
+    if( !pTableNd )
+        return false;
+
+    std::vector<SwTableLine*> aRowArr; // For Lines collecting
+    ::lcl_CollectLines( aRowArr, rCursor, true );
+
+    if( aRowArr.empty() )
+        return false;
+
+    for( auto pLn : aRowArr )
+    {
+        auto pHasTextChangesOnlyProp = pLn->GetFrameFormat()->GetAttrSet().GetItem<SvxPrintItem>(RES_PRINT);
+        if ( !pHasTextChangesOnlyProp || pHasTextChangesOnlyProp->GetValue() )
+            // there is a not deleted row in the table selection
+            return true;
+    }
+    return false;
+}
+
 void SwDoc::SetRowNotTracked( const SwCursor& rCursor, const SvxPrintItem &rNew, bool bAll )
 {
     SwTableNode* pTableNd = rCursor.GetPoint()->nNode.GetNode().FindTableNode();
diff --git a/sw/source/core/frmedt/fetab.cxx b/sw/source/core/frmedt/fetab.cxx
index f388067c27c5..ec9968271d3d 100644
--- a/sw/source/core/frmedt/fetab.cxx
+++ b/sw/source/core/frmedt/fetab.cxx
@@ -332,8 +332,11 @@ bool SwFEShell::DeleteRow(bool bCompleteTable)
     // and set IsNoTracked table line property to false
     if ( GetDoc()->GetDocShell()->IsChangeRecording() )
     {
+        // all rows have already had tracked row change in the table selection
+        if ( !SwDoc::HasRowNotTracked( *getShellCursor( false ) ) )
+            return false;
+
         SwEditShell* pEditShell = GetDoc()->GetEditShell();
-        SwRedlineTable::size_type nOldRedlineCount = pEditShell->GetRedlineCount();
         StartUndo(bCompleteTable ? SwUndoId::UI_TABLE_DELETE : SwUndoId::ROW_DELETE);
         StartAllAction();
 
@@ -347,12 +350,7 @@ bool SwFEShell::DeleteRow(bool bCompleteTable)
 
         EndAllActionAndCall();
         EndUndo(bCompleteTable ? SwUndoId::UI_TABLE_DELETE : SwUndoId::ROW_DELETE);
-
-        // track row deletion only if there were tracked text changes
-        // FIXME redline count can be the same in special cases, e.g. adding a
-        // new tracked deletion with removing an own tracked insertion...
-        if ( nOldRedlineCount != pEditShell->GetRedlineCount() )
-            return true;
+        return true;
     }
 
     StartAllAction();